From c1c6d8f65f520fa244702287813ddb42cf25e85d Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Mon, 6 Feb 2023 19:13:34 +0100 Subject: [PATCH] Use DisplayListMatrixClipTracker in DiffContext --- .../display_list_matrix_clip_tracker.h | 2 +- flow/diff_context.cc | 84 ++++++++++++------- flow/diff_context.h | 48 ++++++----- flow/layers/backdrop_filter_layer.cc | 4 +- .../layers/backdrop_filter_layer_unittests.cc | 1 + flow/layers/clip_shape_layer.h | 3 +- flow/layers/color_filter_layer.cc | 3 +- flow/layers/display_list_layer.cc | 3 +- flow/layers/image_filter_layer.cc | 5 +- flow/layers/opacity_layer.cc | 3 +- flow/layers/physical_shape_layer.cc | 2 +- flow/layers/shader_mask_layer.cc | 3 +- 12 files changed, 92 insertions(+), 69 deletions(-) diff --git a/display_list/display_list_matrix_clip_tracker.h b/display_list/display_list_matrix_clip_tracker.h index 1bde44a3d9f08..563d489cecc62 100644 --- a/display_list/display_list_matrix_clip_tracker.h +++ b/display_list/display_list_matrix_clip_tracker.h @@ -23,7 +23,7 @@ class DisplayListMatrixClipTracker { DisplayListMatrixClipTracker(const SkRect& cull_rect, const SkMatrix& matrix); DisplayListMatrixClipTracker(const SkRect& cull_rect, const SkM44& matrix); - bool using_4x4_matrix() { return current_->is_4x4(); } + bool using_4x4_matrix() const { return current_->is_4x4(); } SkM44 matrix_4x4() const { return current_->matrix_4x4(); } SkMatrix matrix_3x3() const { return current_->matrix_3x3(); } diff --git a/flow/diff_context.cc b/flow/diff_context.cc index a828dcef968d2..9612cef549d2d 100644 --- a/flow/diff_context.cc +++ b/flow/diff_context.cc @@ -12,7 +12,8 @@ DiffContext::DiffContext(SkISize frame_size, PaintRegionMap& this_frame_paint_region_map, const PaintRegionMap& last_frame_paint_region_map, bool has_raster_cache) - : rects_(std::make_shared>()), + : clip_tracker_(DisplayListMatrixClipTracker(kGiantRect, SkMatrix::I())), + rects_(std::make_shared>()), frame_size_(frame_size), frame_device_pixel_ratio_(frame_device_pixel_ratio), this_frame_paint_region_map_(this_frame_paint_region_map), @@ -21,12 +22,18 @@ DiffContext::DiffContext(SkISize frame_size, void DiffContext::BeginSubtree() { state_stack_.push_back(state_); - state_.rect_index_ = rects_->size(); + + bool had_integral_transform = state_.integral_transform; + state_.rect_index = rects_->size(); state_.has_filter_bounds_adjustment = false; state_.has_texture = false; - if (state_.transform_override) { - state_.transform = *state_.transform_override; - state_.transform_override = std::nullopt; + state_.integral_transform = false; + + state_.clip_tracker_save_count = clip_tracker_.getSaveCount(); + clip_tracker_.save(); + + if (had_integral_transform) { + MakeCurrentTransformIntegral(); } } @@ -35,23 +42,33 @@ void DiffContext::EndSubtree() { if (state_.has_filter_bounds_adjustment) { filter_bounds_adjustment_stack_.pop_back(); } + clip_tracker_.restoreToCount(state_.clip_tracker_save_count); state_ = state_stack_.back(); state_stack_.pop_back(); } DiffContext::State::State() : dirty(false), - cull_rect(kGiantRect), - rect_index_(0), + rect_index(0), + integral_transform(false), + clip_tracker_save_count(0), has_filter_bounds_adjustment(false), has_texture(false) {} void DiffContext::PushTransform(const SkMatrix& transform) { - state_.transform.preConcat(transform); + clip_tracker_.transform(transform); } -void DiffContext::SetTransform(const SkMatrix& transform) { - state_.transform_override = transform; +void DiffContext::MakeCurrentTransformIntegral() { + // TODO(knopp): This is duplicated from LayerStack. Maybe should be part of + // clip tracker? + if (clip_tracker_.using_4x4_matrix()) { + clip_tracker_.setTransform( + RasterCacheUtil::GetIntegralTransCTM(clip_tracker_.matrix_4x4())); + } else { + clip_tracker_.setTransform( + RasterCacheUtil::GetIntegralTransCTM(clip_tracker_.matrix_3x3())); + } } void DiffContext::PushFilterBoundsAdjustment( @@ -128,21 +145,23 @@ Damage DiffContext::ComputeDamage(const SkIRect& accumulated_buffer_damage, return res; } +SkRect DiffContext::MapRect(const SkRect& rect) { + SkRect mapped_rect(rect); + clip_tracker_.mapRect(&mapped_rect); + return mapped_rect; +} + bool DiffContext::PushCullRect(const SkRect& clip) { - SkRect cull_rect = state_.transform.mapRect(clip); - return state_.cull_rect.intersect(cull_rect); + clip_tracker_.clipRect(clip, SkClipOp::kIntersect, false); + return !clip_tracker_.device_cull_rect().isEmpty(); +} + +SkMatrix DiffContext::GetTransform3x3() const { + return clip_tracker_.matrix_3x3(); } SkRect DiffContext::GetCullRect() const { - SkMatrix inverse_transform; - // Perspective projections don't produce rectangles that are useful for - // culling for some reason. - if (!state_.transform.hasPerspective() && - state_.transform.invert(&inverse_transform)) { - return inverse_transform.mapRect(state_.cull_rect); - } else { - return kGiantRect; - } + return clip_tracker_.local_cull_rect(); } void DiffContext::MarkSubtreeDirty(const PaintRegion& previous_paint_region) { @@ -163,16 +182,17 @@ void DiffContext::AddLayerBounds(const SkRect& rect) { // During painting we cull based on non-overriden transform and then // override the transform right before paint. Do the same thing here to get // identical paint rect. - auto transformed_rect = - ApplyFilterBoundsAdjustment(state_.transform.mapRect(rect)); - if (transformed_rect.intersects(state_.cull_rect)) { - auto paint_rect = state_.transform_override - ? ApplyFilterBoundsAdjustment( - state_.transform_override->mapRect(rect)) - : transformed_rect; - rects_->push_back(paint_rect); + auto transformed_rect = ApplyFilterBoundsAdjustment(MapRect(rect)); + if (transformed_rect.intersects(clip_tracker_.device_cull_rect())) { + if (state_.integral_transform) { + clip_tracker_.save(); + MakeCurrentTransformIntegral(); + transformed_rect = ApplyFilterBoundsAdjustment(MapRect(rect)); + clip_tracker_.restore(); + } + rects_->push_back(transformed_rect); if (IsSubtreeDirty()) { - AddDamage(paint_rect); + AddDamage(transformed_rect); } } } @@ -208,8 +228,8 @@ void DiffContext::AddReadbackRegion(const SkIRect& rect) { PaintRegion DiffContext::CurrentSubtreeRegion() const { bool has_readback = std::any_of( readbacks_.begin(), readbacks_.end(), - [&](const Readback& r) { return r.position >= state_.rect_index_; }); - return PaintRegion(rects_, state_.rect_index_, rects_->size(), has_readback, + [&](const Readback& r) { return r.position >= state_.rect_index; }); + return PaintRegion(rects_, state_.rect_index, rects_->size(), has_readback, state_.has_texture); } diff --git a/flow/diff_context.h b/flow/diff_context.h index 6688bdd5549f0..a8361ea432ef9 100644 --- a/flow/diff_context.h +++ b/flow/diff_context.h @@ -9,8 +9,8 @@ #include #include #include +#include "display_list/display_list_matrix_clip_tracker.h" #include "flutter/flow/paint_region.h" -#include "flutter/fml/logging.h" #include "flutter/fml/macros.h" #include "third_party/skia/include/core/SkMatrix.h" #include "third_party/skia/include/core/SkRect.h" @@ -83,16 +83,16 @@ class DiffContext { // subtree will have bounds adjusted by this function. void PushFilterBoundsAdjustment(const FilterBoundsAdjustment& filter); - // Returns transform matrix for current subtree - const SkMatrix& GetTransform() const { return state_.transform; } + // Instruct DiffContext that current layer will paint with integral transform. + void WillPaintWithIntegralTransform() { state_.integral_transform = true; } - // Overrides transform matrix for current subtree - void SetTransform(const SkMatrix& transform); + // Returns current transform as SkMatrix. + SkMatrix GetTransform3x3() const; - // Return cull rect for current subtree (in local coordinates) + // Return cull rect for current subtree (in local coordinates). SkRect GetCullRect() const; - // Sets the dirty flag on current subtree; + // Sets the dirty flag on current subtree. // // previous_paint_region, which should represent region of previous subtree // at this level will be added to damage area. @@ -196,24 +196,29 @@ class DiffContext { Statistics& statistics() { return statistics_; } + SkRect MapRect(const SkRect& rect); + private: struct State { State(); bool dirty; - SkRect cull_rect; // in screen coordinates - - // In order to replicate paint process closely, we need both the original - // transform, and the overriden transform (set for layers that need to paint - // on integer coordinates). The reason for this is that during paint the - // transform matrix is overriden only after layer passes the cull check - // first (with original transform). So to cull layer we use transform, but - // to get paint coordinates we use transform_override. Child layers are - // painted after transform override, so if set we use transform_override as - // base when diffing child layers. - SkMatrix transform; - std::optional transform_override; - size_t rect_index_; + + size_t rect_index; + + // In order to replicate paint process closely, DiffContext needs to take + // into account that some layers are painted with transform translation + // snapped to integral coordinates. + // + // It's not possible to simply snap the transform itself, because culling + // needs to happen with original (unsnapped) transform, just like it does + // during paint. This means the integral coordinates must be applied after + // culling before painting the layer content (either the layer itself, or + // when starting subtree to paint layer children). + bool integral_transform; + + // Used to restoring clip tracker when popping state. + int clip_tracker_save_count; // Whether this subtree has filter bounds adjustment function. If so, // it will need to be removed from stack when subtree is closed. @@ -223,6 +228,9 @@ class DiffContext { bool has_texture; }; + void MakeCurrentTransformIntegral(); + + DisplayListMatrixClipTracker clip_tracker_; std::shared_ptr> rects_; State state_; SkISize frame_size_; diff --git a/flow/layers/backdrop_filter_layer.cc b/flow/layers/backdrop_filter_layer.cc index 7091e4f5c69dd..cd4a1dffb357e 100644 --- a/flow/layers/backdrop_filter_layer.cc +++ b/flow/layers/backdrop_filter_layer.cc @@ -26,11 +26,11 @@ void BackdropFilterLayer::Diff(DiffContext* context, const Layer* old_layer) { context->AddLayerBounds(paint_bounds); if (filter_) { - context->GetTransform().mapRect(&paint_bounds); + paint_bounds = context->MapRect(paint_bounds); auto filter_target_bounds = paint_bounds.roundOut(); SkIRect filter_input_bounds; // in screen coordinates filter_->get_input_device_bounds( - filter_target_bounds, context->GetTransform(), filter_input_bounds); + filter_target_bounds, context->GetTransform3x3(), filter_input_bounds); context->AddReadbackRegion(filter_input_bounds); } diff --git a/flow/layers/backdrop_filter_layer_unittests.cc b/flow/layers/backdrop_filter_layer_unittests.cc index 0697682e44cc5..e335544f3e96b 100644 --- a/flow/layers/backdrop_filter_layer_unittests.cc +++ b/flow/layers/backdrop_filter_layer_unittests.cc @@ -383,6 +383,7 @@ TEST_F(BackdropLayerDiffTest, BackdropLayer) { auto path1 = SkPath().addRect(SkRect::MakeLTRB(180, 180, 190, 190)); l4.root()->Add(std::make_shared(path1)); damage = DiffLayerTree(l4, l3); + EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(180, 180, 190, 190)); MockLayerTree l5; diff --git a/flow/layers/clip_shape_layer.h b/flow/layers/clip_shape_layer.h index b8c6ddbf68e7d..591d4b22237d5 100644 --- a/flow/layers/clip_shape_layer.h +++ b/flow/layers/clip_shape_layer.h @@ -33,8 +33,7 @@ class ClipShapeLayer : public CacheableContainerLayer { } } if (UsesSaveLayer() && context->has_raster_cache()) { - context->SetTransform( - RasterCacheUtil::GetIntegralTransCTM(context->GetTransform())); + context->WillPaintWithIntegralTransform(); } if (context->PushCullRect(clip_shape_bounds())) { DiffChildren(context, prev); diff --git a/flow/layers/color_filter_layer.cc b/flow/layers/color_filter_layer.cc index a5966e4542c36..ee19cc3f56461 100644 --- a/flow/layers/color_filter_layer.cc +++ b/flow/layers/color_filter_layer.cc @@ -28,8 +28,7 @@ void ColorFilterLayer::Diff(DiffContext* context, const Layer* old_layer) { } if (context->has_raster_cache()) { - context->SetTransform( - RasterCacheUtil::GetIntegralTransCTM(context->GetTransform())); + context->WillPaintWithIntegralTransform(); } DiffChildren(context, prev); diff --git a/flow/layers/display_list_layer.cc b/flow/layers/display_list_layer.cc index cf2b314d8e359..7ba4feb7276dc 100644 --- a/flow/layers/display_list_layer.cc +++ b/flow/layers/display_list_layer.cc @@ -53,8 +53,7 @@ void DisplayListLayer::Diff(DiffContext* context, const Layer* old_layer) { } context->PushTransform(SkMatrix::Translate(offset_.x(), offset_.y())); if (context->has_raster_cache()) { - context->SetTransform( - RasterCacheUtil::GetIntegralTransCTM(context->GetTransform())); + context->WillPaintWithIntegralTransform(); } context->AddLayerBounds(display_list()->bounds()); context->SetLayerPaintRegion(this, context->CurrentSubtreeRegion()); diff --git a/flow/layers/image_filter_layer.cc b/flow/layers/image_filter_layer.cc index 456ada3afcd0d..b759ee97005bb 100644 --- a/flow/layers/image_filter_layer.cc +++ b/flow/layers/image_filter_layer.cc @@ -29,12 +29,11 @@ void ImageFilterLayer::Diff(DiffContext* context, const Layer* old_layer) { context->PushTransform(SkMatrix::Translate(offset_.fX, offset_.fY)); if (context->has_raster_cache()) { - context->SetTransform( - RasterCacheUtil::GetIntegralTransCTM(context->GetTransform())); + context->WillPaintWithIntegralTransform(); } if (filter_) { - auto filter = filter_->makeWithLocalMatrix(context->GetTransform()); + auto filter = filter_->makeWithLocalMatrix(context->GetTransform3x3()); if (filter) { // This transform will be applied to every child rect in the subtree context->PushFilterBoundsAdjustment([filter](SkRect rect) { diff --git a/flow/layers/opacity_layer.cc b/flow/layers/opacity_layer.cc index 71a6073f6f331..3636af4e569bb 100644 --- a/flow/layers/opacity_layer.cc +++ b/flow/layers/opacity_layer.cc @@ -29,8 +29,7 @@ void OpacityLayer::Diff(DiffContext* context, const Layer* old_layer) { } context->PushTransform(SkMatrix::Translate(offset_.fX, offset_.fY)); if (context->has_raster_cache()) { - context->SetTransform( - RasterCacheUtil::GetIntegralTransCTM(context->GetTransform())); + context->WillPaintWithIntegralTransform(); } DiffChildren(context, prev); context->SetLayerPaintRegion(this, context->CurrentSubtreeRegion()); diff --git a/flow/layers/physical_shape_layer.cc b/flow/layers/physical_shape_layer.cc index 77f347271e28e..172ac41b8c840 100644 --- a/flow/layers/physical_shape_layer.cc +++ b/flow/layers/physical_shape_layer.cc @@ -38,7 +38,7 @@ void PhysicalShapeLayer::Diff(DiffContext* context, const Layer* old_layer) { } else { bounds = DisplayListCanvasDispatcher::ComputeShadowBounds( path_, elevation_, context->frame_device_pixel_ratio(), - context->GetTransform()); + context->GetTransform3x3()); } context->AddLayerBounds(bounds); diff --git a/flow/layers/shader_mask_layer.cc b/flow/layers/shader_mask_layer.cc index d22c41b1b27f6..a460760c7a668 100644 --- a/flow/layers/shader_mask_layer.cc +++ b/flow/layers/shader_mask_layer.cc @@ -27,8 +27,7 @@ void ShaderMaskLayer::Diff(DiffContext* context, const Layer* old_layer) { } } if (context->has_raster_cache()) { - context->SetTransform( - RasterCacheUtil::GetIntegralTransCTM(context->GetTransform())); + context->WillPaintWithIntegralTransform(); } DiffChildren(context, prev);