Fix handling of SubtreeCaptureId in EffectTree and draw_property_utils #29332
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This CL addresses 2 related issues: 1. When determining whether an EffectNode is drawn, we only check if current node has a valid subtree capture id, instead of checking whether it belongs to a subtree that is being captured. 2. When checking if LayerShouldBeSkippedForDrawPropertiesComputation() in draw_property_utils, we also only look at the subtree capture id, instead of checking whether the layer's node belongs to a captured subtree. Those 2 issues cause the layer's quads not to be emitted to a render pass (they both cause the layer to not be considered as contributing to drawn render surface), even if it seems that they should, since they are being captured. Note that this CL should not change any visible behavior, since the EffectTree::EffectiveOpacity() still uses `subtree_hidden` attribute of EffectNode to determine whether it should be transparent, and therefore the CL won't fix the bug 999305 yet. Other changes: - Add `ToString()` on AggregatedRenderFrame, along with the `AsValueInto()` methods to AggregatedRenderFrame and AggregatedRenderPass to help with the investigations. Those are called from viz::Display post-aggregation if VLOG level is set to 3. - Slightly tweak CompositorRenderPass::AsValueInto() to include properties specific to CompositorRenderPass(). - Add logging of `subtree_hidden` in EffectNode's AsValueInto(), reorder the code to align it a bit more with declaration order in the .h file (the JSON output should stay the same as the properties will be listed in alphabetical order anyway). Bug: 999305 Change-Id: I9a1d6efb750aee82106c0f30afe7f44f2f948ca6
wpt-pr-bot
approved these changes
Jun 10, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This CL addresses 2 related issues:
current node has a valid subtree capture id, instead of checking whether
it belongs to a subtree that is being captured.
in draw_property_utils, we also only look at the subtree capture id,
instead of checking whether the layer's node belongs to a captured
subtree.
Those 2 issues cause the layer's quads not to be emitted to a render
pass (they both cause the layer to not be considered as contributing to
drawn render surface), even if it seems that they should, since they are
being captured. Note that this CL should not change any visible
behavior, since the EffectTree::EffectiveOpacity() still uses
subtree_hiddenattribute of EffectNode to determine whether it shouldbe transparent, and therefore the CL won't fix the bug 999305 yet.
Other changes:
ToString()on AggregatedRenderFrame, along with theAsValueInto()methods to AggregatedRenderFrame andAggregatedRenderPass to help with the investigations. Those are called
from viz::Display post-aggregation if VLOG level is set to 3.
properties specific to CompositorRenderPass().
subtree_hiddenin EffectNode's AsValueInto(), reorderthe code to align it a bit more with declaration order in the .h file
(the JSON output should stay the same as the properties will be listed
in alphabetical order anyway).
Bug: 999305
Change-Id: I9a1d6efb750aee82106c0f30afe7f44f2f948ca6
Reviewed-on: https://chromium-review.googlesource.com/2923120
WPT-Export-Revision: d5e108d97ca5b4af8e45668ef7c339c8b0045d65