UI extraction improvements v2 #9668
Conversation
Instead of a single list of `ExtractedUiNodes`, we have a separate list for each extraction function. Then we don't need to sort `ExtractedUiNodes` in `prepare_uinodes` as each sublist is already ordered.
Instead of sorting `ExtractedUiNodes`, add a lookup index. Then in prepare_uinodes sort the indices by `stack_index` then use the sorted indices to retrieve each `ExtractedUiNode` in the correct order. * Added a field `indices` to `ExtractedUiNodes`. * Added `ExtractedIndex` type.
* New component, holds the uinode entity's stack index. Updated in `ui_stack_system after the `UiStack` is generated. `bevy_ui::node_bundles` * Added the `UiStackIndex` to all UI node bundles. `bevy_ui::render` * Extract stack indices from the `UiNode` component.
…ck-index-component
* Made `ExtractedIndex` and the fields of `ExtractedUiNodes` private. * Added a `clear` method to `ExtractedUiNodes`.
`ExtractedIndex` holds a range of indices instead of a single index. The `ExtractedUiNodes::push_multiple` method is used to push multiple nodes and store them contiguously.
Renamed `ExtractedUiNodes::push` and `ExtractedUiNodes::push_multiple` to `ExtractedUiNodes::push_node` and `ExtractedUiNodes::push_nodes` respectively.
Renamed `ExtractedIndex` to `ExtractedSpan`.
* Changed inner value to `pub(crate)` and added an accessor method. * Added derive `Reflect` and reflect `Component`, `Default` attributes * Added a `register_type` to UIPlugin. `bevy_ui_node::ExtractedRange`: * Renamed from `ExtractedSpan`, `spans` field of `ExtractedUiNode` also renamed to `ranges`.
| pub fn queue_uinodes(mut extracted_uinodes: ResMut<ExtractedUiNodes>) { | ||
| extracted_uinodes | ||
| .ranges | ||
| .sort_unstable_by_key(|extracted_range| extracted_range.sort_key); |
There was a problem hiding this comment.
I'll try it out.
I made an alternative implementation as well that doesn't require a sort. Instead of storing the layout geometry in components on the uinode entities it has a single layout entity that has a list of all the uinode rects in a vector. It might be a better approach and the prototype seems quite promising but I don't have most of the systems like clipping, interaction, etc working yet. Everything needs to be rewritten to support it.
There was a problem hiding this comment.
Which is which? What is being compared?
| ) { | ||
| let draw_function = draw_functions.read().id::<DrawUi>(); | ||
|
|
||
| for (view, mut transparent_phase) in &mut render_phases.p1() { |
There was a problem hiding this comment.
This approach goes against the design of the renderer. How come you didn't do the queuing of phase items to phases based on the ranges in queue and then let the phase sort them? This is necessary for flexibility to allow plugins to also queue items to the phase.
alice-i-cecile
added
A-Rendering
|
@ickshonpe I want to keep this in the release milestone; can your resolve conflicts? |
alice-i-cecile
added
the
X-Controversial
This was called out as a blocking architectural decision. Once that's resolved, we can remove the Controversial label. |
alice-i-cecile
added
X-Contentious
|
Closed, superseded by the extract to a vec PR |
Objective
Reimplements #9212 with the recent rendering changes.
Solution
The design is mostly the same as #9212.
Benchmarks
(Yellow is this PR, Red is main)
These look good but it still seems to be around 10% to 25% worse than #9212 overall depending on the UI layout.
Not clear why, tried a lot of different ideas but they all made things worse. Merging with the latest visibility changes seemed to cost another 5% or so as well.
Changelog
ExtractedUiNodesto use aVecinstead of aSparseSetto store theExtractedUiNodesMigration Guide