Picture caching doesn't work well when zoomed in on android
Categories
(Core :: Graphics: WebRender, defect, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox73 | --- | verified |
People
(Reporter: jnicol, Assigned: ktaeleman)
References
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
STR:
- enable webrender (
gfx.webrender.debug.picture-caching
) and picture caching debug overlay (gfx.webrender.debug.picture-caching
) - open any page in geckoview_example app (eg http://mozilla.org)
- scroll around - most tiles should remain green, and the valid-rects remain full tile sized - picture caching is working well.
- zoom in ever so slightly
- scroll around
Expected Results: Most tiles should remain green as you scroll around
Actual results: tiles turn red and amber whilst scrolling, only becoming green again once you stop. the invalidation rects become very small.
Comment 1•5 years ago
|
||
I wonder if this might be due to some floating point inaccuracies during zoom that are resulting in things being invalidated when they shouldn't be?
The main entry point to the dependency comparisons is the update_content_validity
method in picture.rs
. This eventually gets to the is_prim_same
method.
Could you add some logging into that function and see (a) what is causing the invalidation (e.g. clip rects, image keys etc) and (b) what the value differences are in the comparisons that fail?
That might help us identify if the guess above is right, or if something else is going on.
Reporter | ||
Comment 2•5 years ago
|
||
It fails because either origin.y
or origin.x
doesn't match, depending on whether I scroll vertically or horizontally.
The values can be quite different, up to 1.0 sometimes. Usually 0.1 to 0.6ish. Is that a bit high to be due to floating point inaccuracies?
Comment 3•5 years ago
|
||
Yea that does seem too high for FP inaccuracies, and also too high for us to quantize / round to (since it would break other sites).
We probably need to trace what the cause is of this. Is the difference coming directly from the zoom transform matrix when mapping the primitive origins? Some logging of the transform matrices in the un-zoomed and zoomed case might help identify what's occurring here?
Comment 4•5 years ago
|
||
Poking this bug along, Jamie do you have answers to Glenn's questions? :)
Assignee | ||
Comment 5•5 years ago
|
||
Been looking at this more today.
Currently my theory is 2 fold:
- When updating the scrollposition, the rounding is done after the zoom has been taken out of the translation causing the scrolling to be not device aligned: https://searchfox.org/mozilla-central/rev/690e903ef689a4eca335b96bd903580394864a1c/gfx/layers/apz/src/APZCTreeManager.cpp#729
- Snapping code doesn't take zooming into account causing the scrollposition to be not device aligned when zoomed:
https://searchfox.org/mozilla-central/rev/690e903ef689a4eca335b96bd903580394864a1c/gfx/wr/webrender/src/spatial_node.rs#116
Rounding before dividing by zoom and removing the snap_offset function seems to make tile cache invalidation to stop happening on every scroll.
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 6•5 years ago
|
||
- Fix crash due to shift left causing overflow (debug only)
- Remove rounding of scrolling offsets and snap to view space instead of
world space
Updated•5 years ago
|
Pushed by ktaeleman@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/6493da33ecac Fix snapping and rounding errors causing picture caching invalidation when zoomed in. r=aosmond,botond
Comment 8•5 years ago
|
||
Backed out changeset 6493da33ecac (bug 1589669) for failing at test_bug1277814.html on a CLOSED TREE.
Backout link: https://hg.mozilla.org/integration/autoland/rev/691a752d9319c85e73c4e62d577e9ba95f656407
Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=6493da33ecacf937e915726cb71f9f1ec3f68da1&selectedJob=282414366
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=282414366&repo=autoland&lineNumber=17524
Log snippet:
[task 2019-12-23T17:59:42.762Z] 17:59:42 INFO - GECKO(3034) | Dispatching 0 onpaint listeners
[task 2019-12-23T17:59:51.589Z] 17:59:51 INFO - TEST-INFO | started process screentopng
[task 2019-12-23T17:59:51.866Z] 17:59:51 INFO - TEST-INFO | screentopng: exit 0
[task 2019-12-23T17:59:51.867Z] 17:59:51 INFO - TEST-UNEXPECTED-FAIL | gfx/layers/apz/test/mochitest/test_bug1277814.html | Test timed out.
[task 2019-12-23T17:59:51.867Z] 17:59:51 INFO - SimpleTest.ok@SimpleTest/SimpleTest.js:277:18
[task 2019-12-23T17:59:51.867Z] 17:59:51 INFO - reportError@SimpleTest/TestRunner.js:121:22
[task 2019-12-23T17:59:51.868Z] 17:59:51 INFO - TestRunner._checkForHangs@SimpleTest/TestRunner.js:142:18
[task 2019-12-23T17:59:51.868Z] 17:59:51 INFO - setTimeout handlerTestRunner._checkForHangs@SimpleTest/TestRunner.js:170:15
[task 2019-12-23T17:59:51.868Z] 17:59:51 INFO - setTimeout handlerTestRunner._checkForHangs@SimpleTest/TestRunner.js:170:15
[task 2019-12-23T17:59:51.868Z] 17:59:51 INFO - setTimeout handlerTestRunner._checkForHangs@SimpleTest/TestRunner.js:170:15
[task 2019-12-23T17:59:51.868Z] 17:59:51 INFO - setTimeout handlerTestRunner._checkForHangs@SimpleTest/TestRunner.js:170:15
[task 2019-12-23T17:59:51.868Z] 17:59:51 INFO - setTimeout handlerTestRunner._checkForHangs@SimpleTest/TestRunner.js:170:15
[task 2019-12-23T17:59:51.868Z] 17:59:51 INFO - setTimeout handlerTestRunner._checkForHangs@SimpleTest/TestRunner.js:170:15
[task 2019-12-23T17:59:51.868Z] 17:59:51 INFO - setTimeout handlerTestRunner._checkForHangs@SimpleTest/TestRunner.js:170:15
[task 2019-12-23T17:59:51.868Z] 17:59:51 INFO - setTimeout handlerTestRunner._checkForHangs@SimpleTest/TestRunner.js:170:15
[task 2019-12-23T17:59:51.868Z] 17:59:51 INFO - setTimeout handlerTestRunner._checkForHangs@SimpleTest/TestRunner.js:170:15
[task 2019-12-23T17:59:51.868Z] 17:59:51 INFO - setTimeout handlerTestRunner._checkForHangs@SimpleTest/TestRunner.js:170:15
[task 2019-12-23T17:59:51.868Z] 17:59:51 INFO - setTimeout handlerTestRunner._checkForHangs@SimpleTest/TestRunner.js:170:15
[task 2019-12-23T17:59:51.868Z] 17:59:51 INFO - TestRunner.runTests/<@SimpleTest/TestRunner.js:388:20
[task 2019-12-23T17:59:51.868Z] 17:59:51 INFO - promise callbackTestRunner.runTests@SimpleTest/TestRunner.js:375:50
[task 2019-12-23T17:59:51.868Z] 17:59:51 INFO - RunSet.runtests@SimpleTest/setup.js:201:14
[task 2019-12-23T17:59:51.868Z] 17:59:51 INFO - RunSet.runall@SimpleTest/setup.js:180:12
[task 2019-12-23T17:59:51.868Z] 17:59:51 INFO - hookupTests@SimpleTest/setup.js:273:12
[task 2019-12-23T17:59:51.868Z] 17:59:51 INFO - parseTestManifest@http://mochi.test:8888/manifestLibrary.js:48:13
[task 2019-12-23T17:59:51.868Z] 17:59:51 INFO - getTestManifest/req.onload@http://mochi.test:8888/manifestLibrary.js:61:28
[task 2019-12-23T17:59:51.868Z] 17:59:51 INFO - EventHandlerNonNullgetTestManifest@http://mochi.test:8888/manifestLibrary.js:57:3
[task 2019-12-23T17:59:51.868Z] 17:59:51 INFO - hookup@SimpleTest/setup.js:253:20
[task 2019-12-23T17:59:51.868Z] 17:59:51 INFO - EventHandlerNonNull@http://mochi.test:8888/tests?autorun=1&closeWhenDone=1&consoleLevel=INFO&manifestFile=tests.json&dumpOutputDirectory=%2Ftmp&cleanupCrashes=true:11:1
[task 2019-12-23T17:59:52.589Z] 17:59:52 INFO - GECKO(3034) | MEMORY STAT | vsize 2548MB | residentFast 153MB | heapAllocated 20MB
[task 2019-12-23T17:59:52.597Z] 17:59:52 INFO - TEST-UNEXPECTED-ERROR | gfx/layers/apz/test/mochitest/test_bug1277814.html | unexpected-crash-dump-found - This test left crash dumps behind, but we weren't expecting it to!
[task 2019-12-23T17:59:52.598Z] 17:59:52 INFO - Found unexpected crash dump file /tmp/tmp8Y6kLA.mozrunner/minidumps/334efe77-c8ac-6b55-b00b-b27155d2db59.dmp.
[task 2019-12-23T17:59:52.599Z] 17:59:52 INFO - Found unexpected crash dump file /tmp/tmp8Y6kLA.mozrunner/minidumps/334efe77-c8ac-6b55-b00b-b27155d2db59.extra.
Pushed by ktaeleman@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/03fe3d76bd48 Fix snapping and rounding errors causing picture caching invalidation when zoomed in. r=aosmond,botond
Comment 10•5 years ago
|
||
bugherder |
Assignee | ||
Updated•5 years ago
|
Updated•5 years ago
|
Comment 11•5 years ago
|
||
Hi, environment on which i tried to reproduce -> build ID: 20191024144735 with GV:72.0a1 from: https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=724c75f9d98fa441703a4e36f294672859e138d7&searchStr=android&selectedJob=272793812 .
I did encounter, that while scrolling all the tiles remain red, and while zooming there have been tiles with red and amber.
Pre-requisites:
- have webrender enabled and option "gfx.webrender.debug.picture-caching" set to true.
Tested scenarios on the geckoview_example.apk from Comment9 with Google Pixel 2 (Android 9):
- scrolling on the unzoomed page
- cheked while zooming
Note:
When scrolling the tiles are still red, but it flickers red when scrolling is not constant red.
Please check video.
Is this the intended behavior? The flicker shouldn't be present right?
Reporter | ||
Comment 12•5 years ago
|
||
Some amount of red flickering is expected. As you can see in the video when scrolling around before you did any zooming.
In your video, you don't do much scrolling after you start zooming (instead you keep zooming in and out). It would be useful if you could try first zooming in a small amount, and then just scrolling around a lot whilst already zoomed in. Thanks!
Comment 13•5 years ago
|
||
Hi, sorry for the late reply, the results with Google Pixel XL (Android 10) and Google Pixel 3a (Android 9), for the build from comment10 are as follows:
- Google Pixel XL (Android 10)
- Google Pixel 3a (Android 9)
for scenario with scrolling after slightly zooming, please check now.
Reporter | ||
Comment 14•5 years ago
|
||
These look good, confirms the change has worked. Thanks Diana!
Comment 15•5 years ago
|
||
Hi, verified as fixed from mobile Firefox Preview Beta 3.2.0-beta.3 (Build #20250044) GV: 73.0-20200123180644 and on Firefox Preview Nightly 200203 (Build #20340607) with Google Pixel 3a (Android 9) and Google Pixel 3a XL (Android 10).
Reporter | ||
Updated•5 years ago
|
Description
•