Closed Bug 1667124 Opened 2 years ago Closed 2 years ago

Pinch-zooming can cause elements to be transiently or permanently mispositioned (macOS CA layer problem?)

Categories

(Core :: Graphics: WebRender, defect)

defect

Tracking

()

RESOLVED FIXED
83 Branch
Tracking Status
firefox83 --- fixed

People

(Reporter: kats, Assigned: gw)

References

Details

Attachments

(3 files)

On a local debug build, WR enabled, desktop zoom enabled (both are on by default), go to https://staktrace.github.io/moz-pages/griddiv.html and pinch zoom in and out. While zooming bits of text get mispositioned or misrendered. It looks pretty bad.

While doing this I also ran into an assertion:

Assertion failure: mFrontSurface->mInvalidRegion.Intersect(mDisplayRect).IsEmpty() (Parts of the display rect are invalid! This shouldn't happen.), at /Users/kats/zspace/gecko-cleanup/gfx/layers/NativeLayerCA.mm:776
#01: mozilla::wr::RenderCompositorNative::UnbindNativeLayer()[/Users/kats/zspace/gecko-cleanup/obj-host-debugopt/dist/NightlyDebug.app/Contents/MacOS/XUL +0x17aad7c]
#02: webrender::renderer::Renderer::draw_frame::h8c74408e66db2d8d[/Users/kats/zspace/gecko-cleanup/obj-host-debugopt/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7cdd314]

which makes me think this is related to the macOS compositor stuff in WR.

Summary: Pinch-zooming can cause elements to be transiently or permanently mispositioned → Pinch-zooming can cause elements to be transiently or permanently mispositioned (macOS CA layer problem?)
Attached video Video of badness

The red text above and below should always be the same size. It is not. Also you can clearly see things shifting around during zoom

Setting gfx.webrender.compositor to false seems to resolve the issues, it certainly seems related to the CA layer thing. Markus, thoughts?

Flags: needinfo?(mstange.moz)

The assertion means that WR called RenderCompositorNativeOGL::Bind with an aValidRect argument that is too large or an aDirtyRect argument that is too small. For the TileId that is hitting the assertion, if you log those two rects for all calls to Bind with that TileId, you should be able to see what's wrong. Two potential causes would be: 1. calling Bind for the first time with aDirtyRect smaller than aValidRect, or 2. calling Bind again with an enlarged aValidRect without including the added area in aDirtyRect.

Flags: needinfo?(mstange.moz)

Note that I only hit the assertion once. I don't think I can trigger it reliably.

Severity: -- → S3

I can try it reliably with my patch in bug 1642308 -- see https://treeherder.mozilla.org/#/jobs?repo=try&revision=f3cdedaa47975518765bcce06eb61b6395b635f8&selectedTaskRun=ax6_sCScQm-FjSOCmNmowg.0

Note that we appear to be running these tests with WR enabled, and the envvar isn't set (or was somehow broken).

Markus wrote a patch that emits the CA layer tree in HTML form so it can be inspecting in a browser. Here is one such capture using the STR in comment 0: https://mozilla.staktrace.com/bug/1667124.html

The red text that says it's below the div is not where it's supposed to be. It looks like the tile in question is mostly transparent, but with a few bits of opaque stuff - the text, the back button, and the tab from the tab bar. The transform is correctly isolating the text piece, but the position on the enclosing div/layer is not what it should be.

Glenn, are you able to reproduce this problem? (Steps are in comment 0). I think the assertion is a red herring, as I haven't seen the assertion again since I reported this bug, but I can easily reproduce the visual problem.

Flags: needinfo?(gwatson)

I can reproduce the problem, and confirm it only occurs with WR + CA compositor mode enabled (WR + normal picture caching works fine). I'll take a look today. Do you happen to know if the same bug occurs on Windows + DC?

Flags: needinfo?(gwatson)

I don't know, unfortunately. The Windows device on which I can do pinch-zooming is an arm64 laptop and WR doesn't run there.

I was able to reproduce this on a Win10 desktop machine with pinch zooming. I think the problem relates to the picture cache code not correctly invalidating a slice that is attached to a zoom spatial node, when the scale changes. Working on a test and fix for this now.

Detect when the scale of the root spatial node of a picture cache
changes, and invalidate tiles when that occurs.

Assignee: nobody → gwatson
Status: NEW → ASSIGNED
Keywords: leave-open
Keywords: leave-open
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/02bb35f3059d
Fix invalidation of picture cache tiles attached to zoom nodes. r=aosmond
https://hg.mozilla.org/integration/autoland/rev/74b10613b8f0
Add a reftest that captures improper picture caching behaviour, in at least some of the cases. r=gw

Backed out 2 changesets (bug 1667124) for reftest failures.

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&selectedTaskRun=e3ifEN2JR5GSudd8pDBkBg.1&searchStr=reftest&fromchange=83dbc65c2b8fe9fc89b762a4686ec7cc694d4b2c&tochange=6319812430c53bcbc154934dc364ef7ad917eafb

Backout link: https://hg.mozilla.org/integration/autoland/rev/6319812430c53bcbc154934dc364ef7ad917eafb

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=318123613&repo=autoland&lineNumber=6270

[task 2020-10-09T01:58:57.547Z] 01:58:57     INFO -  REFTEST TEST-START | gfx/tests/reftest/picture-caching-on-async-zoom.html == gfx/tests/reftest/picture-caching-on-async-zoom.html?ref
[task 2020-10-09T01:58:57.548Z] 01:58:57     INFO -  REFTEST TEST-LOAD | http://10.0.2.2:8854/tests/gfx/tests/reftest/picture-caching-on-async-zoom.html | 825 / 7101 (11%)
[task 2020-10-09T01:58:57.549Z] 01:58:57     INFO -  REFTEST TEST-LOAD | http://10.0.2.2:8854/tests/gfx/tests/reftest/picture-caching-on-async-zoom.html?ref | 825 / 7101 (11%)
[task 2020-10-09T01:58:57.552Z] 01:58:57  WARNING -  REFTEST TEST-UNEXPECTED-FAIL | gfx/tests/reftest/picture-caching-on-async-zoom.html == gfx/tests/reftest/picture-caching-on-async-zoom.html?ref | image comparison, max difference: 255, number of differing pixels: 61934
[task 2020-10-09T01:58:57.569Z] 01:58:57     INFO -  REFTEST   IMAGE 1 (TEST): data:image/png;base64,iVBORw0KGgoAAAANSUhEU...
[task 2020-10-09T01:58:57.580Z] 01:58:57     INFO -  REFTEST   IMAGE 2 (REFERENCE): data:image/png;base64,iVBORw0KGgoAAAANSU...
[task 2020-10-09T01:58:57.580Z] 01:58:57     INFO -  REFTEST INFO | Saved log: START http://10.0.2.2:8854/tests/gfx/tests/reftest/picture-caching-on-async-zoom.html
[task 2020-10-09T01:58:57.580Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] OnDocumentLoad triggering WaitForTestEnd
[task 2020-10-09T01:58:57.580Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] WaitForTestEnd: Adding listeners
[task 2020-10-09T01:58:57.580Z] 01:58:57     INFO -  REFTEST INFO | Saved log: Initializing canvas snapshot
[task 2020-10-09T01:58:57.580Z] 01:58:57     INFO -  REFTEST INFO | Saved log: DoDrawWindow 0,0,800,1000
[task 2020-10-09T01:58:57.580Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] MakeProgress
[task 2020-10-09T01:58:57.580Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] AttrModifiedListener fired
[task 2020-10-09T01:58:57.580Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] AttrModifiedListener fired
[task 2020-10-09T01:58:57.580Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] AfterPaintListener in http://10.0.2.2:8854/tests/gfx/tests/reftest/picture-caching-on-async-zoom.html
[task 2020-10-09T01:58:57.580Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] MakeProgress: STATE_WAITING_TO_FIRE_INVALIDATE_EVENT
[task 2020-10-09T01:58:57.580Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] MakeProgress: waiting for MozAfterPaint
[task 2020-10-09T01:58:57.580Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] MakeProgress: waiting for updateCanvasPending
[task 2020-10-09T01:58:57.581Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] HandlePendingTasksAfterMakeProgress waiting for a MozAfterPaint
[task 2020-10-09T01:58:57.581Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] AfterPaintListener in http://10.0.2.2:8854/tests/gfx/tests/reftest/picture-caching-on-async-zoom.html
[task 2020-10-09T01:58:57.581Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] HandlePendingTasksAfterMakeProgress updating canvas
[task 2020-10-09T01:58:57.581Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] Webrender enabled, sending update whole canvas for invalidation
[task 2020-10-09T01:58:57.581Z] 01:58:57     INFO -  REFTEST INFO | Saved log: Updating entire canvas for invalidation
[task 2020-10-09T01:58:57.581Z] 01:58:57     INFO -  REFTEST INFO | Saved log: DoDrawWindow 0,0,800,1000
[task 2020-10-09T01:58:57.581Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] MakeProgress
[task 2020-10-09T01:58:57.581Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] MakeProgress: STATE_WAITING_TO_FIRE_INVALIDATE_EVENT
[task 2020-10-09T01:58:57.581Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] MakeProgress: dispatching MozReftestInvalidate
[task 2020-10-09T01:58:57.581Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] MakeProgress
[task 2020-10-09T01:58:57.581Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] MakeProgress: STATE_WAITING_FOR_REFTEST_WAIT_REMOVAL
[task 2020-10-09T01:58:57.582Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] MakeProgress
[task 2020-10-09T01:58:57.582Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] MakeProgress: STATE_WAITING_FOR_SPELL_CHECKS
[task 2020-10-09T01:58:57.582Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] MakeProgress: STATE_WAITING_FOR_APZ_FLUSH
[task 2020-10-09T01:58:57.582Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] MakeProgress: done requesting APZ flush
[task 2020-10-09T01:58:57.582Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] MakeProgress: apz-repaints-flushed fired
[task 2020-10-09T01:58:57.582Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] MakeProgress
[task 2020-10-09T01:58:57.582Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] MakeProgress: STATE_WAITING_TO_FINISH
[task 2020-10-09T01:58:57.582Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] MakeProgress: waiting for MozAfterPaint
[task 2020-10-09T01:58:57.582Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] HandlePendingTasksAfterMakeProgress waiting for a MozAfterPaint
[task 2020-10-09T01:58:57.582Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] AfterPaintListener in http://10.0.2.2:8854/tests/gfx/tests/reftest/picture-caching-on-async-zoom.html
[task 2020-10-09T01:58:57.582Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] HandlePendingTasksAfterMakeProgress updating canvas
[task 2020-10-09T01:58:57.583Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] Webrender enabled, sending update whole canvas for invalidation
[task 2020-10-09T01:58:57.583Z] 01:58:57     INFO -  REFTEST INFO | Saved log: Updating entire canvas for invalidation
[task 2020-10-09T01:58:57.583Z] 01:58:57     INFO -  REFTEST INFO | Saved log: DoDrawWindow 0,0,800,1000
[task 2020-10-09T01:58:57.583Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] MakeProgress
[task 2020-10-09T01:58:57.583Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] MakeProgress: STATE_WAITING_TO_FINISH
[task 2020-10-09T01:58:57.583Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] MakeProgress: Completed
[task 2020-10-09T01:58:57.583Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] RecordResult fired
[task 2020-10-09T01:58:57.583Z] 01:58:57     INFO -  REFTEST INFO | Saved log: RecordResult fired
[task 2020-10-09T01:58:57.583Z] 01:58:57     INFO -  REFTEST INFO | Saved log: START http://10.0.2.2:8854/tests/gfx/tests/reftest/picture-caching-on-async-zoom.html?ref
[task 2020-10-09T01:58:57.583Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] OnDocumentLoad triggering AfterOnLoadScripts
[task 2020-10-09T01:58:57.583Z] 01:58:57     INFO -  REFTEST INFO | Saved log: Initializing canvas snapshot
[task 2020-10-09T01:58:57.583Z] 01:58:57     INFO -  REFTEST INFO | Saved log: DoDrawWindow 0,0,800,1000
[task 2020-10-09T01:58:57.583Z] 01:58:57     INFO -  REFTEST INFO | Saved log: [CONTENT] RecordResult fired
[task 2020-10-09T01:58:57.583Z] 01:58:57     INFO -  REFTEST INFO | Saved log: RecordResult fired
[task 2020-10-09T01:58:57.583Z] 01:58:57     INFO -  REFTEST TEST-END | gfx/tests/reftest/picture-caching-on-async-zoom.html == gfx/tests/reftest/picture-caching-on-async-zoom.html?ref
Flags: needinfo?(gwatson)

Oh, Android. I can take care of this.

Flags: needinfo?(gwatson)

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #15)

Oh, Android. I can take care of this.

Part of the problem is that apz.allow_zooming is not true by default on Android reftests, so I flipped that on. But now I get a failure that's the reverse - the test image is zoomed while the reference is not. Which means that the CSS transform is not taking effect.. whereas it was in the version of the test that bounced. Not sure what's going on there, but for now I'll skip the test on Android and reland.

Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/654d8a949539
Fix invalidation of picture cache tiles attached to zoom nodes. r=aosmond
https://hg.mozilla.org/integration/autoland/rev/5c2816e0436d
Add a reftest that captures improper picture caching behaviour, in at least some of the cases. r=gw
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 83 Branch
You need to log in before you can comment on or make changes to this bug.