1472830 - Work from ApplyOpacityToChildren is never reused

Status: RESOLVED FIXED

(Core :: Web Painting, enhancement)

Description

[Bas Schouten (:bas.schouten)]

With retained display list, if ApplyOpacityToChildren has concluded the opacity can not be handled by the children, it doesn't need to check again next frame if the child list has not mutated at all.

Comment 1

[Bas Schouten (:bas.schouten)]

Attachment: Bug 1472830: Reuse the work from ApplyOpacityToChildren when possible.

Review commit: https://reviewboard.mozilla.org/r/254338/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/254338/

Comment 2

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8989283 [details]
Bug 1472830: Reuse the work from ApplyOpacityToChildren when possible.

https://reviewboard.mozilla.org/r/254338/#review261198

::: layout/painting/RetainedDisplayListBuilder.cpp:300
(Diff revision 1)
>            if (mBuilder->MergeDisplayLists(aNewItem->GetChildren(),
>                                            oldItem->GetChildren(),
>                                            aNewItem->GetChildren(),
>                                            containerASRForChildren,
>                                            aNewItem->GetPerFrameKey())) {
> +            aNewItem->InvalidateCachedChildInfo();

This call is unnecessary, since aNewItem is new, and there can't be any cached state on it.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7e6eabfa350b
Reuse the work from ApplyOpacityToChildren when possible. r=mattwoodrow

Comment 4

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/7e6eabfa350b

Comment 5

[Miko Mynttinen]

Comment on attachment 8989283 [details]
Bug 1472830: Reuse the work from ApplyOpacityToChildren when possible.

Approval Request Comment
[Feature/Bug causing the regression]: Used by patch for bug 1472465 to fix opacity invalidation issue.
[User impact if declined]: Blocks uplifting bug 1472465. A similar state check would have to be implemented anyway for uplift.
[Is this code covered by automated tests?]: The code is run during most tests that use opacity, especially mochitests.
[Has the fix been verified in Nightly?]: N/A, this is just an optimization.
[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?]: Low risk.
[Why is the change risky/not risky?]: The code is easy to reason about - caching of enum value instead of recalculating it.
[String changes made/needed]: No.

Comment 6

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8989283 [details]
Bug 1472830: Reuse the work from ApplyOpacityToChildren when possible.

Looks like we should land this along with the patch in bug 1472465.
Fix for recent regression, has good existing test coverage.

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/84f7e9706a49