Closed
Bug 1384387
Opened 7 years ago
Closed 7 years ago
should call ClearInvalidationStateBits in layers-free mode
Categories
(Core :: Graphics: WebRender, enhancement)
Core
Graphics: WebRender
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 hidden (mozreview-request) |
Comment 2•7 years ago
|
||
mozreview-review |
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+
Assignee | ||
Comment 3•7 years ago
|
||
(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.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
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
Comment 7•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in
before you can comment on or make changes to this bug.
Description
•