Closed Bug 1589669 Opened 5 years ago Closed 5 years ago

Picture caching doesn't work well when zoomed in on android

Categories

(Core :: Graphics: WebRender, defect, P3)

Unspecified
Android
defect

Tracking

()

VERIFIED FIXED
mozilla73
Tracking Status
firefox73 --- verified

People

(Reporter: jnicol, Assigned: ktaeleman)

References

Details

Attachments

(1 file)

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.

Flags: needinfo?(gwatson)

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.

Flags: needinfo?(gwatson)

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?

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?

Poking this bug along, Jamie do you have answers to Glenn's questions? :)

Flags: needinfo?(jnicol)

Been looking at this more today.

Currently my theory is 2 fold:

Rounding before dividing by zoom and removing the snap_offset function seems to make tile cache invalidation to stop happening on every scroll.

Flags: needinfo?(jnicol)
Assignee: nobody → ktaeleman
Priority: -- → P3
  • Fix crash due to shift left causing overflow (debug only)
  • Remove rounding of scrolling offsets and snap to view space instead of
    world space
Attachment #9115628 - Attachment description: Bug 1589669 - Picture Caching doesn't work well when zoomed in on android. → Bug 1589669 - Fix snapping and rounding errors causing picture caching invalidation when zoomed in.
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

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 handler
TestRunner._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 handler
TestRunner._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 handler
TestRunner._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 handler
TestRunner._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 handler
TestRunner._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 callback
TestRunner.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.

Flags: needinfo?(ktaeleman)
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
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla73
Flags: needinfo?(ktaeleman)
Blocks: 1598434
Flags: qe-verify+

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?

Flags: needinfo?(jnicol)

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!

Flags: needinfo?(jnicol)

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:

Flags: needinfo?(jnicol)

These look good, confirms the change has worked. Thanks Diana!

Flags: needinfo?(jnicol)

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).

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: