Skip to content

Commit

Permalink
Use clip tree to map visual rect across pixel-moving filters
Browse files Browse the repository at this point in the history
The new method may slightly improve performance of visual rect mapping
between property tree states with different effect states. We still
loop through PixelMovingFilterClipExpander clip nodes with cached
nearest nodes in GeometryMapperClipCache.

This prepares for the scroll update optimization for non-stacking-
context scrollers. When adjusting property tree state of a scrolling
contents to above the scroller, we don't need to adjust the effect
state because the clip tree and transform tree contain all needed
information for visual rect mapping.

ClusterTelemetry jobs show no obvious performance change for
rasterize_and_record_micro benchmark, except for a 2% progression
for raster_invalidation_and_convert_time on Linux:
Android: https://ct.skia.org/results/cluster-telemetry/tasks/chromium_perf_runs/wangxianzhu-ChromiumPerf-6689/html/index.html
Linux: https://ct.skia.org/results/cluster-telemetry/tasks/chromium_perf_runs/wangxianzhu-ChromiumPerf-6688/html/index.html

Bug: 1348768
Change-Id: Ica5b407062e9ca689ba46946258f33f7ef9f7d0d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3803207
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1031226}
  • Loading branch information
wangxianzhu authored and Chromium LUCI CQ committed Aug 3, 2022
1 parent 11b77f2 commit fe8909b
Show file tree
Hide file tree
Showing 21 changed files with 198 additions and 155 deletions.
8 changes: 4 additions & 4 deletions third_party/blink/renderer/core/paint/fragment_data.cc
Expand Up @@ -96,10 +96,6 @@ const TransformPaintPropertyNodeOrAlias& FragmentData::ContentsTransform()

const ClipPaintPropertyNodeOrAlias& FragmentData::PreClip() const {
if (const auto* properties = PaintProperties()) {
if (const auto* clip = properties->PixelMovingFilterClipExpander()) {
DCHECK(clip->Parent());
return *clip->Parent();
}
if (const auto* clip = properties->ClipPathClip()) {
DCHECK(clip->Parent());
return *clip->Parent();
Expand All @@ -112,6 +108,10 @@ const ClipPaintPropertyNodeOrAlias& FragmentData::PreClip() const {
DCHECK(css_clip->Parent());
return *css_clip->Parent();
}
if (const auto* clip = properties->PixelMovingFilterClipExpander()) {
DCHECK(clip->Parent());
return *clip->Parent();
}
}
return LocalBorderBoxProperties().Clip();
}
Expand Down
4 changes: 4 additions & 0 deletions third_party/blink/renderer/core/paint/fragment_data_test.cc
Expand Up @@ -16,6 +16,7 @@ TEST_F(FragmentDataTest, PreClip) {
width: 400px; height: 400px; position: absolute;
clip: rect(0, 50px, 100px, 0);
clip-path: inset(0%);
filter: blur(10px);
}
</style>
<div id='target'></div>
Expand All @@ -26,6 +27,9 @@ TEST_F(FragmentDataTest, PreClip) {
target->FirstFragment().PaintProperties();
EXPECT_TRUE(properties->ClipPathClip());
EXPECT_TRUE(properties->CssClip());
EXPECT_TRUE(properties->PixelMovingFilterClipExpander());
EXPECT_EQ(properties->CssClip(),
properties->PixelMovingFilterClipExpander()->Parent());
EXPECT_EQ(properties->ClipPathClip(), properties->CssClip()->Parent());
EXPECT_EQ(properties->ClipPathClip()->Parent(),
&target->FirstFragment().PreClip());
Expand Down
Expand Up @@ -221,10 +221,6 @@ class CORE_EXPORT ObjectPaintProperties {
// [ FragmentClip ]
// | Clips to a fragment's bounds.
// | This is only present for content under a fragmentation container.
// +-[ PixelMovingFilterClipExpander ]
// | Clip created by pixel-moving filter. Instead of intersecting with the
// / current clip, this clip expands the current clip to include all pixels
// / in the filtered content that may affect the pixels in the current clip.
// +-[ ClipPathClip ]
// | Clip created by path-based CSS clip-path. Only exists if the
// / clip-path is "simple" that can be applied geometrically. This and
Expand All @@ -245,6 +241,11 @@ class CORE_EXPORT ObjectPaintProperties {
// +-[ OverflowControlsClip ]
// | Clip created by overflow clip to clip overflow controls
// | (scrollbars, resizer, scroll corner) that would overflow the box.
// +-[ PixelMovingFilterClipExpander ]
// | Clip created by pixel-moving filter. Instead of intersecting with
// | the current clip, this clip expands the current clip to include all
// / pixels in the filtered content that may affect the pixels in the
// / current clip.
// +-[ InnerBorderRadiusClip ]
// | Clip created by a rounded border with overflow clip. This clip is
// | not inset by scrollbars.
Expand Down
5 changes: 2 additions & 3 deletions third_party/blink/renderer/core/paint/paint_layer_clipper.cc
Expand Up @@ -356,9 +356,8 @@ PhysicalRect PaintLayerClipper::LocalVisualRect(
// overflow induced by paint, prior to applying filters. This function is
// expected the return the final visual rect after filtering.
if (layer_->PaintsWithFilters() &&
// If we use GeometryMapper to map to an ancestor layer, GeometryMapper
// will handle filter effects.
(!use_geometry_mapper_ || context.root_layer == layer_)) {
// GeometryMapper will handle filter effects.
!use_geometry_mapper_) {
layer_bounds_with_visual_overflow =
layer_->MapRectForFilter(layer_bounds_with_visual_overflow);
}
Expand Down
Expand Up @@ -1798,7 +1798,7 @@ void FragmentPaintPropertyTreeBuilder::UpdateFilter() {
OnUpdateClip(properties_->UpdatePixelMovingFilterClipExpander(
*context_.current.clip,
ClipPaintPropertyNode::State(context_.current.transform,
properties_->Filter())));
*properties_->Filter())));
} else {
OnClearClip(properties_->ClearPixelMovingFilterClipExpander());
}
Expand Down
Expand Up @@ -113,11 +113,11 @@ class PropertyTreePrinterTraits<ClipPaintPropertyNodeOrAlias> {
const ObjectPaintProperties& properties,
PropertyTreePrinter<ClipPaintPropertyNodeOrAlias>& printer) {
printer.AddNode(properties.FragmentClip());
printer.AddNode(properties.PixelMovingFilterClipExpander());
printer.AddNode(properties.ClipPathClip());
printer.AddNode(properties.MaskClip());
printer.AddNode(properties.CssClip());
printer.AddNode(properties.CssClipFixedPosition());
printer.AddNode(properties.PixelMovingFilterClipExpander());
printer.AddNode(properties.OverflowControlsClip());
printer.AddNode(properties.InnerBorderRadiusClip());
printer.AddNode(properties.OverflowClip());
Expand Down Expand Up @@ -250,13 +250,13 @@ void UpdateDebugNames(const LayoutObject& object,
object);

SetDebugName(properties.FragmentClip(), "FragmentClip", object);
SetDebugName(properties.PixelMovingFilterClipExpander(),
"PixelMovingFilterClip", object);
SetDebugName(properties.ClipPathClip(), "ClipPathClip", object);
SetDebugName(properties.MaskClip(), "MaskClip", object);
SetDebugName(properties.CssClip(), "CssClip", object);
SetDebugName(properties.CssClipFixedPosition(), "CssClipFixedPosition",
object);
SetDebugName(properties.PixelMovingFilterClipExpander(),
"PixelMovingFilterClip", object);
SetDebugName(properties.OverflowControlsClip(), "OverflowControlsClip",
object);
SetDebugName(properties.InnerBorderRadiusClip(), "InnerBorderRadiusClip",
Expand Down
Expand Up @@ -42,31 +42,20 @@ void ChunkToLayerMapper::SwitchToChunk(const PaintChunk& chunk) {
-layer_offset_.y());
}

bool new_has_filter_that_moves_pixels = has_filter_that_moves_pixels_;
if (&new_chunk_state.Effect() != &chunk_state_.Effect()) {
new_has_filter_that_moves_pixels = false;
for (const auto* effect = &new_chunk_state.Effect();
effect && effect != &layer_state_.Effect();
effect = effect->UnaliasedParent()) {
if (effect->HasFilterThatMovesPixels()) {
new_has_filter_that_moves_pixels = true;
break;
}
}
}
has_filter_that_moves_pixels_ =
new_chunk_state.Clip().NearestPixelMovingFilterClip() !=
layer_state_.Clip().NearestPixelMovingFilterClip();

bool needs_clip_recalculation =
new_has_filter_that_moves_pixels != has_filter_that_moves_pixels_ ||
&new_chunk_state.Clip() != &chunk_state_.Clip();
if (needs_clip_recalculation) {
if (has_filter_that_moves_pixels_) {
clip_rect_ = InfiniteLooseFloatClipRect();
} else if (&new_chunk_state.Clip() != &chunk_state_.Clip()) {
clip_rect_ =
GeometryMapper::LocalToAncestorClipRect(new_chunk_state, layer_state_);
if (!clip_rect_.IsInfinite())
clip_rect_.Move(-layer_offset_);
}

chunk_state_ = new_chunk_state;
has_filter_that_moves_pixels_ = new_has_filter_that_moves_pixels;
}

gfx::Rect ChunkToLayerMapper::MapVisualRect(const gfx::Rect& rect) const {
Expand Down
Expand Up @@ -162,14 +162,16 @@ TEST_F(ChunkToLayerMapperTest, SlowPath) {
CompositorFilterOperations filter2;
filter2.AppendBlurFilter(20);
auto effect2 = CreateFilterEffect(LayerState().Effect(), std::move(filter2));
auto chunk2 = Chunk(PropertyTreeState(LayerState().Transform(),
LayerState().Clip(), *effect2));
auto clip_expander =
CreatePixelMovingFilterClipExpander(LayerState().Clip(), *effect2);
auto chunk2 = Chunk(
PropertyTreeState(LayerState().Transform(), *clip_expander, *effect2));

// Chunk3 has a different effect which inherits from chunk2's effect.
// Should use the slow path.
auto effect3 = CreateOpacityEffect(*effect2, 1.f);
auto chunk3 = Chunk(PropertyTreeState(LayerState().Transform(),
LayerState().Clip(), *effect3));
auto chunk3 = Chunk(
PropertyTreeState(LayerState().Transform(), *clip_expander, *effect3));

// Chunk4 has an opacity filter effect which inherits from the layer's effect.
// Should use the fast path.
Expand Down
Expand Up @@ -4749,8 +4749,7 @@ TEST_P(PaintArtifactCompositorTest,
filter_op.AppendBlurFilter(5);
auto filter =
CreateFilterEffect(e0(), filter_op, CompositingReason::kWillChangeFilter);
auto clip_expander = ClipPaintPropertyNode::Create(
c0(), ClipPaintPropertyNode::State(&t0(), filter.get()));
auto clip_expander = CreatePixelMovingFilterClipExpander(c0(), *filter);

Update(TestPaintArtifact()
.Chunk(t0(), *clip_expander, *filter)
Expand All @@ -4769,8 +4768,7 @@ TEST_P(PaintArtifactCompositorTest,
CompositorFilterOperations filter_op;
filter_op.AppendBlurFilter(5);
auto filter = CreateFilterEffect(e0(), filter_op);
auto clip_expander = ClipPaintPropertyNode::Create(
c0(), ClipPaintPropertyNode::State(&t0(), filter.get()));
auto clip_expander = CreatePixelMovingFilterClipExpander(c0(), *filter);

EffectPaintPropertyNode::State mask_state;
mask_state.local_transform_space = &t0();
Expand Down
Expand Up @@ -374,11 +374,14 @@ void ConversionContext::SwitchToClip(const ClipPaintPropertyNode& target_clip) {
// This should never happen unless the DCHECK in step 1 failed.
if (!clip)
break;
pending_clips.push_back(clip);
if (!clip->PixelMovingFilter())
pending_clips.push_back(clip);
}

if (pending_clips.IsEmpty())
return;

// Step 3: Now apply the list of clips in top-down order.
DCHECK(pending_clips.size());
auto pending_combined_clip_rect = pending_clips.back()->PaintClipRect();
const auto* lowest_combined_clip_node = pending_clips.back();
for (auto i = pending_clips.size() - 1; i--;) {
Expand Down
Expand Up @@ -1273,8 +1273,9 @@ TEST_P(PaintChunksToCcLayerTest, ReferenceFilterOnChunkWithDrawingDisplayItem) {
filter.SetReferenceBox(gfx::RectF(11, 22, 33, 44));
ASSERT_TRUE(filter.HasReferenceFilter());
auto e1 = CreateFilterEffect(e0(), t0(), &c0(), filter);
auto clip_expander = CreatePixelMovingFilterClipExpander(c0(), *e1);
TestChunks chunks;
chunks.AddChunk(t0(), c0(), *e1, gfx::Rect(5, 10, 200, 300),
chunks.AddChunk(t0(), *clip_expander, *e1, gfx::Rect(5, 10, 200, 300),
gfx::Rect(10, 15, 20, 30));

auto cc_list = base::MakeRefCounted<cc::DisplayItemList>(
Expand Down
Expand Up @@ -89,9 +89,9 @@ class PLATFORM_EXPORT ClipPaintPropertyNode
}
State(scoped_refptr<const TransformPaintPropertyNodeOrAlias>
local_transform_space,
const EffectPaintPropertyNode* pixel_moving_filter)
const EffectPaintPropertyNode& pixel_moving_filter)
: local_transform_space(std::move(local_transform_space)),
pixel_moving_filter(pixel_moving_filter) {
pixel_moving_filter(&pixel_moving_filter) {
DCHECK(layout_clip_rect_.IsInfinite());
paint_clip_rect_ = FloatRoundedRect(layout_clip_rect_.Rect());
}
Expand Down Expand Up @@ -179,6 +179,10 @@ class PLATFORM_EXPORT ClipPaintPropertyNode
return state_.pixel_moving_filter;
}

const ClipPaintPropertyNode* NearestPixelMovingFilterClip() const {
return GetClipCache().NearestPixelMovingFilterClip();
}

std::unique_ptr<JSONObject> ToJSON() const;

private:
Expand All @@ -203,20 +207,18 @@ class PLATFORM_EXPORT ClipPaintPropertyNode

// For access to GetClipCache();
friend class GeometryMapper;
friend class GeometryMapperClipCache;
friend class GeometryMapperTest;

GeometryMapperClipCache& GetClipCache() const {
return const_cast<ClipPaintPropertyNode*>(this)->GetClipCache();
}

GeometryMapperClipCache& GetClipCache() {
if (!clip_cache_)
clip_cache_.reset(new GeometryMapperClipCache());
return *clip_cache_.get();
clip_cache_ = std::make_unique<GeometryMapperClipCache>();
clip_cache_->UpdateIfNeeded(*this);
return *clip_cache_;
}

State state_;
std::unique_ptr<GeometryMapperClipCache> clip_cache_;
mutable std::unique_ptr<GeometryMapperClipCache> clip_cache_;
};

} // namespace blink
Expand Down

0 comments on commit fe8909b

Please sign in to comment.