Adding TextOutline component to add outlining by upstreaming implemen… #19639

TotalKrill · 2025-06-14T14:31:05Z

Add Text Outline component to add text borders/outline to text nodes by upstreaming implementation from bevy_cobweb_ui

hukasu · 2025-06-14T14:50:25Z

Can you add some screenshots for showcase? With and without the component

I don't know if permission to upstream something is necessary, but have you talked with either someone from bevy and/or bevy_cobweb_ui?

hukasu · 2025-06-14T14:52:50Z

Also have 2 different lines for the fixes, GitHub only linked to the first issue

…tation from bevy_cobweb_ui fixes: bevyengine#17076 fixes: bevyengine#7210 Co-authored-by: koe <ukoe@protonmail.com>

alice-i-cecile · 2025-06-14T19:41:26Z

@UkoeHB are you okay if we upstream this?

UkoeHB · 2025-06-14T20:19:38Z

@alice-i-cecile yes, no problem.

crates/bevy_ui/src/widget/text.rs

release-content/release-notes/text_outline.md

ickshonpe · 2025-06-14T21:42:58Z

crates/bevy_ui/src/render/mod.rs

+        for offset_x in -width..=width {
+            for offset_y in -width..=width {


It's not obvious, but this has catastrophically awful worst case performance.

For example, suppose that a Text has three glyphs, each from a different font, and an outline width of 3. In the inner PositionedGlyphs loop the font texture atlas changes for each glyph, so a new batch is generated per glyph. The inner loop is repeated 48 times, so 144 batches are produced for just three glyphs.

Instead the PositionedGlyphs need to be iterated in the outer loop. Then all the outline sprites for each glyph will be batched together, at worst resulting in one batch per glyph.

Please have a look if I understood you correctly when you have a chance, I just moved the order of the iterations as I think you meant.

Co-authored-by: UkoeHB <37489173+UkoeHB@users.noreply.github.com>

…r to only one batch per glyph

UkoeHB · 2025-06-15T20:22:19Z

crates/bevy_ui/src/render/mod.rs

+                    if text_layout_info.glyphs.get(i + 1).is_none_or(|info| {
+                        info.span_index != *span_index
+                            || info.atlas_info.texture != atlas_info.texture
+                    }) {


Suggested change

if text_layout_info.glyphs.get(i + 1).is_none_or(|info| {

info.span_index != *span_index

|| info.atlas_info.texture != atlas_info.texture

}) {

if text_layout_info.glyphs.get(i + 1).is_none_or(|info| {

info.atlas_info.texture != atlas_info.texture

}) {

I think it's safe to remove this, since span-bound text color is irrelevant. @ickshonpe what do you think?

Also, this ExtractedUiNode bit needs to be moved to the outer loop.

If i move the extraced_uinodes.push bit, I get strange artifacts in the output...

If i move the extraced_uinodes.push bit, I get strange artifacts in the output...

Only extracted_uinodes.uinodes should be in the outer loop (after the inner loop). The .glyphs needs to stay in the inner loop.

Yeah, thats what I tried, and that ended up giving me the artifacts, I am not quite sure as to why, but the output hints at me pushing the values wrongly somehow..

extracted_uinodes.glyphs.push(ExtractedGlyph { transform: Affine2::from_mat3( transform * Mat3::from_translation(*position), ), rect, }); } // offset y } // offset x if text_layout_info.glyphs.get(i + 1).is_none_or(|info| { info.span_index != *span_index || info.atlas_info.texture != atlas_info.texture }) { start = end; extracted_uinodes.uinodes.push(ExtractedUiNode { render_entity: commands.spawn(TemporaryRenderEntity).id(), stack_index: uinode.stack_index, color, image: atlas_info.texture.id(), clip: clip.map(|clip| clip.clip), extracted_camera_entity, rect, item: ExtractedUiItem::Glyphs { range: start..end }, main_entity: entity.into(), }); } end += 1; }

Ah looks like end += 1; needs to be moved to the inner loop after pushing to .glyphs. And start = end; needs to move to the top of the outer loop. The conditional there should be moved to a separate bool variable since you'll have two blocks conditioned on it.

Ok I fixed it in bevy_cobweb_ui: https://github.com/UkoeHB/bevy_cobweb_ui/blob/a3265e1c80a4b21cdd7cf8200e294f2a6712248e/src/ui_bevy/ui_ext/text_rendering.rs#L11

I think it's safe to remove this, since span-bound text color is irrelevant. @ickshonpe what do you think?

Yep not needed.

ickshonpe

Just had another look before I go to sleep, I see more problems now but too tired to think things through properly:

It's going to be awkward to get it to work well with scale factor or transform scaling transform, might not be possible even.
The offsets should radiate out rather than being arranged in a grid.
Outline width should be clamped to a relatively low max value.

crates/bevy_ui/src/render/mod.rs

ickshonpe · 2025-06-15T23:35:01Z

crates/bevy_ui/src/render/mod.rs

+
+                    let transform = Affine2::from(*global_transform)
+                        * Affine2::from_translation(
+                            -0.5 * uinode.size() + offset / uinode.inverse_scale_factor(),


I think multiplying the offset scale factor is probably wrong, it might cause gaps in the outline at higher scale factors even. The outline width needs to be scaled instead.

ickshonpe · 2025-06-15T23:38:58Z

crates/bevy_ui/src/render/mod.rs

+            for offset_x in -width..=width {
+                for offset_y in -width..=width {


The offsets could be in a circular pattern instead, so the outline doesn't get distorted.

crates/bevy_ui/src/render/mod.rs

UkoeHB · 2025-06-16T03:26:07Z

@TotalKrill I applied @ickshonpe's feedback: https://github.com/UkoeHB/bevy_cobweb_ui/blob/main/src/ui_bevy/ui_ext/text_rendering.rs

Note we need to batch anti-aliased glyphs separate from non-anti-aliased glyphs since they have different colors.

UkoeHB · 2025-06-16T03:29:23Z

Outline width should be clamped to a relatively low max value.

I don't like this, but you are probably right that it's necessary. With a UI scale of 6, on an M1 Mac (scale factor 2), with width 4, the lag penetrated my OS (laggy cursor even without the test app focused).

ickshonpe · 2025-06-16T10:41:37Z

Outline width should be clamped to a relatively low max value.

I don't like this, but you are probably right that it's necessary. With a UI scale of 6, on an M1 Mac (scale factor 2), with width 4, the lag penetrated my OS (laggy cursor even without the test app focused).

Yep it's very non-obvious that text outlines should be so expensive.

We could make width into an enum even. Have thin, medium, and thick variants or something?

crates/bevy_ui/src/render/mod.rs

Co-authored-by: UkoeHB <37489173+UkoeHB@users.noreply.github.com>

Co-authored-by: ickshonpe <david.curthoys@googlemail.com>

TotalKrill · 2025-06-16T16:10:03Z

We could make width into an enum even. Have thin, medium, and thick variants or something?

I would argue that we could go with a clearer enum of Px1, Px2, Px3. Mostly because Px4 is so expensive as to be almost unusable. It would also quite clearly communicate the limitations of this implementation, while still allowing us to have it.

I imagine that for non-changing outlined text, this would be very usable anyway.

Co-authored-by: ickshonpe <david.curthoys@googlemail.com>

TotalKrill · 2025-06-16T17:04:46Z

Let me know what You think of this impl, I dont particulary like it, but it is very hard to miss the potential performance cost of using this feature.

For my usecase to be able to read white text, on a mostly white background when viewed from certain angles, this will be very sufficient

UkoeHB

@TotalKrill can you check the updated reference implementation? In order to get antialiasing right, some changes are needed.

crates/bevy_ui/src/widget/text.rs

Co-authored-by: UkoeHB <37489173+UkoeHB@users.noreply.github.com>

TotalKrill · 2025-06-17T16:04:42Z

Updated with your reference implementation, just adjusted for the changes from GlobalTransform -> UiGlobalTransform

TotalKrill force-pushed the text_outline branch from 484bd77 to 0b70e16 Compare June 14, 2025 14:32

TotalKrill requested a review from ickshonpe June 14, 2025 14:35

Adding TextOutline component to add outlining by upstreaming implemen…

d527d93

…tation from bevy_cobweb_ui fixes: bevyengine#17076 fixes: bevyengine#7210 Co-authored-by: koe <ukoe@protonmail.com>

TotalKrill force-pushed the text_outline branch from 0b70e16 to d527d93 Compare June 14, 2025 15:06

Fix reflect and serialize feature

d362052

TotalKrill force-pushed the text_outline branch from cb6214e to d362052 Compare June 14, 2025 17:17

appease the markdown linter

4264108

alice-i-cecile added A-Rendering Drawing game state to the screen A-Text Rendering and layout for characters M-Needs-Release-Note Work that should be called out in the blog due to impact labels Jun 14, 2025

github-project-automation bot added this to Rendering Jun 14, 2025

alice-i-cecile added the S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged label Jun 14, 2025

alice-i-cecile requested a review from UkoeHB June 14, 2025 19:41

UkoeHB suggested changes Jun 14, 2025

View reviewed changes

crates/bevy_ui/src/widget/text.rs Outdated Show resolved Hide resolved

release-content/release-notes/text_outline.md Outdated Show resolved Hide resolved

release-content/release-notes/text_outline.md Outdated Show resolved Hide resolved

ickshonpe suggested changes Jun 14, 2025

View reviewed changes

TotalKrill and others added 4 commits June 15, 2025 21:42

Update crates/bevy_ui/src/widget/text.rs

2c27200

Co-authored-by: UkoeHB <37489173+UkoeHB@users.noreply.github.com>

Update release-content/release-notes/text_outline.md

d90af4c

Co-authored-by: UkoeHB <37489173+UkoeHB@users.noreply.github.com>

Update release-content/release-notes/text_outline.md

af182c7

Co-authored-by: UkoeHB <37489173+UkoeHB@users.noreply.github.com>

Iterate postionedglyphs in outer loop to batch the outline sprites fo…

6bd8c75

…r to only one batch per glyph

UkoeHB reviewed Jun 15, 2025

View reviewed changes

improve text outline performance

bc38c41

ickshonpe reviewed Jun 16, 2025

View reviewed changes

UkoeHB reviewed Jun 16, 2025

View reviewed changes

crates/bevy_ui/src/render/mod.rs Outdated Show resolved Hide resolved

ickshonpe reviewed Jun 16, 2025

View reviewed changes

crates/bevy_ui/src/render/mod.rs Show resolved Hide resolved

TotalKrill and others added 2 commits June 16, 2025 18:02

Update crates/bevy_ui/src/render/mod.rs

71ccce9

Co-authored-by: UkoeHB <37489173+UkoeHB@users.noreply.github.com>

Update crates/bevy_ui/src/render/mod.rs

acc5e6c

Co-authored-by: ickshonpe <david.curthoys@googlemail.com>

TotalKrill and others added 2 commits June 16, 2025 18:35

Update crates/bevy_ui/src/render/mod.rs

08b570e

Co-authored-by: ickshonpe <david.curthoys@googlemail.com>

Communicate the implications of the width clearly using an enum

2132f79

UkoeHB suggested changes Jun 16, 2025

View reviewed changes

crates/bevy_ui/src/widget/text.rs Outdated Show resolved Hide resolved

TotalKrill and others added 2 commits June 17, 2025 17:56

Update crates/bevy_ui/src/widget/text.rs

cfb2468

Co-authored-by: UkoeHB <37489173+UkoeHB@users.noreply.github.com>

Update reference implementation

f5e7938

remove unused

007517f

		for offset_x in -width..=width {
		for offset_y in -width..=width {

Uh oh!

Adding TextOutline component to add outlining by upstreaming implemen… #19639

Are you sure you want to change the base?

Adding TextOutline component to add outlining by upstreaming implemen… #19639

Uh oh!

Conversation

TotalKrill commented Jun 14, 2025

Uh oh!

hukasu commented Jun 14, 2025

Uh oh!

hukasu commented Jun 14, 2025

Uh oh!

alice-i-cecile commented Jun 14, 2025

Uh oh!

UkoeHB commented Jun 14, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ickshonpe Jun 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TotalKrill Jun 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

UkoeHB Jun 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ickshonpe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

UkoeHB commented Jun 16, 2025

Uh oh!

UkoeHB commented Jun 16, 2025

Uh oh!

ickshonpe commented Jun 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

TotalKrill commented Jun 16, 2025

Uh oh!

TotalKrill commented Jun 16, 2025

Uh oh!

UkoeHB left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

TotalKrill commented Jun 17, 2025

Uh oh!

Uh oh!

ickshonpe Jun 14, 2025 •

edited

Loading

TotalKrill Jun 15, 2025 •

edited

Loading

UkoeHB Jun 15, 2025 •

edited

Loading

ickshonpe commented Jun 16, 2025 •

edited

Loading