Closed Bug 1384387 Opened 7 years ago Closed 7 years ago

should call ClearInvalidationStateBits in layers-free mode

Categories

(Core :: Graphics: WebRender, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: ethlin, Assigned: ethlin)

References

Details

Attachments

(1 file)

We check nsDisplayItem::IsInvalid() to know if we need to repaint the item. We should clear the invalidation bit after each transaction for frames or the frames will keep having the NS_FRAME_NEEDS_PAINT bit. [1] https://dxr.mozilla.org/mozilla-central/source/layout/painting/nsDisplayList.h#1757
Comment on attachment 8890184 [details] Bug 1384387 - Clear frame's invalidation state bits in layers-free mode. https://reviewboard.mozilla.org/r/161254/#review166804 Mostly a rubberstamp since I don't know what this code is doing but since it's a copy-paste from the non-WR codepath it seems reasonable enough. ::: layout/painting/nsDisplayList.cpp:2111 (Diff revision 1) > + if (widgetTransaction || > + // SVG-as-an-image docs don't paint as part of the retained layer tree, > + // but they still need the invalidation state bits cleared in order for > + // invalidation for CSS/SMIL animation to work properly. > + (document && document->IsBeingUsedAsImage())) { > + frame->ClearInvalidationStateBits(); In the non-WR codepath this happens before calling EndTransaction. Is there a reason you're doing after calling EndTransaction here? If so that reason should be documented somewhere. If there's no reason then I'd prefer doing it before EndTransaction for consistency - if we ever do end up unifying the non-WR codepath with this then having the same ordering makes it one less thing to check.
Attachment #8890184 - Flags: review?(bugmail) → review+
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #2) > Comment on attachment 8890184 [details] > Bug 1384387 - Clear frame's invalidation state bits in layers-free mode. > > https://reviewboard.mozilla.org/r/161254/#review166804 > > Mostly a rubberstamp since I don't know what this code is doing but since > it's a copy-paste from the non-WR codepath it seems reasonable enough. > > ::: layout/painting/nsDisplayList.cpp:2111 > (Diff revision 1) > > + if (widgetTransaction || > > + // SVG-as-an-image docs don't paint as part of the retained layer tree, > > + // but they still need the invalidation state bits cleared in order for > > + // invalidation for CSS/SMIL animation to work properly. > > + (document && document->IsBeingUsedAsImage())) { > > + frame->ClearInvalidationStateBits(); > > In the non-WR codepath this happens before calling EndTransaction. Is there > a reason you're doing after calling EndTransaction here? If so that reason > should be documented somewhere. If there's no reason then I'd prefer doing > it before EndTransaction for consistency - if we ever do end up unifying the > non-WR codepath with this then having the same ordering makes it one less > thing to check. I have to do this after EndTransaction. For layers mode, FrameLayerBuilder checks the flags and set the invalidation regions to layers, so we can clear the bits before EndTransaction. For layers-free mode, everything happens in EndTransaction so we must clear the bits after EndTransaction. I will add comments above the changes.
Pushed by ethlin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4415d55c49f7 Clear frame's invalidation state bits in layers-free mode. r=kats
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: