Skip to content

Conversation

ickshonpe
Copy link
Contributor

@ickshonpe ickshonpe commented May 30, 2025

Objective

The names of some of the components in bevy_text are confusing and inconsistent:

  • TextLayoutInfo
    Contains the final computed text layout. The -Info suffix is redundant and potentially confusing.

  • TextLayout
    Holds the settings for text justification and line breaking. Not the layout.

  • ComputedTextBlock
    This holds the cosmic-text buffer. The name suggests that it contains the final result of a text relayout, but the buffer can be out-of-date until it is updated during the text schedule.

Solution

  • Rename
    • TextLayoutInfo -> ComputedTextLayout
    • ComputedTextBlock -> TextBuffer
  • Remove TextLayout.
  • Make JustifyText and LineBreak into components.
  • Require JustifyText and LineBreak on Text and Text2d.

Fixes #19467

@ickshonpe ickshonpe added D-Trivial Nice and easy! A great choice to get started with Bevy C-Code-Quality A section of code that is hard to understand or change A-Text Rendering and layout for characters S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 30, 2025
@ickshonpe ickshonpe added the M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label May 30, 2025
ickshonpe added 7 commits May 30, 2025 20:30
TextLayoutInfo -> ComputedTextLayout
ComputedTextBlock -> TextBuffer
Remove the TextLayout component.

`JustifyText` and `LineBreak` are now components `Require`d by `Text` and `Text2d`.
Copy link
Contributor

github-actions bot commented Jun 2, 2025

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-19444

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

@ickshonpe ickshonpe added D-Straightforward Simple bug fixes and API improvements, docs, test and examples and removed D-Trivial Nice and easy! A great choice to get started with Bevy labels Jun 2, 2025
@ickshonpe ickshonpe changed the title Rename TextLayout Text renamings Jun 2, 2025
@ickshonpe ickshonpe changed the title Text renamings Text components refactor Jun 7, 2025
@rparrett
Copy link
Contributor

Just noting that this seems to be going in the opposite direction of a sentiment expressed here: #15014 (comment) with regard to the new LineBreak and JustifyText components.

I think this might merit more discussion to reduce potential churn. The main issue I am seeing with TextLayout at the moment is that I think LineHeight should be in there instead of in TextFont.

@kristoff3r
Copy link
Contributor

Just noting that this seems to be going in the opposite direction of a sentiment expressed here: #15014 (comment) with regard to the new LineBreak and JustifyText components.

I think this might merit more discussion to reduce potential churn. The main issue I am seeing with TextLayout at the moment is that I think LineHeight should be in there instead of in TextFont.

Ah yeah this was heavily discussed there and also in #15591 and #15797, I wasn't aware of that. I think this needs review from the people involved in those discussions.

@alice-i-cecile alice-i-cecile added X-Controversial There is active debate or serious implications around merging this PR S-Needs-SME Decision or review from an SME is required and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jun 29, 2025
@UkoeHB
Copy link
Contributor

UkoeHB commented Jul 1, 2025

I agree with ComputedTextLayout for consistency, but don't agree about changing ComputedTextBlock -> TextBuffer. The Computed prefix is a good pattern for marking components driven by bevy systems (like ComputedNode). ComputedTextBlock cannot be mutated outside bevy, so it should remain Computed.

Also, the fact ComputedTextBlock contains a buffer is an implementation detail that users almost never need to care about. So calling it a Buffer is less appropriate than TextBlock.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Text Rendering and layout for characters C-Code-Quality A section of code that is hard to understand or change D-Straightforward Simple bug fixes and API improvements, docs, test and examples M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Needs-SME Decision or review from an SME is required X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Misleading namings in bevy_text
5 participants