Jeff Muizelaar [:jrmuizel] Reporter
	Description • 9 years ago

STR:
- turn on apz.highlight_checkerboarded_areas
- on MBP scroll quickly using the trackpad to the bottom of a page.
- continue to keep scrolling quickly and the page will occaisionally flicker red

	Botond Ballo [:botond]
	Comment 1 • 9 years ago

Do you see a flicker (of white / the page background color) with the pref off?

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 2 • 9 years ago

This might be fixed by bug 1198021.

	Markus Stange [:mstange] Assignee
	Comment 3 • 9 years ago

I can still reproduce this in a build that has the fix for bug 1198021.

	Jeff Muizelaar [:jrmuizel] Reporter
	Comment 4 • 9 years ago

(In reply to Botond Ballo [:botond] from comment #1)
> Do you see a flicker (of white / the page background color) with the pref
> off?

I do.

	Botond Ballo [:botond]
	Comment 5 • 9 years ago

(In reply to Jeff Muizelaar [:jrmuizel] from comment #4)
> (In reply to Botond Ballo [:botond] from comment #1)
> > Do you see a flicker (of white / the page background color) with the pref
> > off?
> 
> I do.

Ok, thanks. That tells us this is actual checkerboarding happening, not just a bug in the checkerboard highlight code.

	Markus Stange [:mstange] Assignee
	Comment 6 • 9 years ago

Here are the relevant parts from a layers.dump-host-layers log. Based on the scrolling "speed", we sometimes enlarge the displayport, but it gets placed too high and doesn't end up intersecting the visible part of the layer. Look for the lines towards the end that have a shadow y translation of -134345 and a valid region with bottom < 134345.

	Markus Stange [:mstange] Assignee
	Comment 7 • 9 years ago

Bug 1229125 - Correct velocity computation for pan gesture events. r?botond

When scrolling down, panInput.mLocalDisplacement.y will be positive, so
aAdditionalDelta will be positive, and newVelocity must be positive.

	Markus Stange [:mstange] Assignee
	Comment 8 • 9 years ago

Bug 1229125 - Clear the velocity queue if we're at the edge of the page. r?botond

Resetting only the current velocity isn't sufficient because EndTouch will
reconstruct a velocity from the velocity queue once the fingers leave the
touchpad.

	Markus Stange [:mstange] Assignee
	Comment 9 • 9 years ago

https://reviewboard.mozilla.org/r/26701/#review24195

This causes two tests in the APZOverscrollHandoffTester gtest to fail. I need to find out why.

	Markus Stange [:mstange] Assignee
	Comment 10 • 9 years ago

I'll just drop that patch for now. It's not necessary to fix the bug.

	Botond Ballo [:botond]
	Comment 11 • 9 years ago

Comment on attachment 8694197 [details]
MozReview Request: Bug 1229125 - Correct velocity computation for pan gesture events. r?botond

https://reviewboard.mozilla.org/r/26699/#review24327

::: gfx/layers/apz/src/Axis.cpp:79
(Diff revision 1)
> -  float newVelocity = mAxisLocked ? 0.0f : (float)(mPos - aPos - aAdditionalDelta) / (float)(aTimestampMs - mPosTimeMs);
> +  float newVelocity = mAxisLocked ? 0.0f : (float)(mPos - aPos + aAdditionalDelta) / (float)(aTimestampMs - mPosTimeMs);

Can you help me understand why this won't regress bug 1212876, where we made the opposite change?

	Botond Ballo [:botond]
	Comment 12 • 9 years ago

Comment on attachment 8694198 [details]
MozReview Request: Bug 1229125 - Reset velocity in OnPanEnd if there's nowhere to scroll. r?botond

Dropping review flag until the test failures are cleared up.

	Markus Stange [:mstange] Assignee
	Comment 13 • 9 years ago

https://reviewboard.mozilla.org/r/26699/#review24327

I think the change I made in bug 1212876 was just wrong.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 14 • 9 years ago

Could it be that for the x-axis we need a - and for the y-axis we need a + ?

	Markus Stange [:mstange] Assignee
	Comment 15 • 9 years ago

I don't think so.

It made sense to invert the sign in bug 1212876 because the value I was passing in also got its sign inverted, but that just means that calculation had the wrong sign all along. Now that I've actually looked at the computed velocity values and compared them to the velocity values that I get from smooth scrolling with a regular mousewheel, I'm confident that the patch here is the right fix.

	Botond Ballo [:botond]
	Comment 16 • 9 years ago

Comment on attachment 8694197 [details]
MozReview Request: Bug 1229125 - Correct velocity computation for pan gesture events. r?botond

https://reviewboard.mozilla.org/r/26699/#review24335

OK, thanks. I just wanted to make sure we weren't making a changing without realizing that we recently made the opposite change :)

	Markus Stange [:mstange] Assignee
	Comment 17 • 9 years ago

Comment on attachment 8694197 [details]
MozReview Request: Bug 1229125 - Correct velocity computation for pan gesture events. r?botond

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26699/diff/1-2/

	Markus Stange [:mstange] Assignee
	Comment 18 • 9 years ago

Comment on attachment 8694198 [details]
MozReview Request: Bug 1229125 - Reset velocity in OnPanEnd if there's nowhere to scroll. r?botond

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26701/diff/1-2/

	Botond Ballo [:botond]
	Comment 19 • 9 years ago

Comment on attachment 8694198 [details]
MozReview Request: Bug 1229125 - Reset velocity in OnPanEnd if there's nowhere to scroll. r?botond

https://reviewboard.mozilla.org/r/26701/#review24339

	Markus Stange [:mstange] Assignee
	Comment 20 • 9 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fd31c11e7765

	Markus Stange [:mstange] Assignee
	Comment 21 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/378af567670fd9b982b1e835cef9767c2566cd5d
Bug 1229125 - Correct velocity computation for pan gesture events. r=botond

https://hg.mozilla.org/integration/mozilla-inbound/rev/b51a04a3514ec0b15eec12a8dcce1e9666892543
Bug 1229125 - Reset velocity in OnPanEnd if there's nowhere to scroll. r=botond

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 22 • 9 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/378af567670f
https://hg.mozilla.org/mozilla-central/rev/b51a04a3514e