Disappearing images/flickering on Yelp carousel

RESOLVED FIXED in Firefox 58

Status

()

defect
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: yoasif, Assigned: mattwoodrow)

Tracking

({nightly-community})

Trunk
mozilla59
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox58 fixed, firefox59 fixed)

Details

()

Attachments

(2 attachments)

Steps to reproduce:

1. Create a new profile. Enable layout.display-list.retain. Restart
2. Navigate to a Yelp listing with photos, eg: https://www.yelp.com/biz/los-tacos-no-1-new-york
3. Hover over photos in photo carousel



Actual results:

Images disappear/reappear and flicker. 


Expected results:

This should work the way it does when retained display lists are not enabled.
Has STR: --- → yes
We already mark all preserve-3d frames for display (MarkPreserve3DFramesForDisplayList), but attempting to save and restore the preserve-3d was messing us up.

ComputeRebuildRegion computes the dirty rect for the middle transformed frame, and store that on its stacking context (the outer transformed frame), and sets MarkFrameForDisplayIfVisible on all ancestors.

When we build, we reach the outer frame with an empty dirty region (but we carry on since we have MarkFrameForDisplayIfVisible). We store that empty dirty region with SavePreserve3DRects.

We reach the middle transformed frame and load our stored dirty rect from ComputeRebuildRegion.

We reach the leaf transformed frame and load the cached preserve-3d rect (empty), and building stops there. The background color item never gets built, and the test fails.

This just makes sure we rebuild everything within preserve-3d if we touch it at all.
Assignee: nobody → matt.woodrow
Attachment #8928028 - Flags: review?(mikokm)
Comment on attachment 8928028 [details] [diff] [review]
Don't do partial building for preserve-3d

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

LGTM.
Attachment #8928028 - Flags: review?(mikokm) → review+
Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/609829a7c75e
Don't do partial display list building within preserve-3d contexts. r=miko
https://hg.mozilla.org/mozilla-central/rev/609829a7c75e
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Comment on attachment 8928028 [details] [diff] [review]
Don't do partial building for preserve-3d

Approval Request Comment
[Feature/Bug causing the regression]: bug 1352499. This is code that is preffed off, but we want to run a shield study enabling the pref.
[User impact if declined]: None, preffed off code.
[Is this code covered by automated tests?]: Yes, when the pref is enabled.
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Code is preffed off.
[String changes made/needed]: None
Attachment #8928028 - Flags: approval-mozilla-beta?
Comment on attachment 8928028 [details] [diff] [review]
Don't do partial building for preserve-3d

Take this to support the shield study in 58. Beta58+.
Attachment #8928028 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Matt Woodrow (:mattwoodrow) from comment #5)
> [Is this code covered by automated tests?]: Yes, when the pref is enabled.
> [Needs manual test from QE? If yes, steps to reproduce]: No

Marking as qe-verify- since it does not require manual testing and it has automated coverage.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.