Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Text should not store a flat list of sections #7714

Closed
alice-i-cecile opened this issue Feb 16, 2023 · 10 comments
Closed

Text should not store a flat list of sections #7714

alice-i-cecile opened this issue Feb 16, 2023 · 10 comments
Labels
A-Accessibility A problem that prevents users with disabilities from using Bevy A-Text Rendering and layout for characters A-UI Graphical user interfaces, styles, layouts, and widgets C-Usability A targeted quality-of-life change that makes Bevy easier to use

Comments

@alice-i-cecile
Copy link
Member

What problem does this solve or what need does it fill?

Currently, the Text component stores TextSection objects in a simple vector. Each entity with a Text component represents a contiguously laid out block of text that can be used to make a text node in the UI (or in your game): the distinct sections are required to be able to change the style within this block text.

This leads to very brittle, hard to read code, as seen in this real world example.
This uses a classic "retained-mode" pattern to initialize text, and then only update the segments that need updating.

There are three key problems here:

  1. It is very hard to tell which section is being modified, without looking at a non-local initialiatization.
  2. It's very easy to accidentally index out of bounds, causing a crash.
  3. When we modify the order or number of text sections, any code that reads or mutates these fields will be silently broken.

This is particularly troublesome as automated testing of UI is unusually hard: both because of the visual nature and because of the large number of possible states to check.

Proposal: HashMap in Text

The simplest solution: just replace the Vec<TextSection> with a Hashmap<TextSection>.
This works reasonably well, but forces us to always store keys and may increase verbosity.

Hashmaps are also pretty silly to use for large numbers of objects with a median count of 2 or so.

Proposal: Text sections as entities

Text becomes a marker component. TextSection becomes a component, each stored on a distinct child of the Text entity.
Then use the Name component to differentiate these text sections.
Either use marker components or something like get_child_by_name to access the text sections.

Verbose, fragile, much more ECS.
Like all things in life, really would be easier and more reliable if we had #3742.

Additional context

We should consider how this fits in with #6874: the desire to name the text sections reminds me a lot of the element labels that AccesKit wants.

This "text sections" problem is commonly encountered when working with localization. We could force all text to be easily localized, and store a fluent-rs data type as part of our TextSection type, along with style data.

May be better tackled as part of or after #7616.

@alice-i-cecile alice-i-cecile added A-Accessibility A problem that prevents users with disabilities from using Bevy A-UI Graphical user interfaces, styles, layouts, and widgets C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Feb 16, 2023
@IceSentry
Copy link
Contributor

IceSentry commented Feb 16, 2023

I think the current api makes sense as a low level construct and shouldn't be changed for high level needs.

I think the main issue is that all the text stuff is too low level. I believe this should be solved with something like a TextWidget that would be higher level with a nicer api. The text section stuff mostly only exist as an optimization for rendering just so you don't re-render text that hasn't changed (at least, that's what I remember, it's been a while since I looked at this part). This potential widget could also be able to keep track of which part of the text actually changed and only update that section accordingly. It would need some way to bind a value to a section, but updating the text could be done automatically.

@tigregalis
Copy link
Contributor

tigregalis commented Feb 17, 2023

I agree with the proposal that sections/spans should be entities (the parent decides how to layout the children). For making a single section/span dynamic, you could just add a marker component to it. The parent itself probably doesn't need to know about text, just inline and/or reflowable child elements, which means you could support inline images for example (you would probably need to hack the layout algorithm here though).

While we're at it, rename TextSection to TextSpan; Span is more familiar terminology from the Web (the HTML <span> element). The reason Section was adopted was because it was ab_glyph terminology. Span also benefits from being less characters to type. :)

@nicoburns
Copy link
Contributor

nicoburns commented Feb 17, 2023

IMO TextSection shouldn't be storing string data at all. I think:

  • Text should store the entire text data as a single String (and/or a Rope or similar data structure)
  • TextSection/TextSpan should consist of a style and start/end indexes into the underling Text

I believe that this is basically the format that cosmic-text expects as input anyway.

I concur with @tigregalis that a higher level API is required (e.g. a minimal subset of HTML, markdown or similar). It shouldn't be common to manually juggle text sections.

Regarding inline images and other embedded objects. I think it might be appropriate to push layout of these down into cosmic-text (cosmic-text wouldn't know about images, it would just know how to layout rectangles mixes in with text). At the Bevy level, I think support for this ought to be built in to the Text component if possible.

@ickshonpe
Copy link
Contributor

Not that its worth changing now, but Is the current Text implementation incorrect?
It seems like maybe the intent with the glyph_layout API is that you store your text in a single contiguous string and then what you pass to glyph_layout is a vector of slices that partition the string along with the font and scaling data.

@tigregalis
Copy link
Contributor

tigregalis commented May 6, 2023

Not that its worth changing now, but Is the current Text implementation incorrect? It seems like maybe the intent with the glyph_layout API is that you store your text in a single contiguous string and then what you pass to glyph_layout is a vector of slices that partition the string along with the font and scaling data.

I guess it depends how you interpret the example:

https://docs.rs/glyph_brush_layout/latest/glyph_brush_layout/#example

// Layout "hello glyph_brush_layout" on an unbounded line with the second
// word suitably bigger, greener and serif-ier.
let glyphs = Layout::default().calculate_glyphs(
    fonts,
    &SectionGeometry {
        screen_position: (150.0, 50.0),
        ..SectionGeometry::default()
    },
    &[
        SectionText {
            text: "hello ",
            scale: PxScale::from(20.0),
            font_id: FontId(0),
        },
        SectionText {
            text: "glyph_brush_layout",
            scale: PxScale::from(25.0),
            font_id: FontId(1),
        },
    ],
);

In the main example SectionText holds string slices, but these are &'static strs rather than slices of owned Strings. But you probably could just pass slices.

cosmic_text on the other hand is something like:

let buffer = Buffer {
    lines: vec![ // Vec of lines
        BufferLine {
            text: String::from("something"), // owned string for the current line
            attrs_list: AttrsList {
                spans: RangeMap::<usize, AttrsOwned>::new() // apply "attributes" (font, etc.) to slices of this line
            }
        }
    ]
}

It's worth noting glyph_brush_layout splits text into lines for you, but cosmic_text only splits these into lines if you use the Buffer:: set_text method.

In my WIP integration of cosmic_text, because of the need for more than one style, I can't simply use Buffer::set_text as it only takes a single attribute. I construct each BufferLine independently:

        // all sections need to be combined and broken up into lines
        // e.g.
        // style0"Lorem ipsum\ndolor sit amet,"
        // style1" consectetur adipiscing\nelit,"
        // style2" sed do eiusmod tempor\nincididunt"
        // style3" ut labore et dolore\nmagna aliqua."
        // becomes:
        // line0: style0"Lorem ipsum"
        // line1: style0"dolor sit amet,"
        //        style1" consectetur adipiscing,"
        // line2: style1"elit,"
        //        style2" sed do eiusmod tempor"
        // line3: style2"incididunt"
        //        style3"ut labore et dolore"
        // line4: style3"magna aliqua."

cosmic_text could probably expose a cleaner way to construct text with mixed styles, or "push" text with different styles, but what I've put together works.


I'm curious, without treating each span as its own potentially mutable string, how you express "dynamic rich text" through code.
The DOM uses text nodes and/or spans for mutability, and spans for styling.

I think text spans as entities is probably a good low-level API which you could build a high-level API on top of. In theory it supports arbitrary nesting too, so it could map pretty well to the DOM, though I don't know if we'd want that, or perhaps we might explicitly not want it.

For a high-level API (for UI in general), I think you'd probably want something JSX-like for familiarity and flexibility. This would generate the relevant spans (and UI nodes) and provide the logic to somehow keep the UI in sync with whatever state you want the UI to observe (instead of mutating the text and UI entities directly, a bevy engine system(s) handles that for you).

@nicopap
Copy link
Contributor

nicopap commented May 13, 2023

Dynamic text requires some data structure that supports growing inner sections. Consider this:

The player has hit training dummy {dummy_hits} times.

Where we want to replace {dummy_hits} with an arbitrary number.

When we go from 9 to 10, we need to insert an additional character within the text. A single str for the whole text would require .inserting, a O(n) operation. The current impl and the proposed entity-based impl are a list of strings (also known as Iliffe vector), which perfectly supports inner extension. A rope would also supports inner extension. It breaks down on a large string.

Now consider styling in this. If the style is tracked separately from the text, it is now out of sync! Do I have to manually update this as well? That sounds fairly error-prone.

This is not an edge case, it's really the basic use case. An FPS counter falls into this already.

Regarding how "dynamic rich text" would look like, I've an answer, but I think it's out of scope for this issue :P

@viridia
Copy link
Contributor

viridia commented Sep 9, 2023

It's interesting to consider this issue in light of recent discussions about "UIs as scenes" and entity relationships.

The proposed syntax for defining a scene asset is likely going to be convenient for building node graphs of widgets and react-like components, but maybe not so friendly for spans of text. JSX, HTML, XML and similar SGML-derived markups all give text nodes special status in that it's the default node type - you don't need quotes or special markers to identify a text section. But if you're talking about any non-SGML-derived serialization format (JSON, RON, bsn, etc.), there's going to be quotes and possibly even explicit "this is a text node" syntax. This is fine, for the most part, because we're not trying to write a book, we just want things like button labels and lists of saved game titles. (In cases where we are trying to write a book - like help pages or quest descriptions - it's probably the case we're reading text in a different format, probably from a localized catalog asset.)

An advantage of having text-nodes be entities is that you can then intermix text blocks with other kinds of renderable UI nodes which are entities. However, traditionally games have not used this kind of inline layout much, most text displays in games contain only text, with possibly hyperlinks being underlined and colored. The only use case that comes to minds is inline icons and drop-caps. Still, I do like the idea of text spans being entities.

Inline layout is more complex than flex layout, and you only want to use it when you need to - that is, only when a 2d node has one or more inline children. This could either be detected automatically, or via an explicit hint (display: block).

@tigregalis
Copy link
Contributor

tigregalis commented Sep 10, 2023

An advantage of having text-nodes be entities is that you can then intermix text blocks with other kinds of renderable UI nodes which are entities.

I agree with this. The existing text.sections would be not nice (but still possible) to do this with.

Inline layout is more complex than flex layout, and you only want to use it when you need to - that is, only when a 2d node has one or more inline children. This could either be detected automatically, or via an explicit hint (display: block).

I think you only need to be able to layout arbitrary rectangles. Ideally the text layout (provided by glyph_brush_layout and soon cosmic-text) handles this. See pop-os/cosmic-text#80 proposal, but it can be hacked around by passing an invisible "glyph" of your chosen size, then drawing the thing you actually want to render there at that location.

@viridia
Copy link
Contributor

viridia commented Oct 13, 2023

See also #9944.

@viridia viridia added the A-Text Rendering and layout for characters label Apr 17, 2024
@viridia
Copy link
Contributor

viridia commented Apr 17, 2024

@nicoburns @alice-i-cecile Ran into this issue again today, wanted to share my experiences.

I'm strongly in favor of a "text as entities" approach. This does not necessarily mean a "flat" list. Instead, I would follow the CSS/HTML approach:

  • Introduce a new layout type, Inline, which is exclusive from Flex or Grid layout.
  • Any container that has "inline" layout will position its children by wrapping/breaking the text content in it, as well as any non-text objects.
  • Inline layouts can be nested, in which case the result is "flattened" during rendering. This gives you tree-like abilities within the standard ECS parent/child framework. Inline containers can be used as interior tree nodes, in a manner analogous to SpatialBundle.

While I recognize that there will be challenges with this approach, it has some compelling advantages: It allows a unified approach to applying styles to text and non-text elements.

In the current system, the way that styles are applied to text sections is completely different than the way styles are applied to non-text elements. While this may not seem like a big deal - there's only so much overlap between the set of style properties relevant to text and non-text entities - consider what happens when we start to introduce animation or reactivity into the picture. In my case, there's a large reactive infrastructure that is predicated on the idea that styles are targeted at entities. In order to apply styles to text sections, I'd need to build a whole new kind of reaction infrastructure that was capable of targeting individual text sections.

Even without the complexity introduced by reactivity, consider what happens when want to animate something like text colors - for example a slowly pulsating / throbbing hyperlink, an idea that is not entirely inconceivable in the context of a game UI. Ideally, animation should be driven by components - attach an animation component to a UI node in order to animate it. But this won't work with text sections as they are represented today, you'd need to build an entirely different animation framework.

An additional advantage of this approach is familiarity: for people who are coming from the web/HTML world, this "just works" the way they would expect it to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Accessibility A problem that prevents users with disabilities from using Bevy A-Text Rendering and layout for characters A-UI Graphical user interfaces, styles, layouts, and widgets C-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants