1532731 - www.office.com has blurry/missing content when scrolling down, in Fennec and Geckoview (with "transform-style: preserve-3d" and "perspective: 1px")

Status: RESOLVED FIXED

(Core :: Graphics, defect, P2)

Description

[Thomas Wisniewski [:twisniewski]]

Attachment: test.html

While scrolling down the initial page at https://www.office.com, Fennec and GeckoView both struggle to paint the content, resulting in the "fuzzy text" effect and other visual glitches for many seconds at a time on my S7 Edge.

I've created a reduced test-case which exhibits the same behavior, and attached it here. It seems to be caused by a combination of having multiple CSS scrollbars and certain transform/perspective rules.

I ran through mozregression, and the problem seems to still be present as far back as Fennec 52.

This was originally reported at https://webcompat.com/issues/27247

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Wow, it does seem really weird. More likely to be a Graphics / APZ / web-painting issue... I would guess we think something is clipped away or not visible but it is, or something of that sort.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Layout seems to work fine, you can select text and such, but tiles just disappear sometimes.

Comment 3

[Daniel Holbert [:dholbert]]

[qf-] for now, since this doesn't seem like a perf-related bug, but rather some sort of {painting|clipping|invalidation} glitch.

(Definitely worth fixing, but just doesn't fall under the qf umbrella)

Comment 4

[Jeff Muizelaar [:jrmuizel]]

Jamie, do you want to take a look at this?

Comment 5

[Jamie Nicol [:jnicol]]

Attachment: layers dump

Here's a layer dump of test.html.

Comment 6

[Jamie Nicol [:jnicol]]

The problem is that the critical display port does not move as you scroll down the page, so at a certain point the on-screen content is outside of the critical display port and we paint it with low-precision tiles.

Here [1] is where we should apply the transform to the critical display port as we scroll. But for the PaintedLayer 0x7fa9a4811000, displayportAncestor is its grandparent 0x7fa9a722cc00. The transform to that layer is identity. The great-grandparent 0x7fa9a722c800 is the layer whose transform contains the scroll position.

Matt, should the scroll offset transform be on the grandparent rather than the great-grandparent? Is this a known quirk of perspective/3d transforms or a bug?

Kats suggested that ignoring layers with combines3DTransformWithAncestor when calculating displayportAncestor could work, if the transform is actually on the expected layer. That indeed seems to fix the problem, but it'd be good to know why the transform is not where we expect it.

[1] https://searchfox.org/mozilla-central/source/gfx/layers/client/ClientTiledPaintedLayer.cpp#186

Comment 7

[Matt Woodrow (:mattwoodrow)]

This is indeed a known quirk of how perspective works.

I don't think we want to change the displayportAncestor to the grandparent, since that has different metrics (for scrollId 2 instead of 3).

I think the change that we need is to make GetTransformToAncestorsParentLayer handle combines3DTransformWithAncestor, so that it first walks up the parent chain to the first parent that isn't combined3DTransformWithAncestor, and then computes a transform relative to the parent of that.

Adding ni? botond too, to make sure this matches what the APZ side does.

Comment 8

[Botond Ballo [:botond]]

I've reviewed the APZ changes we made related to perspective, and all the APZ logic is in terms of Layer::TransformIsPerspective().

Could you remind me what is the relationship between TransformIsPerspective() and Combines3DTransformWithAncestor()?

Comment 9

[Matt Woodrow (:mattwoodrow)]

Combines3DTransformWithAncestors means that we need to include the transform from our parent layer before trying to do 2D coordinate conversions.

This can be set on the (single!) child layer of a perspective layer (returns true for TransformIsPerspective()), and also for layers where the parent establishes or extends a preserve-3d context.

The case we have here, where the layer has both scroll metadata, and Combines3DTransformWithAncestor() is exclusive to perspective though, since a value for overflow != visible is specified as disabling preserve-3d.

Given that, I think it would be equivalent to use the current logic to find the parent of the layer with metadata, and then if that layer has TransformIsPerspective(), jump up one level further. My previous suggestion of using Combines3DTransformWithAncestor is the more idiomatic way of doing it though.

--- Discussion of a side-issue that doesn't directly affect this bug follows. ---

Botond also raised the question (on irc) of why the scroll offset is on the perspective layer, despite it not being the layer with scroll metadata.

The transforms we need to apply are (for this case where the transformed frame scrolls, but the perspective is on the non-scrolling outer scroll frame):

css-transform * offset-from-transform-frame-to-perspective-frame * perspective-transform * offset-from-perspective-frame-to-reference-frame

Gecko currently constructs the transform layer with just the first term of that sequence, and the perspective item with the other three. That makes it such that the scroll offset (included in the offset between transform and perspective frame) is part of the transform on the perspective layer.

We could just as easily include the first two terms on the transform layer (and it would probably make more sense to do so), at which point the scroll offset is now on the same frame as the transform. Note that that wouldn't affect this bug, and we still need to combine the perspective transform to get correct coordinate conversion.

APZ will be apply any async scrolling to the layer with scroll metadata (the transform layer), so we currently have the weird behaviour where the initial scroll offset is part of the perspective layer (pre-translated), and the async scroll offset is part of the transform layer (post-translated). Again, this doesn't affect behaviour in any way, since we always need to multiply the two together before using them, it's just a bit confusing.

Comment 10

[Botond Ballo [:botond]]

Thanks for the details!

One further question: in this case, the perspective layer has scroll metadata for an enclosing scroll frame (scrollid=2). I understand that on this page, the enclosing scroll frame doesn't actually scroll, but on a different page could it have a scroll transform on its own, and if so, would we want our GetTransformToAncestorsParentLayer() to include that scroll transform?

Comment 11

[Matt Woodrow (:mattwoodrow)]

The pair of Layers (transform and perspective) are effectively one Layer (which is how it was before we added perspective scrolling support to APZ), and there are two Metrics on this 'single' Layer.

That's something that can happen without perspective, so the logic should be the same I believe.

Comment 12

[Botond Ballo [:botond]]

Ok, thanks, that helps reason about it better.

The suggestion in comment 7 sounds good to me.

Comment 13

[Jamie Nicol [:jnicol]]

Attachment: Bug 1532731 - Handle 3D transformed ancestors when calculating display port transform. r?mattwoodrow

ClientTiledPaintedLayer::GetTransformToAncestorsParentLayer calculates
the transform from the layer to its display port ancestor's
parent. This transform is then applied to the calculated display
port.

However, if the display port ancestor participates in a preserve-3d
context then the scroll offset will not be included in the that
layer's transform, it will instead be on the root layer of the
preserve-3d context. This was causing the critical display port to
remain still as the contents of a perspective transform were scrolled,
resulting in content being permanently painted in low-precision as the
page was scrolled down.

Instead, if the display port ancestor participates in a 3d context, we
must find the root of that 3d context then calculate the transform to
that layer's parent.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/68ea0add020c
Handle 3D transformed ancestors when calculating display port transform. r=mattwoodrow

Comment 15

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/68ea0add020c

Comment 17

[Ciprian Ciocan [:ciprian]]

Retested https://github.com/webcompat/web-bugs/issues/27247 on Firefox Nightly 68.0a1(2019 -05-17) and the issue is not reproducible anymore.