bevyengine / bevy

A refreshingly simple data-driven game engine built in Rust
https://bevyengine.org
Apache License 2.0
36.74k stars 3.61k forks source link

Changing the color of a TextSection triggers a redraw of the full Text #6967

Open ickshonpe opened 1 year ago

ickshonpe commented 1 year ago

Bevy version

0.9.1

What went wrong

Changing the Color of a TextSection trips change detection causing the entire Text to be needlessly recomputed.

Additional information

Workaround:

text_query.for_each_mut(|mut text| {
  text.bypass_change_detection().text.sections[0].style.color = new_color;
});

related: #6956, #5935

tigregalis commented 1 year ago

That is true. colors are the only parts of Text that don't affect layout.

Text {
    sections: vec![
        TextSection {
            style: TextStyle {
                color: // ...
                // ...
            }
            // ...
        }
    ],
    // ...
}
... ```rust pub struct Text { pub sections: Vec, pub alignment: TextAlignment, } pub struct TextSection { pub value: String, pub style: TextStyle, } pub struct TextAlignment { pub vertical: VerticalAlign, pub horizontal: HorizontalAlign, } pub enum VerticalAlign { Top, Center, Bottom, } pub enum HorizontalAlign { Left, Center, Right, } pub struct TextStyle { pub font: Handle, pub font_size: f32, pub color: Color, } ```

Just exploring the many possible solutions:

Detect changes to color only and don't re-layout

Adapt the text layout system to special-case for color-only changes, but does change detection even have this level of granularity? Can it?

color lifted out of Text into its own component

But then how do you link the individual text sections to the various colors? e.g. would you have

#[derive(Component)]
struct TextSectionColors(Vec<Color>);

but this would be painful to synchronise with the entity's text sections.

color is a handle to an asset

struct TextStyle {
    color: ColorInstance
}

struct ColorInstance(Color);

This makes creating the text and changing the color less nice.

fn add_color_and_spawn(
    mut colors: ResMut<Assets<ColorInstance>>,
) {
    let handle = colors.add(ColorInstance(Color::RED));
    // do something with the handle
}

fn change_color(text_query: Query<Text>, mut colors: ResMut<Assets<ColorInstance>>) {
    text_query.for_each_mut(|mut text| {
      if let Some(color) = colors.get(&text.sections[0].style.color) {
          color.0 = new_color;
      }
    });
}

On the other hand, you could potentially reuse that color for other sections (even in other entities). Still, if someone actually swaps out the referenced color handle (for whatever reason), it can still result in a re-layout, so best practice would be to always only have one color handle to one section. You'd also want to destroy these assets as sections are destroyed. Perhaps it could be made more ergonomic to provide some type of special garbage collected asset and/or some type of "unique" handle.

TextColor could be an enum

struct TextStyle {
    color: TextColor
}

enum TextColor {
    Plain(Color)
    Handle(Handle<ColorInstance>)
}

It could be a plain color, or a handle to an asset, so you get to choose (and you document the tradeoff).

TextSections are their own entities, as children of Text entities

Then you could have

#[derive(Component)]
struct TextSectionColor(Color);

// Parent
struct Text {
    sections: Vec<Entity>,
    alignment: 
}

// Child

The challenge is ensuring you only use TextSection as children*. This is the kinded entities problem.

*On the other hand, at this point, the only thing from Text that's still relevant is TextAlignment, but this starts to look simply like a Block and InlineChildAlignment, and there's no reason you can't have other inline elements (other than implementation).

This is probably the "correct" solution (and some of the explorations above can be orthogonal), but the ergonomics are a bit lacking (compared to the current status quo) and it's the most work.