Closed Bug 1026756 Opened 5 years ago Closed 5 years ago
Parts of the screen can get left painted in low-res even in a stable state
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.
The test case at the URL reproduces it for me on latest master on a Flame device.
(Inserting a 200ms sleep at the top of APZCTM::UPZCT will make this trigger more reliably if needed).
This fix makes sense to me and does the job. I'll test it with bug 1021248 before flagging for review.
5 years ago
blocking-b2g: --- → 2.0?
Attachment #8442042 - Flags: review?(chrislord.net)
Try push at https://tbpl.mozilla.org/?tree=Try&rev=0fdf3b2a8552 for reference.
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.
Attachment #8442042 - Flags: review?(chrislord.net) → review+
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...
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.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Depends on: 1023882
Flagging NO_UPLIFT for being suspected for causing the smoketest regression in bug 1028271.
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.
This should be safe to uplift now.
No longer blocks: 1025946
Duplicate of this bug: 1025946
2 years ago
You need to log in before you can comment on or make changes to this bug.