should call ClearInvalidationStateBits in layers-free mode

RESOLVED FIXED in Firefox 56

Status

()

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: ethlin, Assigned: ethlin)

Tracking

unspecified
mozilla56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

2 years ago
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

2 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

2 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)

Comment 6

2 years ago
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

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4415d55c49f7
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.