Closed Bug 1428993 Opened 8 years ago Closed 8 years ago

Graphical glitch with retained display lists

Categories

(Core :: Web Painting, defect, P2)

Product:
Core
Component:
Web Painting
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla59
Tracking Flags:
Tracking Status
firefox59 --- fixed

People

(Reporter: birtles, Assigned: mikokm)

References

Details

Attachments

(5 files, 1 obsolete file)

CSS for test
29.40 KB, text/css
Details
Test
1.13 KB, text/html
Details
simplified.html
808 bytes, text/html
Details
Bug 1428993 - Part 2: Override dirty rect for stacking contexts between OOF frame placeholder and the containing block
59 bytes, text/x-review-board-request
mattwoodrow
: review+
Details
Bug 1428993 - Part 1: Split RetainedDisplayListBuilder::ComputeRebuildRegion() and PreProcessDisplayList() into multiple functions
59 bytes, text/x-review-board-request
mattwoodrow
: review+
Details
See forthcoming test. If you hover the quarter-circles in the bottom of the test they should enlarge and cover the white area but most of the time they incorrectly go underneath the white area. Reproduces for me in Nightly on Linux and Windows. Does not reproduce if I disable retained display lists.
Attached file CSS for test —
Attached file Test (obsolete) —
Reproduces slightly more readily on the red circle than the green.
Sorry, the CSS file is massive. I reduced the HTML file pretty far, but the CSS could use a lot more reduction. The more I reduced the HTML, however, the less likely it would reproduce so I thought I'd post the test in its complex-but-reproduces-very-easily state.
Attached file Test —
Just updating the test title
Attachment #8940979 - Attachment is obsolete: true
Thank you for the report. I can reproduce this locally on macOS.
Assignee: nobody → mikokm
Status: NEW → ASSIGNED
Attached file simplified.html —
Simplified testcase
Priority: -- → P2
Comment on attachment 8943025 [details] Bug 1428993 - Part 2: Override dirty rect for stacking contexts between OOF frame placeholder and the containing block https://reviewboard.mozilla.org/r/213288/#review219088 ::: layout/painting/RetainedDisplayListBuilder.cpp:646 (Diff revision 1) > > + // If the current frame is an OOF frame, DisplayListBuildingData needs to be > + // set on all the ancestor stacking contexts of the placeholder frame, up > + // to the containing block of the OOF frame. This is done to ensure that the > + // content that might be behind the OOF frame is built for merging. > + nsIFrame* placeholder = previousFrame->HasAnyStateBits(NS_FRAME_OUT_OF_FLOW) We could move this whole placeholder handling block into a helper, and put it before the TransformFrameRectToAncestor call to avoid needing previousFrame. ::: layout/painting/RetainedDisplayListBuilder.cpp:647 (Diff revision 1) > + // If the current frame is an OOF frame, DisplayListBuildingData needs to be > + // set on all the ancestor stacking contexts of the placeholder frame, up > + // to the containing block of the OOF frame. This is done to ensure that the > + // content that might be behind the OOF frame is built for merging. > + nsIFrame* placeholder = previousFrame->HasAnyStateBits(NS_FRAME_OUT_OF_FLOW) > + ? do_QueryFrame(previousFrame->GetPlaceholderFrame()) Do we need do_QueryFrame here? We should only need it when we're casting to a derived type, nsPlaceholderFrame -> nsIFrame should just work.
Attachment #8943025 - Flags: review?(matt.woodrow) → review+
Comment on attachment 8943365 [details] Bug 1428993 - Part 1: Split RetainedDisplayListBuilder::ComputeRebuildRegion() and PreProcessDisplayList() into multiple functions https://reviewboard.mozilla.org/r/213684/#review219452
Attachment #8943365 - Flags: review?(matt.woodrow) → review+
Pushed by mikokm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/7f88ebf8d0d1 Part 1: Split RetainedDisplayListBuilder::ComputeRebuildRegion() and PreProcessDisplayList() into multiple functions r=mattwoodrow https://hg.mozilla.org/integration/autoland/rev/080c07796f4c Part 2: Override dirty rect for stacking contexts between OOF frame placeholder and the containing block r=mattwoodrow
Pushed by mikokm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/2eb0a49e3192 Part 1: Split RetainedDisplayListBuilder::ComputeRebuildRegion() and PreProcessDisplayList() into multiple functions r=mattwoodrow https://hg.mozilla.org/integration/autoland/rev/2c2e56a87ad1 Part 2: Override dirty rect for stacking contexts between OOF frame placeholder and the containing block r=mattwoodrow
The failing reftest layout/reftests/display-list/1428993-1.html is now disabled on Android due to weird behavior around the button edges. The second added reftest layout/reftests/display-list/1428993-2.html tests the same functionality.
Flags: needinfo?(mikokm)
https://hg.mozilla.org/mozilla-central/rev/2eb0a49e3192 https://hg.mozilla.org/mozilla-central/rev/2c2e56a87ad1
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox59: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Attachment 8940980 [details] still doesn't seem to be fixed for me in the latest nightly.
Flags: needinfo?(nerli)
Sorry, I meant to direct that needinfo to Miko!
Flags: needinfo?(nerli) → needinfo?(mikokm)
(In reply to Brian Birtles (:birtles) from comment #20) > Attachment 8940980 [details] still doesn't seem to be fixed for me in the > latest nightly. Thank you again Brian. It seems that there are some additional problems that the simplified testcase did not catch. I will investigate this again, and file a new bug for possible follow-up work.
Flags: needinfo?(mikokm)
Thanks Miko! Please CC me on any follow-up bugs.
Depends on: 1432553
Depends on: 1446667
Depends on: 1453601
Depends on: 1636071
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: