Parts of the screen can get left painted in low-res even in a stable state

RESOLVED FIXED in Firefox 32, Firefox OS v2.0

Status

()

Core
Graphics: Layers
RESOLVED FIXED
4 years ago
8 months ago

People

(Reporter: kats, Assigned: kats)

Tracking

Trunk
mozilla33
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.0+, firefox31 wontfix, firefox32 fixed, firefox33 fixed, b2g-v2.0 fixed, b2g-v2.1 fixed)

Details

(URL)

Attachments

(1 attachment)

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).
Created attachment 8442042 [details] [diff] [review]
Patch

This fix makes sense to me and does the job. I'll test it with bug 1021248 before flagging for review.
Blocks: 1025946
blocking-b2g: --- → 2.0?
Attachment #8442042 - Flags: review?(chrislord.net)

Comment 5

4 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.
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.
blocking-b2g: 2.0? → 2.0+
https://hg.mozilla.org/mozilla-central/rev/b776a7d35ba3
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Depends on: 1023882
Flagging NO_UPLIFT for being suspected for causing the smoketest regression in bug 1028271.
Whiteboard: [NO_UPLIFT]
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.
status-b2g-v2.0: --- → affected
status-b2g-v2.1: --- → fixed
This should be safe to uplift now.
Whiteboard: [NO_UPLIFT]
https://hg.mozilla.org/releases/mozilla-aurora/rev/3a93fc4f0265
status-b2g-v2.0: affected → fixed
status-firefox31: --- → wontfix
status-firefox32: --- → fixed
status-firefox33: --- → fixed
No longer blocks: 1025946
Duplicate of this bug: 1025946

Updated

3 years ago
See Also: → bug 1083119
You need to log in before you can comment on or make changes to this bug.