[BlinkGenPropertyTrees] Implement property tree damage tracking |
||
Issue descriptionThe compositor computes damage regions for layers associated with changed property tree nodes, and layers associated with any descendant of a changed property tree node (see: DamageTracker::AccumulateDamageFromLayer). This is done for all LayerImpls with |LayerImpl::LayerPropertyChanged()| which is set from |Layer::subtree_property_changed()|. In non-layer-list mode, |Layer::subtree_property_changed()| is set when a property changes (e.g., Layer::SetTransform) and descendants later have subtree_property_changed set recursively in the cc property tree builder. In layer-list mode (BlinkGenPropertyTrees), we do not run the cc property tree builder so we do not set |Layer::subtree_property_changed()| recursively and therefore do not cause damage for layers in a changed property node subtree (see: damage.html and the test in https://chromium-review.googlesource.com/c/chromium/src/+/1312625). Here is a proposal for property tree damage tracking for BlinkGenPropertyTrees: 1) Add a PropertyTreeDamage struct on LocalFrameView that contains a set of property tree nodes that have changed. 2) When updating property tree nodes in the PrePaintTreeWalk, have the PropertyTreeBuilder return a set of changed nodes. This is straightforward because we already return a bool for whether any node changed for raster invalidation purposes. 3) Have the PrePaintTreeWalk accumulate changed nodes in the PropertyTreeDamage struct. 4) In PaintArtifactCompositor, when updating cc::Layers, call cc::Layer::SetSubtreePropertyChanged if the layer's property tree state is changed or is in a changed subtree using PropertyTreeDamage. Initially this could be a naive O(n^2) approach that iterates over all ancestor nodes of the layer's property tree state looking for entries in the PropertyTreeDamage. 5) Clear PropertyTreeDamage.
,
Nov 6
Thanks for the link to blink::PropertyTreeNode::Changed()! That solves steps 1-3 above. I didn't really understand raster invalidator before but I do now. I think this bug applies to SPV2 as well (the test fails in spv2 mode). The raster invalidator detects changes inside a layer but we don't have code for detecting changes between a layer and the root. A very simple patch that calls cc::Layer::SetSubtreePropertyChanged if any property tree is changed on a layer or any ancestor seems to work for my simple tests. I'm going to try putting together a patch using this approach.
,
Nov 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c0b74347b18c30a9286d0a815be9a8b76ef1c8c9 commit c0b74347b18c30a9286d0a815be9a8b76ef1c8c9 Author: Philip Rogers <pdr@chromium.org> Date: Thu Nov 08 15:39:02 2018 [BlinkGenPropertyTrees] Inform cc of damage caused by property trees The compositor has a concept of "damage" that is similar to the raster invalidation calculated in blink and represents the part of the screen that needs to be redrawn. Damage in the compositor is calculated using |Layer::subtree_property_changed()|. This patch uses SPV2's property node "changed" bits to mark cc::Layers as changed if any ancestor property tree node changes. Bug: 901051 Change-Id: I8899ce2ef8dd2b37855f38c71dea443d28520788 Reviewed-on: https://chromium-review.googlesource.com/c/1324093 Commit-Queue: Philip Rogers <pdr@chromium.org> Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org> Cr-Commit-Position: refs/heads/master@{#606472} [modify] https://crrev.com/c0b74347b18c30a9286d0a815be9a8b76ef1c8c9/third_party/blink/renderer/core/exported/web_frame_test.cc [modify] https://crrev.com/c0b74347b18c30a9286d0a815be9a8b76ef1c8c9/third_party/blink/renderer/core/frame/local_frame_view.cc [modify] https://crrev.com/c0b74347b18c30a9286d0a815be9a8b76ef1c8c9/third_party/blink/renderer/platform/graphics/compositing/paint_artifact_compositor.cc [modify] https://crrev.com/c0b74347b18c30a9286d0a815be9a8b76ef1c8c9/third_party/blink/renderer/platform/graphics/compositing/paint_artifact_compositor.h [modify] https://crrev.com/c0b74347b18c30a9286d0a815be9a8b76ef1c8c9/third_party/blink/renderer/platform/graphics/paint/paint_controller.cc [modify] https://crrev.com/c0b74347b18c30a9286d0a815be9a8b76ef1c8c9/third_party/blink/renderer/platform/graphics/paint/paint_controller.h
,
Nov 8
|
||
►
Sign in to add a comment |
||
Comment 1 by wangxianzhu@chromium.org
, Nov 1