Bug 227285 – Pixel snapping is incorrect for async scrolled overflow layers

• Component: Compositing
• Status: NEW

[Chris Lord]

I'm not sure if this is a side-effect of fractional coordinates, a bug specifically in the WPE(/GTK) layer rendering or a bug in layout/rendering, but there appear to be 1-pixel differences in clipping in some situations, where the clip rectangle seems too tall. I expect this bug won't be WPE-specific or CSS grid-specific, but I'm labelling this bug as such until I have confirmation.

This is causing imported/w3c/web-platform-tests/css/css-grid/grid-model/grid-container-scrollbar-vertical-lr-001.html and at least the rl variant of the same test to fail on WPE (which runs these tests with async overflow scrolling enabled).

[Chris Lord]

I'm more convinced there's a legitimate bug here (maybe just a rounding error?), but I've not found it yet... If you force mock scrollbars and load this test in minibrowser with async overflow scrolling enabled, it's clear to me that behaviour isn't quite right with respect to positioning.

I'm examining positions and sizes at the moment, but I've not found any odd fractional sizes that might be causing this yet.

[Chris Lord]

I have a patch for this now and it is indeed a general problem (with an associated FIXME comment), but I'd like to get bug 227061 resolved first.

[Chris Lord]

Small update, it looks like fixing this bug fixes all the remaining failures in bug 224596 :)

[Simon Fraser (smfr)]

Can we do this one first?

[Chris Lord]

(In reply to Simon Fraser (smfr) from comment #4)
> Can we do this one first?

We can, just it's two awkward rebases for me :) I'll do that tomorrow assuming there's no movement on bug 227061 though.

[Radar WebKit Bug Importer]

<rdar://problem/79957479>

[Chris Lord]

Created attachment 432579 [details]
Patch

[Chris Lord]

(In reply to Chris Lord from comment #3)
> Small update, it looks like fixing this bug fixes all the remaining failures
> in bug 224596 :)

Whoops, so when I ran tests, I was using the wrong platform - this only fixes 2 of those failures, not all of them.

On the plus side, I was also wrong about the rebases being awkward, they were actually trivial.

[Chris Lord]

Comment on attachment 432579 [details]
Patch

Not quite correct yet.

[Chris Lord]

Created attachment 432679 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 432679 [details]
Patch

With async overflow scrolling disabled, we render scrollbars in the wrong place too (I think because of integral snapping of Widget?) so I'm not sure how we expect scrollbars to render correctly.

[Simon Fraser (smfr)]

Created attachment 432715 [details]
Subpixel scroller test

[Simon Fraser (smfr)]

Created attachment 432725 [details]
Test case

[Simon Fraser (smfr)]

Tested with the patch, things don't seem right. See test and screenshot.

[Simon Fraser (smfr)]

Created attachment 432726 [details]
Screenshot

[Chris Lord]

(In reply to Simon Fraser (smfr) from comment #14)
> Tested with the patch, things don't seem right. See test and screenshot.

This displays correctly for me, but I'm guessing this is affected by a hidpi screen - thanks for this, I'll figure something out.

[Chris Lord]

(In reply to Simon Fraser (smfr) from comment #14)
> Tested with the patch, things don't seem right. See test and screenshot.

Using a device scale factor of 1.5 I reproduce this issue, but it seems to happen prior to this patch too. Is the issue that you expect this patch to also fix this issue?

[Chris Lord]

(In reply to Chris Lord from comment #17)
> (In reply to Simon Fraser (smfr) from comment #14)
> > Tested with the patch, things don't seem right. See test and screenshot.
> 
> Using a device scale factor of 1.5 I reproduce this issue, but it seems to
> happen prior to this patch too. Is the issue that you expect this patch to
> also fix this issue?

(just an extra note; this patch only touches scrolled contents rendering, it doesn't touch scrollbar rendering - so this behaviour is expected to me)