1167721 - stopping rapid scrolling with a tap can leave the screen partially or fully blurred

Status: VERIFIED FIXED

(Core :: Panning and Zooming, defect)

Description

[David Flanagan [:djf]]

I discovered this bug while investigating bug 1167336, which is about the FirefoxOS Video app. It is not memory related, because I see it on both low-memor and high-memory Flame devices.  It affects both master and v2.2, and seems like something we need to block on for 2.2

I'm guessing that this is an APZ bug, but I suppose it could be a layout or graphics bug as well.

Steps to reproduce:

1) push 200 videos to your phone like this:

$ cd gaia/test_media/reference_workload
$ ./generateVideo.sh 200

2) Launch the video app and wait for it to scan all the videos

3) Scroll as fast as you can until gecko starts to display blurred thumbnails during the scroll

4) Tap  quickly to stop the scrolling.

5) If all of the thumbnails and text become clear, then go to step 3 and try again.

I find that I can get a blurred display at least one out of four times. I'll attach screenshots.

It is probably better to test this on a Flame with 1024mb of memory because then you shouldn't see the blank thumbnail symptoms of bug 1167336.

Comment 1

[David Flanagan [:djf]]

Attachment: screenshot of the screen partially blurred.

Here's an example of the bug. The blur does not go away on its own. It does go away as soon as I interact with the app again.

This does not seem to happen if I let the scrolling slow down and stop on its own. Only if I tap to stop it abruptly.

Comment 2

[David Flanagan [:djf]]

Nominating this as a 2.2 blocker.

Needinfo Kats since he has worked on other APZ-related bugs. Kats: does this sound like an APZ bug to you? Do you know who can take it?

Also needinfo Milan in case this is a graphics bug.

Comment 3

[David Flanagan [:djf]]

Attachment: Gallery app affected by this bug

I can reproduce this bug in the gallery app as well. I put 300 images on my sdcard, waited for gallery to scan them all, then scrolled fast and tapped once the blurring started.

Note that it is important to wait for scanning to stop. If scanning is still going on, then the DOM changes that it causes will apparently produce redraws and make the bluring go away.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Hm, I thought we fixed the last of these, but apparently not. I can look into it. Updating flags based on comment 0.

Comment 5

[David Flanagan [:djf]]

I think that this problem may disappear in the gallery app if I remove will-change:scroll-position from gallery.css for the element that holds all the thumbnails.

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Attachment: Tiling log

I was able to reproduce this and get a log (attached). It seems like the visible area of the layer is marked as valid, so it should be painted in high-res but for some reason it's not. I need to study the log more to see if I can find the root cause there, or if the problem is elsewhere.

Comment 7

[Kartikaya Gupta (email:[e-mail])]

Yeah from the log all seems to be in order. The areas that are invalid are promptly painted and become valid again. There might be some bug in the code that pushes the painted tiles over to the compositor which is outside the scope of the logging I added.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Hm, this doesn't even appear to be tiling related. I can reproduce with low-precision painting disabled and tiling disabled (except in those scenarios I get solid black instead of blurry content, as expected). Something else is going on here...

Comment 9

[Kartikaya Gupta (email:[e-mail])]

Ok, looks like it is a bug in the APZC code, possibly a regression from bug 1034376. While flinging, the APZC positions the displayport margins based on the velocity, and this may be far outside the on-screen area. When we tap down to stop the fling, CancelAnimation() is called and zeros out the velocity, but we never trigger a repaint after that to recompute the margins. This results in a stale displayport which can be far away from what's actually on-screen. Patch coming.

Comment 10

[Kartikaya Gupta (email:[e-mail])]

Attachment: Patch

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4362d9251296

Comment 12

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/4362d9251296

Comment 13

[Kartikaya Gupta (email:[e-mail])]

David, can you double-check to make sure the problem is fixed now? If so I can request uplift to 2.2

Comment 14

[David Flanagan [:djf]]

Kats: I can no longer reproduce on master. That was quick work. Thanks! Please do request uplift.

Comment 15

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8610177 [details] [diff] [review]
Patch

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): possibly bug 1034376 but I'd want a regression window to verify that
User impact if declined: in some cases after flinging hard and stopping the fling using a tap, the content ends up blurry or missing (and stays that way until the user touches/scrolls again).
Testing completed: manual testing by myself and reporter
Risk to taking this patch (and alternatives if risky): fairly low risk, code is well contained
String or UUID changes made by this patch: none

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/376c2b2a4a26

Comment 17

[Kartikaya Gupta (email:[e-mail])]

So. Ryan informed that this caused a gtest failure on b2g37. I debugged it, and it's happening because the IsZero implementation in b2g37 is different (it uses FuzzyEqualsMultiplicative) than master (which uses a 0.01 coordinate-epsilon). As it happens, in the two failing tests, the "OVERCOME_TOUCH_TOLERANCE" is 1 pixel, which over the 100ms pan interval causes a velocity of 0.0099999999 (thanks, floating-point error!). This velocity gets treated as zero in master but non-zero in b2g37, resulting in the test failure.

This does raise a legitimate question: is this patch still safe on b2g37 with the different IsZero implementation? I think the answer is yes because it just means we might trigger repaints slightly more frequently but that's about it.

Considering the patch is still safe, I'd like to apply a test-only fix to make these tests pass. Considering the two failing tests assume touch-action is enabled, and touch action is not actually enabled on b2g37 (and won't be), I'm just going to disable those two tests on b2g37.

Comment 18

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/7012f97f7814

Comment 19

[Pi Wei Cheng [:piwei] (inactive)]

This issue has been verified fixed on Flame 3.0. Tested with original STR, as well as in Gallery as mentioned on comment 3. Abruptly stopping a rapid scroll now always does not leave any blurriness on screen. Aside from 1024MB memory I also tested with 319MB memory since I know the bug in Video app should be fixed on latest. The missing thumbnail issue is still not resolved in Gallery, so in Gallery I didn't test with 319MB.

Device: Flame (KK, full flashed, 319/1024 MB mem)
BuildID: 20150529010201
Gaia: e7d268074ee3c9eeb191c2205c0e35992fb3915d
Gecko: f986e55c4e0b
Gonk: 040bb1e9ac8a5b6dd756fdd696aa37a8868b5c67
Version: 41.0a1 (3.0 Master) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0

Leaving verifyme tag since it has just recently uplifted and I doubt it's in today's nightly.