Retain UI phase items from frame to frame, and consolidate the UI queuing systems into one. - #25290
Retain UI phase items from frame to frame, and consolidate the UI queuing systems into one.#25290pcwalton wants to merge 1 commit into
Conversation
queuing systems into one. Although PR bevyengine#24893 added retention for UI render world instances themselves, in order to avoid re-extracting them from the main world ECS every frame, we still recreate the `TransparentUi` phase items every frame via `add_transient()`, which additionally removes them from the phase at the end of every frame. This is a significant CPU time sink and isn't the preferred pattern in Bevy nowadays. This commit makes the UI-related phase items retained just as 3D meshes are. All `queue_` methods in `bevy_ui_render` have been updated to walk the list of changed and removed meshes and update elements in the `SortedRenderPhase` only as necessary. The calls to `add_transient()` have been removed in favor of the more modern `add_retained()`. Additionally, all the custom queuing systems have been consolidated into a single generic system, `queue_ui_items`. The resources that hold extracted UI items have likewise been consolidated into a generic `UiRenderObjects` resource. The behavior specific to each individual item type (normal UI nodes, box shadows, gradients, etc.) has been factored into a trait named `UiRenderObject`. This has resulted in dramatic simplifications throughout UI rendering. See the documentation for more information. On `many_buttons`, this PR reduces the median frame time from 36.95 ms to 22.78 ms, or 27 FPS to 44 FPS. The `queue_uinodes` system has gone from 6.21 ms/frame to 15.2 μs/frame, a 409× speedup. And, because the Rust standard library's sorting algorithm is good at sorting data that's close to already sorted, the `sort_phase_system` time decreases from 5.17 ms/frame to 1.29 ms/frame, a 4.01× speedup.
pcwalton
added
C-Performance
pcwalton
added
the
S-Needs-Review
|
Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke! If it's expected, please add the M-Deliberate-Rendering-Change label. If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it. |
There was a problem hiding this comment.
Overall, everything looks really good.
There might be some ideas from #25289 that the UiRenderObjects won't map to, I think. For instance with debug outlines, it uses a MainEntityHashMap<ExtractedDebugOutline> map. Because it's just a stack of lines, it only needs to be a single phase item with one representative render entity. That PR isn't as well thought out as this one though, maybe there's some way to make it work with UiRenderObject. And these changes don't force us to use the UiRenderObjects API in every case if we need some flexibility.
Mostly everything is very simple and obviously correct. I found one bug: it seems like renderable objects, apart those extracted into ExtractedUiNodes, aren't drawn unless they are reupdated after spawning.
The gradients example makes it clear:
cargo run --example gradients
The static gradients on the left aren't visible, only the animated nodes on the right are rendered. Pressing the "previous" or "next" buttons updates the gradients, then they become visible and remain visible.
Similarly, with:
cargo run --example box_shadow --features="bevy_feathers"
Initially the shadow is missing. But after changing any of the options in the menu, the shadow appears and then remains visible.
| pipeline_key_builder: E::create_view_pipeline_key_builder(pipeline_key_builder_item), | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
The lib module is getting a bit big, this could be moved into its own module. Either that or we could have a uinode module and move lib's extract_* and prepare_uinodes there, along with all the associated types.
| .ok() | ||
| .and_then(|default_camera_view| { | ||
| let view = extracted_views.get(default_camera_view.0).ok()?; | ||
| let pipeline_key_builder = render_views.get(default_camera_view.0).ok()?; |
There was a problem hiding this comment.
The per camera view components like BoxShadowSamples and UiAntiAlias are stored on the current camera entity, not the UI view entity pointed at by UiCameraView:
| let pipeline_key_builder = render_views.get(default_camera_view.0).ok()?; | |
| let pipeline_key_builder = render_views.get(this_camera_entity).ok()?; |
You can adjust the shadow samples on the box_shadow example to test when this is working.
There was a problem hiding this comment.
Also, there is a second bug I just realised I think. There is no change detection on these query parameters. In the box_shadow example again, the shadow doesn't update on changing the shadow samples. One of the other parameters has to be changed to trigger an update to see the result of the shadow samples change.
|
It is the same bug, changes to the material asset don't trigger a second update. It becomes visible if you change the window size. |
| .get_mut(&main_entity) | ||
| .iter_mut() | ||
| .flat_map(|(_, gradients)| gradients.drain(..)) | ||
| // If there were any previous gradients for this entity, despawn them |
There was a problem hiding this comment.
This is the problem: If an object already exists in the objects list, it is removed and added to changed. But there is no mechanism to add an object to the changed list on the frame it is spawned. Phase items are added from the changed list, so it doesn't get queued for rendering. On a reupdate though, now the object is present in the objects list, so it can be removed, is added to the changed list, and then is queued correctly.
2 participants
Although PR #24893 added retention for UI render world instances themselves, in order to avoid re-extracting them from the main world ECS every frame, we still recreate the
TransparentUiphase items every frame viaadd_transient(), which additionally removes them from the phase at the end of every frame. This is a significant CPU time sink and isn't the preferred pattern in Bevy nowadays.This commit makes the UI-related phase items retained just as 3D meshes are. All
queue_methods inbevy_ui_renderhave been updated to walk the list of changed and removed meshes and update elements in theSortedRenderPhaseonly as necessary. The calls toadd_transient()have been removed in favor of the more modernadd_retained().Additionally, all the custom queuing systems have been consolidated into a single generic system,
queue_ui_items. The resources that hold extracted UI items have likewise been consolidated into a genericUiRenderObjectsresource. The behavior specific to each individual item type (normal UI nodes, box shadows, gradients, etc.) has been factored into a trait namedUiRenderObject. This has resulted in dramatic simplifications throughout UI rendering. See the documentation for more information.On
many_buttons, this PR reduces the median frame time from 36.95 ms to 22.78 ms, or 27 FPS to 44 FPS. Thequeue_uinodessystem has gone from 6.21 ms/frame to 15.2 μs/frame, a 409× speedup. And, because the Rust standard library's sorting algorithm is good at sorting data that's close to already sorted, thesort_phase_systemtime decreases from 5.17 ms/frame to 1.29 ms/frame, a 4.01× speedup.