Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Description • 10 years ago

While investigating bug 1021248 I found this bug where things can get left on the screen in low-res. This happens if the content does a scrollTo to somewhere else, and the new scroll offset has not yet reached the APZC at the time that AboutToCheckerboard is called as part of the progressive paint code. Because the new scroll offset has not yet reached the APZC, but it has reached the client tiling code, AboutToCheckerboard will return true, and so the high-res paint will be aborted. However there is nothing after this point that will trigger a new paint so unless the user does something to trigger a paint it gets left in low-res.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 1 • 10 years ago

The test case at the URL reproduces it for me on latest master on a Flame device.

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

(Inserting a 200ms sleep at the top of APZCTM::UPZCT will make this trigger more reliably if needed).

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 3 • 10 years ago

This fix makes sense to me and does the job. I'll test it with bug 1021248 before flagging for review.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 4 • 10 years ago

Try push at https://tbpl.mozilla.org/?tree=Try&rev=0fdf3b2a8552 for reference.

	Chris Lord [:cwiiis]
	Comment 5 • 10 years ago

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

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

r+, but read and consider the comment :)

::: gfx/layers/client/TiledContentClient.cpp
@@ +238,5 @@
>  }
>  
>  bool
>  SharedFrameMetricsHelper::AboutToCheckerboard(const FrameMetrics& aContentMetrics,
>                                                const FrameMetrics& aCompositorMetrics)

This is fine, but I wonder if AboutToCheckerboard is the right place to put this check, as opposed to the call-site? Your call.

	Milan Sreckovic [:milan] (needinfo for best results)
	Comment 6 • 10 years ago

Based on the current documentation of AboutToCheckerboard() function, it feels like it should be at the call site. The function described is really "BoundsOutsideDisplayPort()", but it so happens that we're using it for checkerboarding.  With this change, the function becomes "NotSafeFromCheckerboarding()" or something like that, so it's worth updating the documentation to match, or renaming it, or using the extra logic at the call site, or...

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 7 • 10 years ago

I'll move it to the call site, that makes sense. Bug 1024126 is on file for enhancing AboutToCheckerboard so it does what it claims.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 8 • 10 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/b776a7d35ba3

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

https://hg.mozilla.org/mozilla-central/rev/b776a7d35ba3

	Jason Smith [:jsmith]
	Comment 10 • 10 years ago

Flagging NO_UPLIFT for being suspected for causing the smoketest regression in bug 1028271.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 11 • 10 years ago

I backed out bug 1021248 for the smoketest regression but I'll leave NO_UPLIFT on this for the time being until we are sure this one doesn't need backing out as well.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Assignee
	Comment 12 • 10 years ago

This should be safe to uplift now.

	Ryan VanderMeulen [:RyanVM]
	Comment 13 • 10 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/3a93fc4f0265