1747238 - Inline SVG is drawn incorrectly in a flex container

Status: NEW

(Core :: Graphics: WebRender, defect)

Description

[aykevanlaethem]

Attachment: svg-height.html

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:95.0) Gecko/20100101 Firefox/95.0

Steps to reproduce:

Load the attached file in Firefox, or open https://aykevl.nl/svg-height.html (same file).
If needed, trigger the bug by resizing the window or opening dev tools (which also resizes the website viewport).

Actual results:

The circle in the middle isn't fully drawn: bits may be chopped off or it might disappear entirely. It doesn't always reproduce though, a bit of window resizing might be necessary.

I can't seem to reproduce this on Android, only on Linux so far (didn't test other OSes).

Expected results:

The circle in the middle should be drawn normally.

Sidenote: this uses the same test case as https://bugs.chromium.org/p/chromium/issues/detail?id=1281085 but Chrome has a different bug. Apparently browsers seem to have problems with inline SVG and flexbox.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::Layout: Flexbox' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

It doesn't have to be a flex container, that just takes care of growing the svg area. Afaict on a block the same happens if I set width: 100% and height: 100%.

I'm a bit surprised this is tripping us off, it's a really simple test-case. I bisected and there was a behavior change here:

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0e0e9c22321ed9f284744cfeb3507710b2d5d606&tochange=26711f10f5f45471b5856fb5cef08948f0e5bc21

Where there are quite a few blobs changes. Bug 1568227 looks particularly suspicious. So tentatively moving to graphics as it doesn't really seem to be a layout issue (the circle is always correctly positioned). Jeff perhaps you know off-hand what might be going wrong?

Comment 3

[Jim Mathies [:jimm]]

Needs a severity

Comment 4

[Jeff Muizelaar [:jrmuizel]]

Nical, do you want to try debugging this?

Comment 5

[Bob Hood [:bhood]]

Nical pinged on Matrix.

Comment 6

[Nicolas Silva [:nical]]

Attachment: A slightly simpler test case

Jeff M and I came up with this test case, which doesn't have the flex stuff as pointed out by emilio.

There is a red circle that shows the bug, and a green one that does not. The difference is that the red circle uses transform: translate(50%, 50%) whereas the green one has cx="50%" cy="70%".
When reloading the page, the bug dissappears until the window is moved again. It's like there is a clip rect that is computed correctly when the page loads but not updated when the window is resized

It looks like it has something to do with SVG reflow/invalidation and its interaction with transforms (or maybe specifically transforms that are not animated but depend on % parameters that change with the size of the svg?). I don't know how all of that works.

In the painting code here: https://searchfox.org/mozilla-central/rev/cc98a15c7327d742d283cddddde712a8a3165006/gfx/layers/wr/WebRenderCommandBuilder.cpp#588
we compute the "visible rect" which defines bounds of the webrender primitive. It is the intersection of itself and "actual bounds".
these two rects behave differently when resizing the window:

• mVisibleRect's origin does not move when resizing the window
• but mActualBounds's origin moves

So when they are intersected after resizing the window, parts of the visible rect are clipped away incorrectly. I suspect that mActualBounds is correct, but that mVisibleRect's initial gets out of date when the svg is resized.

The Visible rect comes from here: https://searchfox.org/mozilla-central/rev/cc98a15c7327d742d283cddddde712a8a3165006/gfx/layers/wr/WebRenderCommandBuilder.cpp#1632
Similarly, layerBounds's origin moves when the SVG is resized but not groupBounds which we get by calling GetUntransformedBounds on the containing item.

It looks like GetUntransformedBounds retuns the item's mBuildingRect which is modified during displaylist building (see SetBuildingRect).

I'm not sure what coordinate space that rect is suppoed to be in (Jeff says "probably relative to closest containing item-ish"). However it appears to be updated (or not) differently depending on whether the displacement is done via a transform.

Also worth noting, chrome's behavior looks kind of buggy too. They don't have odd clipping issues but the position of the circle never updates once the page is loaded.

Emilio do you know what the correct behavior should be? Are we supposed to not update the transform if the size of the svg change (à la chrome)? Also do you know about this BuildingRect business and where we may be missing to update it?

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Emilio do you know what the correct behavior should be? Are we supposed to not update the transform if the size of the svg change (à la chrome)?

I'm pretty sure we're supposed to update the transform when the svg changes, and the correct behavior is the green circle's. Chrome's behavior looks pretty broken too (reloading shouldn't change rendering generally).

Also do you know about this BuildingRect business and where we may be missing to update it?

So the building rect is IIRC relative to the closest reference frame (AKA transformed frame). cx/cy and transforms are different because they affect SVG layout, but CSS transforms don't.

At a glance, this ought to invalidate paint for the relevant elements, maybe it doesn't for CSS transforms? We shouldn't need to reflow for those, but just repainting should do.

It looks like GetUntransformedBounds retuns the item's mBuildingRect which is modified during displaylist building (see SetBuildingRect).

Right... so this seems like an invalidation bug when the transform reference box changes. When this happens on HTML content, I suppose we rely on the relevant paint invalidation to just rebuild the display list, do you know what's different about blobs? Throwing an InvalidateFrame on the outer svg frame doesn't seem to help.

Comment 8

[Jeff Muizelaar [:jrmuizel]]

It looks like nsDisplayListBuilder::mVisibleRect is not properly taking into account the transform or otherwise gets confused.

Comment 9

[Nicolas Silva [:nical]]

do you know what's different about blobs?

I wonder if it has something to do with blobs using the building rect when computing their bounds while webrender display items don't or not for sizing/clipping at least. The blob is invalidated (it is repainted, and its bounds are recomputed alas incorrectly), but somehow the building rect is not updated or perhaps incorrectly.

I think that the regression range points to when we started using the building rect to clip the bounds.

Comment 10

[aykevanlaethem]

For some reason this bug now blocks 646044 but that seems like an entirely unrelated bug to me?

Chrome's behavior looks pretty broken too (reloading shouldn't change rendering generally).

FYI: the (unrelated) Chrome bug has now been fixed. Now I only need my workaround for Firefox, it is still present in Firefox 123.

Comment 11

[Jamie Nicol [:jnicol]]

I presume Bob accidentally changed the wrong bug