-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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_system
split
#7779
text_system
split
#7779
Conversation
* Added a new trait `MeasureNode`. * Added new structs `ImageMeasure` and `BasicMeasure` that implement `MeasureNode`. * Add a field to `CalculatedSize` called `measure` that takes a boxed `MeasureNode`. * `upsert_leaf` uses the `measure` of `CalculatedSize` to create a `MeasureFunc` for the node.
…into split-text-system
* Renamed `CalculatedSize` to `IntrinsicSize`. It is now non-copy. Added a field `measure` and removed `preserve_aspect_ratio`. * Added `measurement` module to `bevy_ui` * Added `Measure` trait. A `Measure` is used to compute the size of an intrisically sized node. * Added `ImageMeasure` and `FixedMeasure` `Measure` implementations. * Changed `update_image_calculated_size_system` to use `ImageMeasure`. * Changed `upsert_leaf` to use the `Measure` of `CalculatedSize` for the `MeasureFunc` of intrinsically sized nodes.
* Added the system (dummy atm) `measure_text_system` * Changed the system execution order so that `measure_text_system` replaces `text_system` in the order. `text_system` now runs after `UiSystem::Flex`. Previously the `text_system` ran before the layout was calculated and the size of the text node was determined, so it couldn't shape the text correctly to fit the layout, and had no way of determining if the text needed to be wrapped. There was a hack, the system `text_constraint` that tried to determine the size of the node from the local size constraints of the node in its `Style` component. This could not work correctly, `Val::Percent` constraints just had to be ignored as they are calcualted from size of the parent node and the `Val::Px` constraints are just a guess, without computing the rest of the layout. Also because the `text_system` queried for changes to the `Style` component, and not the `Node` component, it couldn't react to changes in the layout correctly. The layout system then wouldn't recieve all the information it would need to fit the text node correctly, such as the `max-content` and `min-content` sizes.
changes: * Removed `TextQueue` * Added `min_content`, `max_content` and `ideal` fields to IntrinsicSize. * Added `ideal_height` field to `TextMeasure`. * Fixed text system queueing and change detection issues. `measure_text_system` only queries for modified `Text`, `text_system` queuries for `Text` or `Node` changes.
CC @TimJentzsch @nicoburns for review. |
I think the text node's dimensions need to computed inside the measure function, because the measure function needs to be able to answer the question "Given that your width is X, what should your height be?". That can't be precomputed. The way I'd imagine this working is something like:
|
Oh no. I was trying to do it that way, precalculate the glyph geometry, and then do the layout calculations in the measurefunc except (for no reason at all) I believed I'd still need access to the TextPipeline and Font data from inside the measurefunc 😅. Instead, I've got this terrible hack where I compute the layout initially with the min_content height, generate the text to fit the returned width and then recompute the layout again with the height of the generated text. That simplifies things enormously, should be able to clean up everything now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- This is excellent effort!
- I do agree with Nicoburns on the implementation design.
Co-authored-by: François <mockersf@gmail.com>
Co-authored-by: François <mockersf@gmail.com>
…into split-text-system
# Objective Followup to #7779 which tweaks the actual text measurement algorithm to be more robust. Before: <img width="822" alt="Screenshot 2023-04-17 at 18 12 05" src="https://user-images.githubusercontent.com/1007307/232566858-3d3f0fd5-f3d4-400a-8371-3c2a3f541e56.png"> After: <img width="810" alt="Screenshot 2023-04-17 at 18 41 40" src="https://user-images.githubusercontent.com/1007307/232566919-4254cbfa-1cc3-4ea7-91ed-8ca1b759bacf.png"> (note extra space taken up in header in before example) ## Solution - Text layout of horizontal text (currently the only kind of text we support) is now based solely on the layout constraints in the horizontal axis. It ignores constraints in the vertical axis and computes vertical size based on wrapping subject to the horizontal axis constraints. - I've also added a paragraph to the `grid` example for testing / demo purposes.
* a snapshot * a snapshot with bugs * no compile errors * an attempt * another snapshot still buggy * clean up * a snapshot * use loop for building components * tiny tweaks * switch to Bevy - modified: Cargo.toml - new file: assets/fonts/NotoSansCJKjp-Regular.otf - modified: src/main.rs * modified: Cargo.toml; modified: src/main.rs * add button from examples/ui/button.rs * add async stuff * add async data fetcher * tiny changes * add some memo * change labels to buttons * desktop mode; fix some glitches * add some interaction * switch to reqwest * use a modified copy of https://github.com/TotalKrill/bevy_mod_reqwest * clean up * written massively * add Timeline component * refactor Components and Resources * update display with recieved data * UI * cargo clippy: src/reqwest_plugin.rs * try to make contents wider and wrapped * revise layout settings * editorial design * more UI tweaks * ditto * UI tweaks and refactoring * modified: Cargo.toml * adapt to auto-reflow text by bevyengine/bevy#7779 (comment) * mention https://github.com/TotalKrill/bevy_mod_reqwest
# Objective In Bevy 10.1 and before, the only way to enable text wrapping was to set a local `Val::Px` width constraint on the text node itself. `Val::Percent` constraints and constraints on the text node's ancestors did nothing. #7779 fixed those problems. But perversely displaying unwrapped text is really difficult now, and requires users to nest each `TextBundle` in a `NodeBundle` and apply `min_width` and `max_width` constraints. Some constructions may even need more than one layer of nesting. I've seen several people already who have really struggled with this when porting their projects to main in advance of 0.11. ## Solution Add a `NoWrap` variant to the `BreakLineOn` enum. If `NoWrap` is set, ignore any constraints on the width for the text and call `TextPipeline::queue_text` with a width bound of `f32::INFINITY`. --- ## Changelog * Added a `NoWrap` variant to the `BreakLineOn` enum. * If `NoWrap` is set, any constraints on the width for the text are ignored and `TextPipeline::queue_text` is called with a width bound of `f32::INFINITY`. * Changed the `size` field of `FixedMeasure` to `pub`. This shouldn't have been private, it was always intended to have `pub` visibility. * Added a `with_no_wrap` method to `TextBundle`. ## Migration Guide `bevy_text::text::BreakLineOn` has a new variant `NoWrap` that disables text wrapping for the `Text`. Text wrapping can also be disabled using the `with_no_wrap` method of `TextBundle`.
Objective
text_system
runs before the UI layout is calculated and the size of the text node is determined, so it cannot correctly shape the text to fit the layout, and has no way of determining if the text needs to be wrapped.The function
text_constraint
attempts to determine the size of the node from the local size constraints in theStyle
component. It can't be made to work, you have to compute the whole layout to get the correct size. A simple example of where this fails completely is a text node set to stretch to fill the empty space adjacent to a node with size constraints set toVal::Percent(50.)
. The text node will take up half the space, even though its size constraints areVal::Auto
Also because the
text_system
queries for changes to theStyle
component, when a style value is changed that doesn't affect the node's geometry the text is recomputed unnecessarily.Querying on changes to
Node
is not much better. The UI layout is changed to fit theCalculatedSize
of the text, so the size of the node is changed and so the text and UI layout get recalculated multiple times from a single change to aText
.Also, the
MeasureFunc
doesn't work at all, it doesn't have enough information to fit the text correctly and makes no attempt.Fixes #7663, #6717, #5834, #1490,
Solution
Split the
text_system
into two functions:measure_text_system
which calculates the size constraints for the text node and runs beforeUiSystem::Flex
text_system
which runs afterUiSystem::Flex
and generates the actual text.MeasureFunc
calculations.Text wrapping in main:

With this PR:

Changelog
CalculatedSize
.CalculatedSize
now contains a boxedMeasure
.measurement
module tobevy_ui
.create_text_measure
toTextPipeline
.measure_text_system
that runs beforeUiSystem::Flex
that creates aMeasureFunc
for the text.text_system
to run afterUiSystem::Flex
.Measure
. AMeasure
is used to compute the size of a UI node when the size of that node is based on its content.ImageMeasure
andTextMeasure
which implementMeasure
.UiImageSize
which is used byupdate_image_calculated_size_system
to track image size changes.UiImageSize
component toImageBundle
.Migration Guide
ImageBundle
has a new componentUiImageSize
which contains the size of the image bundle's texture and is updated automatically byupdate_image_calculated_size_system