Text change detection split

[ickshonpe]

Objective

The detect_text_needs_rerender system runs twice, once querying for all Text root UI node entities and once querying for all Text2d 2d root entities. But both times it queries for all TextSpans. This causes two problems:

• All TextSpans are checked twice for changes.

• If there is an error with a TextSpan, detect_text_needs_rerender::<Root> reports the current Root type parameter:

           once!(warn!("found entity {} with a TextSpan that has a TextLayout, which should only be on root \
               text entities (that have {}); this warning only prints once",
               entity, core::any::type_name::<Root>()));

  But since the system checks all TextSpans for both root types, the reported root type is determined by the system order. Not the type of the specific TextSpan's root.

Fixes #19437

Solution

Split detect_text_needs_rerender into two systems detect_text_root_needs_rerender and detect_text_span_needs_rerender.
detect_text_root_needs_rerender runs twice, once for Text and once for Text2d, while detect_text_span_needs_rerender only runs once.

The root type has been removed from all the warnings. I considered adding Has<Text> and Has<Text2d> parameters but this already quite convoluted and I don't think identifying the type of root is very useful here. Also it wouldn't be compatible with a custom text implementation using alternative root components.

Testing

With line 114 of many_buttons changed to:

        app.add_systems(Update, |mut text_query: Query<&mut TextSpan>| {

run

cargo run --example many_buttons --release --features bevy/trace_tracy -- --recompute-text --image-freq 0 --buttons 25

many_buttons has no Text2d entities but you can see detect_text_root_needs_rerender::<Text2d> on main doing work:

The result looks exciting but this is with an unrealistically large number of text sections. For normal apps though the improvements won't be measurable and they might even be exceeded by the overhead of the new detect_text_span_needs_rerender system. The fix for the deceptive warnings is the more important part of this PR.

[UkoeHB]

This means text span rerender detection is not locked into the UiSystems system sets. Which means systems in PostUpdate that deal with text stuff .before(UiSystems::Prepare) could be ordered wrong relative to text span rerender detection.

Can you move detect_text_span_needs_rerender to a system set then in this plugin do .configure_sets(PostUpdate, TextSpanDetectSet.in_set(UiSystems::Content)?

[UkoeHB]

Although that may screw up text2d ordering. Which is might be why I originally allowed duplicate text span change detection.

[ickshonpe]

Although that may screw up text2d ordering. Which is might be why I originally allowed duplicate text span change detection.

I guess we could work around this a different way, maybe by adding a Has<Root> to the computed query? Walking all the text spans twice feels really unpleasant though, even if it's cheap in practice.

[UkoeHB]

I don't think Has<Root> in computed would change anything. Afaict we have these options:

• This PR with ordering constraints. Will couple text2d and UI system ordering, possibly losing parallelism.
• Invert the loop and traverse down from roots, manually checking for changed spans. This may be less performant than simply iterating spans twice, since hierarchy traversal is an order of magnitude more expensive per entity IIRC (hashmap lookup vs vec indexing, approximately).
• Change nothing. Continue to iterate all spans twice.