Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OpacityLayer peephole optimization does not optimize multiple non-overlapping children #93899

Closed
Tracked by #85737
flar opened this issue Nov 18, 2021 · 6 comments
Closed
Tracked by #85737
Labels
c: performance Relates to speed or footprint issues (see "perf:" labels) engine flutter/engine repository. See also e: labels. P1 High-priority issues at the top of the work list

Comments

@flar
Copy link
Contributor

flar commented Nov 18, 2021

The peephole optimization that will allow an OpacityLayer to hand off its opacity value to its children to avoid a saveLayer will only currently apply its optimization to a single child as it is initially implemented in flutter/engine#29775.

With further processing, the children could be checked for their paint_bounds() overlapping and multiple children could all inherit the opacity value, increasing the number of cases optimized.

@danagbemava-nc danagbemava-nc added in triage Presently being triaged by the triage team engine flutter/engine repository. See also e: labels. passed first triage c: performance Relates to speed or footprint issues (see "perf:" labels) and removed in triage Presently being triaged by the triage team labels Nov 19, 2021
@chinmaygarde chinmaygarde added P2 Important issues not at the top of the work list P1 High-priority issues at the top of the work list and removed P2 Important issues not at the top of the work list labels Nov 29, 2021
@flar flar changed the title OpacityLayer peephole optimization cannot handle multiple non-overlapping children OpacityLayer peephole optimization does not optimize multiple non-overlapping children Jan 7, 2022
@JsouLiang
Copy link
Contributor

JsouLiang commented Jul 14, 2022

the children could be checked for their paint_bounds() overlapping and multiple children

Could we use RTree to check if the children is intersection?
cc @flar

@flar
Copy link
Contributor Author

flar commented Jul 14, 2022

This optimization was actually implemented before flutter/engine#29775 was even landed.

@flar flar closed this as completed Jul 14, 2022
@flar
Copy link
Contributor Author

flar commented Jul 14, 2022

the children could be checked for their paint_bounds() overlapping and multiple children

Could we use RTree to check if the children is intersection? cc @flar

An RTree can potentially produce a more authoritative answer at a greater cost. The question is how much better is its answer compared to the cost of using it on every container's individual children?

@JsouLiang
Copy link
Contributor

JsouLiang commented Jul 15, 2022

@flar
Copy link
Contributor Author

flar commented Jul 15, 2022

Should we support this : https://github.com/flutter/engine/blob/main/flow/layers/container_layer.cc#L156 cc @flar

That is another possibility. Since we are going to need to deal with RTree sooner or later to support platform embedders that rely on RTree for event distribution, we have to do some research to figure out which of these mechanisms can support what we need. The simple "while new children don't intersect previous bounds" test could probably be done with minimal work using the existing BoundsAccumulator work in the BoundsCalculator, but that isn't integrated with the construction and DL objects must be immutable after construction.

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
c: performance Relates to speed or footprint issues (see "perf:" labels) engine flutter/engine repository. See also e: labels. P1 High-priority issues at the top of the work list
Projects
Status: Done
Development

No branches or pull requests

4 participants