Closed Bug 1408566 Opened 2 years ago Closed 2 years ago

Scrolling causes graphics glitch with position sticky

Categories

(Core :: Graphics: Layers, defect, P1)

56 Branch
Unspecified
Windows 10
defect

Tracking

()

VERIFIED FIXED
mozilla58
Tracking Status
firefox-esr52 --- unaffected
firefox56 --- unaffected
firefox57 + verified
firefox58 --- verified

People

(Reporter: adamopenweb, Assigned: dvander)

References

()

Details

(Keywords: regression, Whiteboard: [webcompat][gfx-noted])

Attachments

(3 files)

Attached image glitch.png
Originally reported: https://webcompat.com/issues/12375

Reproduces in Windows 10 but not OSX.
Tested in Firefox 56-58.

STR:
1) Navigate to: https://mtlynch.io/human-code-reviews-1/
2) Scroll

Expected result:
No graphics glitch

Actual result:
Graphics glitching on left column

The sidebar is using position: sticky, if I change this to position: fixed the issue goes away.
Component: Graphics → Panning and Zooming
Flags: needinfo?(botond)
Whiteboard: [webcompat] → [webcompat][gfx-noted]
This got uplifted to 57.
Priority: P3 → P1
Actually, it does make sense for this to be AL related.
Component: Panning and Zooming → Graphics: Layers
Flags: needinfo?(botond)
Duplicate of this bug: 1408689
STR works for me.
Assignee: nobody → dvander
Status: NEW → ASSIGNED
Flags: needinfo?(dvander)
Looks like an invalidation bug with component-alpha surfaces.
Blocks: 1402737
Oh, not an invalidation bug - it's a coordinate space bug. We're copying the background pixels into the wrong region of the temporary surface, so some of the surface might not contain the correct background. And sometimes this results in it not being cleared, so we get pixels from the previous frame underneath.
Looks like the issue is bound to the Windows graphics rendering, doesn't occur with linux (arch/kde with xorg) and macOS 10.13.
Right, the advanced layers are currently Windows only.
Attached patch part 1, fix bugSplinter Review
This fixes the coordinate space bug, which is that the destination point was at (0, 0). It has to be the visible region relative to the texture's origin, I think.

I also made CopyTexture() bail out if the read or write region overflows. Otherwise, it can cause a TDR, which definitely happened to me when I tried a bad fix.
Attachment #8919139 - Flags: review?(matt.woodrow)
AL creates larger temporary surfaces than the old Compositor. The Compositor has an occlusion culling pass that right-sizes containers. AL can't do this as easily since it doesn't pass the occluded region or clip down to child containers. Instead it simply draws the whole thing, and caches the results more aggressively.

That seemed fine until bug 1402737, which required that we redraw the whole container every frame. Now we're just painting lots of ignored pixels. A quick fix is to just scissor all the batches to the visible region.
Attachment #8919141 - Flags: review?(matt.woodrow)
Duplicate of this bug: 1408669
Duplicate of this bug: 1408796
Attachment #8919139 - Flags: review?(matt.woodrow) → review+
Attachment #8919141 - Flags: review?(matt.woodrow) → review+
Pushed by danderson@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/740c4ed36aa1
Fix a coordinate space bug when composing some intermediate surfaces. (bug 1408566 part 1, r=mattwoodrow)
https://hg.mozilla.org/integration/mozilla-inbound/rev/ba80e255a606
Scissor the visible region for intermediate surfaces that require redraw. (bug 1408566 part 2, r=mattwoodrow)
https://hg.mozilla.org/mozilla-central/rev/740c4ed36aa1
https://hg.mozilla.org/mozilla-central/rev/ba80e255a606
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Hi Matt, Milan, should we uplift this to beta57?
Flags: needinfo?(milan)
Flags: needinfo?(matt.woodrow)
Comment on attachment 8919139 [details] [diff] [review]
part 1, fix bug

Yes.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1402737 
[User impact if declined]: Incorrect rendering when scrolling some antialiased text
[Is this code covered by automated tests?]:
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: STR are in comment #0
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: Not very
[Why is the change risky/not risky?]: bug 1402737 was low-risk with the caveat that it could break rendering in these scenarios. And in fact... it did. This patch fixes the coordinate calculation, and the biggest risk is that it does not fully fix the original bug.
[String changes made/needed]:
Attachment #8919139 - Flags: approval-mozilla-beta?
Flags: needinfo?(milan)
Flags: needinfo?(matt.woodrow)
Comment on attachment 8919139 [details] [diff] [review]
part 1, fix bug

Fix a pretty visible regression, taking it.
Attachment #8919139 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
Reproduced this issue on Windows 10 x64, using an affected Nightly build from 2017-10-13.

Not repro anymore on 57 Beta 13 (20171030163911) and latest Nightly 58.a01 (2017-10-30) with Windows 10 x64.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.