Fix use of `bounding_rect` to calculate the bounds of an item by eira-fransham · Pull Request #11252 · slint-ui/slint

eira-fransham · 2026-04-02T15:06:25Z

This fixes the issue reported in the issue above, and (to my understanding) should implement the correct behaviour. I believe that the issue is that by doing the bounding_rect calculations (which zero out width and height in some cases) as we go along, we were ignoring some rectangles that we should not be and therefore potentially ignoring some translations that are semantically important. By only zeroing them out at the end, we ensure that we take the origins of all item bounds into account.

eira-fransham · 2026-04-08T10:12:44Z

Ok so with the latest fixes this now appears to be correct without regressions. There seems to be an issue that this PR partially mitigates but does not solve, where FemtoVG appears to incorrectly render when there are multiple sibling layers. From testing, though, this does not seem to be a regression and we should open a new ticket for it (if one does not already exist).

Before

Skia

2026-04-08.11-42-17.mp4

FemtoVG

2026-04-08.11-42-57.mp4

After

Skia

2026-04-08.11-43-54.mp4

FemtoVG

2026-04-08.12-15-13.mp4

…n_bounding_rect_inner`

…e inner geometry rather than iteratively

…ot the bounding rectangle

ogoffart · 2026-04-08T16:10:36Z

        let window_adapter = self.window().window_adapter();
        let current_clip = self.get_current_clip();
        if let Some((layer_offset, layer_image)) = self.render_layer(item_rc, &|| {
-            // We don't need to include the size of the "layer" item itself, since it has no content.


imho this comment was correct and calling the function item_children_bounding_rect without the with should be done.

ogoffart · 2026-04-08T16:12:49Z

+                    item_rc,
+                    &window_adapter,
+                )
+                .intersection(&current_clip)


This will regress the original #10277 , the bug there is that if you close the door when the window is resized to be small, then resize the window to be bigger, then open the door, you have glishes.

… size

ogoffart

So I see the folowing changes in this patch:

we no longer do the union with item_rc.geometry() in the intersection call
The if item.as_ref().clips_children() section is greatly simplified by returning the full geometry instead of trying to recurse in some cases only.
We translate the item by the geometry's origin instead of the bounding_rect's origin

The change 3 shouldn't have much impact because most item return the same origin as their geometry right now.

The change 2. might have had a bug before?

The change 1, as i said, i don't understand why it fixes the problem. Could you elaborate?

ogoffart · 2026-04-09T10:02:01Z

            i_slint_core::properties::evaluate_no_tracking(|| {
                i_slint_core::item_rendering::item_children_bounding_rect(item_rc, &window_adapter)
-                    .intersection(&current_clip.union(&item_rc.geometry()))
+                    .intersection(&current_clip)


Why did you remove the .union(&item_rc.geometry()) . Is that fixing a problem, which one?

I thought this was what actually ended up fixing the issue but I was wrong, re-adding this didn't actually break it again. I'll revert this change since it's not necessary for the fix.

So yes, it looks like using item_geom.origin as the child transform instead of bounding.origin is the core fix. I definitely tried switching back and forth on that earlier and it didn't change, though, which I'm quite confused by. I think that the item_with_children_bounding_rect vs item_children_bounding_rect change was somehow covering up that fix when I was testing it earlier.

Then you can also revert all the changes in this file.

ogoffart · 2026-04-09T12:56:04Z

        if let Some((layer_origin, layer_image)) = self.render_layer(item_rc, &|| {
-            // We don't need to include the size of the "layer" item itself, since it has no content.
-            // But intersect with the union of the clip with the geometry to make sure we don't
-            // render insanely large surface.


I think you can put back this comment then.

Already done, sorry I hadn't pushed yet because I've been setting up my linux laptop

ogoffart · 2026-04-09T12:56:20Z

            i_slint_core::properties::evaluate_no_tracking(|| {
                i_slint_core::item_rendering::item_children_bounding_rect(item_rc, &window_adapter)
-                    .intersection(&current_clip.union(&item_rc.geometry()))
+                    .intersection(&current_clip)


Then you can also revert all the changes in this file.

ogoffart · 2026-04-09T12:57:39Z

Is it possible to make a screenshot test like the one i added in tests/screenshots/cases/layer/cache-rendering-hint-large.slint ? It is probably possible to reproduce the bug with the right elements.

eira-fransham · 2026-04-09T13:24:26Z

Is it possible to make a screenshot test like the one i added in tests/screenshots/cases/layer/cache-rendering-hint-large.slint ? It is probably possible to reproduce the bug with the right elements.

I'll have a go, I haven't made a screenshot test yet but I think this should be easy to reproduce

ogoffart · 2026-04-13T08:19:00Z

    renderer.restore_state();
 }

+fn item_with_children_bounding_rect_transformed(


nitpick:

The order of these function is a bit strange:

fn foo_helper1 pub fn foo fn foo_helper2

I'm thinking the item_with_children_bounding_rect_transformed could be moved after.

Yeah that makes sense to me. I usually put helper functions before the function they help, and I guess whoever wrote the other helper prefers to put helpers after. Will change now.

My preference is actually, if a function is only used as a helper in one other function, to put the definition inside the function (although I'll only nest that a maximum of once). In my opinion that's the best way to make the intentions of the function clear.

I used to prefer putting them before, But rust analyzer prefers putting them later when refactoring code into function, or generating helper function, and that's why i changed my habits, but i don't have strong opinions.

ogoffart · 2026-04-13T09:37:31Z

+// Software renderer uses i16 physical coordinates and has no layer cache.
+//ignore: software


There is nothing which can't be in a i16 in this testcase.
so i think you can keep the test active for the software renderer.

ogoffart · 2026-04-13T09:38:36Z

+                Text {
+                    text: "foo";
+                }
+            }


Unfortunately, the skia screenshots with Text don't work as the text rendering is not consistant accross platforms. Can you use a simple image (can use data: url now) or a rectangle?

I've had trouble recreating this bug with any element other than Text.

EDIT: Ok, with the other changes I made to the test-case, switching it back to Rectangle instead of Text will now still reproduce the bug

ogoffart · 2026-04-13T10:21:30Z

+    background: white;
+
+    Rectangle {
+        cache-rendering-hint: false;


Should that not be true?

ogoffart

Please squash and merge

ogoffart · 2026-04-13T18:31:27Z

merged so it can be in the nightly

eira-fransham force-pushed the 10277-text-image-buffer-overwrite branch from 7c182bc to 9118d77 Compare April 7, 2026 09:23

eira-fransham changed the title ~~WIP: Hack Skia and FemtoVG backends to force usage of the partial renderer~~ Partial revert of #10601 to fix incorrect rendering when not using partial renderer Apr 7, 2026

eira-fransham marked this pull request as ready for review April 7, 2026 09:26

eira-fransham requested a review from ogoffart April 7, 2026 09:26

eira-fransham force-pushed the 10277-text-image-buffer-overwrite branch from 24d0087 to d9cd5af Compare April 7, 2026 10:58

This comment was marked as outdated.

Sign in to view

eira-fransham changed the title ~~Partial revert of #10601 to fix incorrect rendering when not using partial renderer~~ Fix use of bounding_rect to calculate the bounds of an item Apr 7, 2026

ogoffart reviewed Apr 7, 2026

View reviewed changes

Comment thread internal/core/item_rendering.rs Outdated

Comment thread internal/core/item_rendering.rs Outdated

Comment thread internal/core/item_rendering.rs Outdated

eira-fransham requested a review from ogoffart April 8, 2026 07:39

ogoffart reviewed Apr 8, 2026

View reviewed changes

Comment thread internal/core/item_rendering.rs Outdated

Comment thread internal/core/item_rendering.rs Outdated

eira-fransham requested a review from ogoffart April 8, 2026 10:09

ogoffart reviewed Apr 8, 2026

View reviewed changes

Comment thread internal/renderers/skia/itemrenderer.rs

Comment thread internal/core/item_rendering.rs Outdated

Comment thread internal/renderers/femtovg/itemrenderer.rs Outdated

Comment thread internal/renderers/skia/itemrenderer.rs

eira-fransham requested a review from ogoffart April 8, 2026 12:36

eira-fransham added 13 commits April 8, 2026 16:12

Fix using bounding_rect instead of item_geometry in `item_childre…

c259ed8

…n_bounding_rect_inner`

Fix clippy/check warnings

5dc3f8b

Minor refactor for clarity

de03337

Re-add bounding_rect, but use it only after calculating the complet…

b872dc8

…e inner geometry rather than iteratively

Skip calculating child bounds when clips_children is true

10e7f13

Fix missing cast

bbfa4aa

WIP: Fix rendering layers

9fbcf15

Fix clippy/check warnings

ae827cf

Fix incorrectly calculated graphics texture origin

f68a988

Improve comments

af9c489

Respond to PR feedback

d6e9275

clips_children should mean that the bounds are the item geometry, n…

f7fee3a

…ot the bounding rectangle

Fix compilation on int-coord targets

c224554

eira-fransham force-pushed the 10277-text-image-buffer-overwrite branch from a88b6eb to c224554 Compare April 8, 2026 14:13

ogoffart reviewed Apr 8, 2026

View reviewed changes

Simplify implementation, ignore layer geometry when calculating layer…

74c0dc3

… size

eira-fransham requested a review from ogoffart April 9, 2026 09:57

ogoffart reviewed Apr 9, 2026

View reviewed changes

Revert unnecessary change

63cb22e

ogoffart reviewed Apr 9, 2026

View reviewed changes

Revert unnecessary changes

c509176

ogoffart modified the milestone: 1.16 Apr 9, 2026

Revert unnecessary changes

3738d2c

ogoffart reviewed Apr 13, 2026

View reviewed changes

Add regression test

f503825

eira-fransham requested a review from ogoffart April 13, 2026 09:31

[autofix.ci] apply automated fixes

9b61d40

ogoffart reviewed Apr 13, 2026

View reviewed changes

eira-fransham added 2 commits April 13, 2026 12:15

Move helper function to after its use

0a899b9

Make regression test cross-platform

bb0968f

ogoffart reviewed Apr 13, 2026

View reviewed changes

eira-fransham and others added 2 commits April 13, 2026 13:08

Fix rendering hint

edd7485

[autofix.ci] apply automated fixes

1f549de

ogoffart approved these changes Apr 13, 2026

View reviewed changes

ogoffart merged commit d5d450c into slint-ui:master Apr 13, 2026
89 of 91 checks passed

eira-fransham mentioned this pull request Apr 23, 2026

Text rendering issues with FemtoVG #11492

Closed

		// Software renderer uses i16 physical coordinates and has no layer cache.
		//ignore: software

Conversation

eira-fransham commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eira-fransham commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Before

Skia

FemtoVG

After

Skia

FemtoVG

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ogoffart left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eira-fransham Apr 9, 2026 • 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!

eira-fransham Apr 9, 2026 • 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!

ogoffart commented Apr 9, 2026

Uh oh!

eira-fransham commented Apr 9, 2026

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!

eira-fransham Apr 13, 2026 • 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!

ogoffart left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ogoffart commented Apr 13, 2026

Uh oh!

Reviewers

Assignees

eira-fransham commented Apr 2, 2026 •

edited

Loading

eira-fransham commented Apr 8, 2026 •

edited

Loading

eira-fransham Apr 9, 2026 •

edited

Loading

eira-fransham Apr 9, 2026 •

edited

Loading

eira-fransham Apr 13, 2026 •

edited

Loading