Skip to content

GitLab

T
tangled
Commit d7159f19 authored by Xianzhu Wang's avatar Xianzhu Wang Committed by Commit Bot
Browse files
Options

[PE] Detect clip and effect node change in their transform space etc. 

Previously, if a clip was between two transforms, and the clip result
was not tight, and the combination of the two transforms didn't change,
we missed raster invalidation for the changed effective clip.

Now check for change of LocalTransformSpace for clip and effect nodes.

Bug: 848782
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I1b60ecc2bc79cba69c667e188053b024de670361
Reviewed-on: https://chromium-review.googlesource.com/1094202
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: default avatarTien-Ren Chen <trchen@chromium.org>
Reviewed-by: default avatarChris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567045}
parent 8a4de1d2
Expand all Show whitespace changes
Inline Side-by-side
Showing with 640 additions and 132 deletions
third_party/WebKit/LayoutTests/paint/invalidation/clip/clip-between-changed-transforms-expected.html 0 → 100644
View file @ d7159f19
<!DOCTYPE html>
<div style="width: 0; height: 0; transform: rotate(5deg) translateX(200px)">
<div style="width: 200px; height: 200px; overflow: hidden">
<div style="width: 400px; height: 400px; background: blue; transform: translateX(-200px)">
</div>
</div>
</div>
third_party/WebKit/LayoutTests/paint/invalidation/clip/clip-between-changed-transforms.html 0 → 100644
View file @ d7159f19
<!DOCTYPE html>
<div id="outer" style="width: 0; height: 0; transform: rotate(5deg) translateX(100px)">
<div id="clip" style="width: 200px; height: 200px; overflow: hidden">
<div id="inner" style="width: 400px; height: 400px; background: blue; transform: translateX(-100px)">
</div>
</div>
</div>
<script src="../../../resources/run-after-layout-and-paint.js"></script>
<script>
runAfterLayoutAndPaint(function() {
// Change both outer and inner transforms, but keep the combined transform
// unchanged. We should invalidate because the effective clip changes
// between the transforms.
outer.style.transform = "rotate(5deg) translateX(200px)";
inner.style.transform = "translateX(-200px)";
}, true);
</script>
third_party/blink/renderer/platform/graphics/paint/clip_paint_property_node.cc
View file @ d7159f19
......@@ -5,6 +5,7 @@
#include "third_party/blink/renderer/platform/graphics/paint/clip_paint_property_node.h"
#include "third_party/blink/renderer/platform/geometry/layout_rect.h"
#include "third_party/blink/renderer/platform/graphics/paint/property_tree_state.h"
namespace blink {
......@@ -17,10 +18,27 @@ const ClipPaintPropertyNode& ClipPaintPropertyNode::Root() {
return *root;
}
bool ClipPaintPropertyNode::Changed(
const PropertyTreeState& relative_to_state,
const TransformPaintPropertyNode* transform_not_to_check) const {
for (const auto* node = this; node && node != relative_to_state.Clip();
node = node->Parent()) {
if (node->NodeChanged())
return true;
if (node->LocalTransformSpace() != transform_not_to_check &&
node->LocalTransformSpace()->Changed(*relative_to_state.Transform()))
return true;
}
return false;
}
std::unique_ptr<JSONObject> ClipPaintPropertyNode::ToJSON() const {
auto json = JSONObject::Create();
if (Parent())
json->SetString("parent", String::Format("%p", Parent()));
if (NodeChanged())
json->SetBoolean("changed", true);
json->SetString("localTransformSpace",
String::Format("%p", state_.local_transform_space.get()));
json->SetString("rect", state_.clip_rect.ToString());
......
third_party/blink/renderer/platform/graphics/paint/clip_paint_property_node.h
View file @ d7159f19
......@@ -17,6 +17,7 @@
namespace blink {
class GeometryMapperClipCache;
class PropertyTreeState;
// A clip rect created by a css property such as "overflow" or "clip".
// Along with a reference to the transform space the clip rect is based on,
......@@ -72,6 +73,15 @@ class PLATFORM_EXPORT ClipPaintPropertyNode
return true;
}
// Checks if the accumulated clip from |this| to |relative_to_state.Clip()|
// has changed in the space of |relative_to_state.Transform()|. We check for
// changes of not only clip nodes, but also LocalTransformSpace relative to
// |relative_to_state.Transform()| of the clip nodes. |transform_not_to_check|
// specifies a transform node that the caller has checked or will check its
// change in other ways and this function should treat it as unchanged.
bool Changed(const PropertyTreeState& relative_to_state,
const TransformPaintPropertyNode* transform_not_to_check) const;
bool EqualIgnoringHitTestRects(const ClipPaintPropertyNode* parent,
const State& state) const {
return parent == Parent() && state_.EqualIgnoringHitTestRects(state);
......
third_party/blink/renderer/platform/graphics/paint/effect_paint_property_node.cc
View file @ d7159f19
......@@ -4,6 +4,8 @@
#include "third_party/blink/renderer/platform/graphics/paint/effect_paint_property_node.h"
#include "third_party/blink/renderer/platform/graphics/paint/property_tree_state.h"
namespace blink {
const EffectPaintPropertyNode& EffectPaintPropertyNode::Root() {
......@@ -22,10 +24,30 @@ FloatRect EffectPaintPropertyNode::MapRect(const FloatRect& input_rect) const {
return result;
}
bool EffectPaintPropertyNode::Changed(
const PropertyTreeState& relative_to_state,
const TransformPaintPropertyNode* transform_not_to_check) const {
for (const auto* node = this; node && node != relative_to_state.Effect();
node = node->Parent()) {
if (node->NodeChanged())
return true;
if (node->HasFilterThatMovesPixels() &&
node->LocalTransformSpace() != transform_not_to_check &&
node->LocalTransformSpace()->Changed(*relative_to_state.Transform()))
return true;
// We don't check for change of OutputClip here to avoid N^3 complexity.
// The caller should check for clip change in other ways.
}
return false;
}
std::unique_ptr<JSONObject> EffectPaintPropertyNode::ToJSON() const {
auto json = JSONObject::Create();
if (Parent())
json->SetString("parent", String::Format("%p", Parent()));
if (NodeChanged())
json->SetBoolean("changed", true);
json->SetString("localTransformSpace",
String::Format("%p", state_.local_transform_space.get()));
json->SetString("outputClip", String::Format("%p", state_.output_clip.get()));
......
third_party/blink/renderer/platform/graphics/paint/effect_paint_property_node.h
View file @ d7159f19
......@@ -16,6 +16,8 @@
namespace blink {
class PropertyTreeState;
// Effect nodes are abstraction of isolated groups, along with optional effects
// that can be applied to the composited output of the group.
//
......@@ -82,6 +84,17 @@ class PLATFORM_EXPORT EffectPaintPropertyNode
return true;
}
// Checks if the accumulated effect from |this| to |relative_to_state
// .Effect()| has changed in the space of |relative_to_state.Transform()|.
// We check for changes of not only effect nodes, but also LocalTransformSpace
// relative to |relative_to_state.Transform()| of the effect nodes having
// filters that move pixels. Change of OutputClip is not checked and the
// caller should check in other ways. |transform_not_to_check| specifies the
// transform node that the caller has checked or will check its change in
// other ways and this function should treat it as unchanged.
bool Changed(const PropertyTreeState& relative_to_state,
const TransformPaintPropertyNode* transform_not_to_check) const;
const TransformPaintPropertyNode* LocalTransformSpace() const {
return state_.local_transform_space.get();
}
......
third_party/blink/renderer/platform/graphics/paint/paint_property_node.h
View file @ d7159f19
......@@ -69,30 +69,6 @@ class PaintPropertyNode : public RefCounted<NodeType> {
return true;
}
// TODO(wangxianzhu): Changed() and ClearChangedToRoot() are inefficient
// due to the tree walks. Optimize this if this affects overall performance.
// Returns true if any node (excluding the lowest common ancestor of
// |relative_to_node| and |this|) is marked changed along the shortest path
// from |this| to |relative_to_node|.
bool Changed(const NodeType& relative_to_node) const {
if (this == &relative_to_node)
return false;
bool changed = false;
for (const auto* n = this; n; n = n->Parent()) {
if (n == &relative_to_node)
return changed;
if (n->changed_)
changed = true;
}
// We reach here if |relative_to_node| is not an ancestor of |this|.
const auto& lca = LowestCommonAncestor(static_cast<const NodeType&>(*this),
relative_to_node);
return Changed(lca) || relative_to_node.Changed(lca);
}
void ClearChangedToRoot() const {
for (auto* n = this; n; n = n->Parent())
n->changed_ = false;
......@@ -129,8 +105,11 @@ class PaintPropertyNode : public RefCounted<NodeType> {
}
void SetChanged() { changed_ = true; }
bool NodeChanged() const { return changed_; }
private:
friend class PaintPropertyNodeTest;
scoped_refptr<const NodeType> parent_;
mutable bool changed_ = true;
......
third_party/blink/renderer/platform/graphics/paint/raster_invalidator.cc
View file @ d7159f19
......@@ -75,7 +75,8 @@ PaintInvalidationReason RasterInvalidator::ChunkPropertiesChanged(
// different, or the effect node's value changed between the layer state and
// the chunk state.
if (new_chunk_state.Effect() != old_chunk.effect_state ||
new_chunk_state.Effect()->Changed(*layer_state.Effect()))
new_chunk_state.Effect()->Changed(layer_state,
new_chunk_state.Transform()))
return PaintInvalidationReason::kPaintProperty;
// Check for accumulated clip rect change, if the clip rects are tight.
......@@ -94,7 +95,7 @@ PaintInvalidationReason RasterInvalidator::ChunkPropertiesChanged(
// different, or the clip node's value changed between the layer state and the
// chunk state.
if (new_chunk_state.Clip() != old_chunk.clip_state ||
new_chunk_state.Clip()->Changed(*layer_state.Clip()))
new_chunk_state.Clip()->Changed(layer_state, new_chunk_state.Transform()))
return PaintInvalidationReason::kPaintProperty;
return PaintInvalidationReason::kNone;
......
third_party/blink/renderer/platform/graphics/paint/transform_paint_property_node.cc
View file @ d7159f19
......@@ -31,10 +31,26 @@ TransformPaintPropertyNode::NearestScrollTranslationNode() const {
return *transform;
}
bool TransformPaintPropertyNode::Changed(
const TransformPaintPropertyNode& relative_to_node) const {
for (const auto* node = this; node; node = node->Parent()) {
if (node == &relative_to_node)
return false;
if (node->NodeChanged())
return true;
}
// |this| is not a descendant of |relative_to_node|. We have seen no changed
// flag from |this| to the root. Now check |relative_to_node| to the root.
return relative_to_node.Changed(Root());
}
std::unique_ptr<JSONObject> TransformPaintPropertyNode::ToJSON() const {
auto json = JSONObject::Create();
if (Parent())
json->SetString("parent", String::Format("%p", Parent()));
if (NodeChanged())
json->SetBoolean("changed", true);
if (!state_.matrix.IsIdentity())
json->SetString("matrix", state_.matrix.ToString());
if (!state_.matrix.IsIdentityOrTranslation())
......
third_party/blink/renderer/platform/graphics/paint/transform_paint_property_node.h
View file @ d7159f19
......@@ -83,6 +83,12 @@ class PLATFORM_EXPORT TransformPaintPropertyNode
return true;
}
// If |relative_to_node| is an ancestor of |this|, returns true if any node is
// marked changed along the path from |this| to |relative_to_node| (not
// included). Otherwise returns the combined changed status of the paths
// from |this| and |relative_to_node| to the root.
bool Changed(const TransformPaintPropertyNode& relative_to_node) const;
const TransformationMatrix& Matrix() const { return state_.matrix; }
const FloatPoint3D& Origin() const { return state_.origin; }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment