733041 - Zoomed-in scrolling is astonishingly checkerboardy

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Joe Drew (not getting mail)]

If you zoom in on a simple page (like a directory listing), we get incredibly checkerboardy, for unknown reasons.

Comment 1

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

This may be a bug in the refreshDisplayPort function; the display port width and height may be incorrectly calculated. I will test my hypothesis once I can stop juggling the patches in my patch queue.

Comment 2

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

Never mind, my hypothesis was incorrect.

Comment 3

[Benoit Girard (:BenWa)]

We should try this example with RenderTrace and see which stage is taking longer.

Comment 4

[Joe Drew (not getting mail)]

This is no longer unusable, just ugly.

Comment 5

[Jeff Muizelaar [:jrmuizel]]

One possible explanation for this is that we try to keep a constant area displayport. This means that when we are zoomed out the displayport will have more room above and below. When we zoom in this value drops to +300 pixels.

Comment 6

[Jeff Muizelaar [:jrmuizel]]

We need to figure out of this is still happening or if it's been fixed.

Comment 7

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

It should still be happening in the default setup, since the FixedMargin strategy that is set as default still does the displayport area-conservation thing which results in less vertical margin when zoomed in. If you change the gfx.displayport.strategy pref to 1 and restart this should no longer manifest because the velocity-bias strategy doesn't do that area-conservation thing. This should be resolved when I switch over to using the velocity-bias as the default, so I'll steal this bug.

Comment 8

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

Attachment: Switch to velocity-bias as default

Increase size of velocity-bias and make it the default. This outperforms the fixed-margin strategy overall, with the various latency fixes applied. Latency due to ShowPress/touchstart still needs to be reduced further to get more gains though.

Comment 9

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

Build with this patch and all the latency patches applied is at https://people.mozilla.com/~kgupta/tmp/vbias15.apk

Comment 10

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

Comment on attachment 613329 [details] [diff] [review]
Switch to velocity-bias as default

Actually better hold off on this, JP is seeing a lot more checkerboarding with this on the GN. I'll investigate more.

Comment 11

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

(In reply to Kartikaya Gupta (:kats) from comment #10)
> Actually better hold off on this, JP is seeing a lot more checkerboarding
> with this on the GN. I'll investigate more.

This seemed to be at least partially a result of the touch event handlers on nytimes.com. Filed bug 743736 to track it.

Comment 12

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

Attachment: (1/2) Split the velocity-bias buffer so we keep a little in the reverse direction

Maybe this patch belongs on a different, but I'm lazy... :)

Comment 13

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

Attachment: (2/2) Turn on velocity-bias as default

Comment 14

[Chris Lord [:cwiiis]]

Comment on attachment 613646 [details] [diff] [review]
(1/2) Split the velocity-bias buffer so we keep a little in the reverse direction

Review of attachment 613646 [details] [diff] [review]:
-----------------------------------------------------------------

Looks fine to me.

Comment 15

[Chris Lord [:cwiiis]]

Comment on attachment 613647 [details] [diff] [review]
(2/2) Turn on velocity-bias as default

Review of attachment 613647 [details] [diff] [review]:
-----------------------------------------------------------------

Fine, though it'd be nice if we could avoid possible creating a redundant DisplayPortCalculator (when the pref isn't the default).

Comment 16

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

(In reply to Chris Lord [:cwiiis] from comment #15)
> 
> Fine, though it'd be nice if we could avoid possible creating a redundant
> DisplayPortCalculator (when the pref isn't the default).

It seems hard to do that without leaving the strategy as null initially (until we find out what the pref is), and I'd rather not do that in case we hit NPEs.


https://hg.mozilla.org/integration/mozilla-inbound/rev/d9b40fa434db
https://hg.mozilla.org/integration/mozilla-inbound/rev/b78eb58c2900

Comment 17

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/mozilla-central/rev/d9b40fa434db
https://hg.mozilla.org/mozilla-central/rev/b78eb58c2900

Comment 18

[Paul Feher]

Verified/fixed on:
Nightly Fennec 15.0a1 (2012-04-25)
Device: HTC Desire Z
OS: Android 2.3.3