From 07fc5a59f9f7e003fbb8a49a412e27338b02a225 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Mon, 21 Nov 2022 09:59:55 -0800 Subject: [PATCH] Add optional offset parameter to ImageFilterLayer (flutter/engine#36863) * add optional offset parameter to ImageFilterLayer * update unit test for LayerStateStack and fix HTML implementation --- .../flutter/flow/layers/image_filter_layer.cc | 14 ++++- .../flutter/flow/layers/image_filter_layer.h | 4 +- .../layers/image_filter_layer_unittests.cc | 53 ++++++++++++++++ engine/src/flutter/lib/ui/compositing.dart | 7 ++- .../lib/ui/compositing/scene_builder.cc | 6 +- .../lib/ui/compositing/scene_builder.h | 2 + .../flutter/lib/web_ui/lib/compositing.dart | 1 + .../lib/src/engine/canvaskit/layer.dart | 6 +- .../engine/canvaskit/layer_scene_builder.dart | 3 +- .../lib/src/engine/html/image_filter.dart | 60 +++++++++++++++++-- .../lib/src/engine/html/scene_builder.dart | 3 +- 11 files changed, 142 insertions(+), 17 deletions(-) diff --git a/engine/src/flutter/flow/layers/image_filter_layer.cc b/engine/src/flutter/flow/layers/image_filter_layer.cc index 79a3c4c273..f42f7240aa 100644 --- a/engine/src/flutter/flow/layers/image_filter_layer.cc +++ b/engine/src/flutter/flow/layers/image_filter_layer.cc @@ -9,9 +9,11 @@ namespace flutter { -ImageFilterLayer::ImageFilterLayer(std::shared_ptr filter) +ImageFilterLayer::ImageFilterLayer(std::shared_ptr filter, + const SkPoint& offset) : CacheableContainerLayer( RasterCacheUtil::kMinimumRendersBeforeCachingFilterLayer), + offset_(offset), filter_(std::move(filter)), transformed_filter_(nullptr) {} @@ -20,11 +22,12 @@ void ImageFilterLayer::Diff(DiffContext* context, const Layer* old_layer) { auto* prev = static_cast(old_layer); if (!context->IsSubtreeDirty()) { FML_DCHECK(prev); - if (NotEquals(filter_, prev->filter_)) { + if (NotEquals(filter_, prev->filter_) || offset_ != prev->offset_) { context->MarkSubtreeDirty(context->GetOldLayerPaintRegion(old_layer)); } } + context->PushTransform(SkMatrix::Translate(offset_.fX, offset_.fY)); if (context->has_raster_cache()) { context->SetTransform( RasterCacheUtil::GetIntegralTransCTM(context->GetTransform())); @@ -47,6 +50,9 @@ void ImageFilterLayer::Diff(DiffContext* context, const Layer* old_layer) { } void ImageFilterLayer::Preroll(PrerollContext* context) { + auto mutator = context->state_stack.save(); + mutator.translate(offset_); + Layer::AutoPrerollSaveLayerState save = Layer::AutoPrerollSaveLayerState::Create(context); @@ -73,7 +79,8 @@ void ImageFilterLayer::Preroll(PrerollContext* context) { SkIRect filter_out_bounds; filter_->map_device_bounds(filter_in_bounds, SkMatrix::I(), filter_out_bounds); - child_bounds = SkRect::Make(filter_out_bounds); + child_bounds.set(filter_out_bounds); + child_bounds.offset(offset_); set_paint_bounds(child_bounds); @@ -92,6 +99,7 @@ void ImageFilterLayer::Paint(PaintContext& context) const { FML_DCHECK(needs_painting(context)); auto mutator = context.state_stack.save(); + mutator.translate(offset_); if (context.raster_cache) { // Always apply the integral transform in the presence of a raster cache diff --git a/engine/src/flutter/flow/layers/image_filter_layer.h b/engine/src/flutter/flow/layers/image_filter_layer.h index 894b5d2e98..b8e64b8cc7 100644 --- a/engine/src/flutter/flow/layers/image_filter_layer.h +++ b/engine/src/flutter/flow/layers/image_filter_layer.h @@ -15,7 +15,8 @@ namespace flutter { class ImageFilterLayer : public CacheableContainerLayer { public: - explicit ImageFilterLayer(std::shared_ptr filter); + explicit ImageFilterLayer(std::shared_ptr filter, + const SkPoint& offset = SkPoint::Make(0, 0)); void Diff(DiffContext* context, const Layer* old_layer) override; @@ -24,6 +25,7 @@ class ImageFilterLayer : public CacheableContainerLayer { void Paint(PaintContext& context) const override; private: + SkPoint offset_; std::shared_ptr filter_; std::shared_ptr transformed_filter_; diff --git a/engine/src/flutter/flow/layers/image_filter_layer_unittests.cc b/engine/src/flutter/flow/layers/image_filter_layer_unittests.cc index 9cf39e2c5c..92b690a5bf 100644 --- a/engine/src/flutter/flow/layers/image_filter_layer_unittests.cc +++ b/engine/src/flutter/flow/layers/image_filter_layer_unittests.cc @@ -114,6 +114,59 @@ TEST_F(ImageFilterLayerTest, SimpleFilter) { EXPECT_TRUE(DisplayListsEQ_Verbose(display_list(), expected_display_list)); } +TEST_F(ImageFilterLayerTest, SimpleFilterWithOffset) { + const SkMatrix initial_transform = SkMatrix::Translate(0.5f, 1.0f); + const SkRect initial_cull_rect = SkRect::MakeLTRB(0, 0, 100, 100); + const SkRect child_bounds = SkRect::MakeLTRB(5.0f, 6.0f, 20.5f, 21.5f); + const SkPath child_path = SkPath().addRect(child_bounds); + const SkPaint child_paint = SkPaint(SkColors::kYellow); + const SkPoint layer_offset = SkPoint::Make(5.5, 6.5); + auto dl_image_filter = std::make_shared( + SkMatrix(), DlImageSampling::kMipmapLinear); + auto mock_layer = std::make_shared(child_path, child_paint); + auto layer = + std::make_shared(dl_image_filter, layer_offset); + layer->Add(mock_layer); + + SkMatrix child_matrix = initial_transform; + child_matrix.preTranslate(layer_offset.fX, layer_offset.fY); + const SkRect child_rounded_bounds = + SkRect::MakeLTRB(10.5f, 12.5f, 26.5f, 28.5f); + + preroll_context()->state_stack.set_preroll_delegate(initial_cull_rect, + initial_transform); + layer->Preroll(preroll_context()); + EXPECT_EQ(layer->paint_bounds(), child_rounded_bounds); + EXPECT_EQ(layer->child_paint_bounds(), child_bounds); + EXPECT_TRUE(layer->needs_painting(paint_context())); + EXPECT_EQ(mock_layer->parent_matrix(), child_matrix); + EXPECT_EQ(preroll_context()->state_stack.device_cull_rect(), + initial_cull_rect); + + DisplayListBuilder expected_builder; + /* ImageFilterLayer::Paint() */ { + expected_builder.save(); + { + expected_builder.translate(layer_offset.fX, layer_offset.fY); + DlPaint dl_paint; + dl_paint.setImageFilter(dl_image_filter.get()); + expected_builder.saveLayer(&child_bounds, &dl_paint); + { + /* MockLayer::Paint() */ { + expected_builder.drawPath(child_path, + DlPaint().setColor(DlColor::kYellow())); + } + } + expected_builder.restore(); + } + expected_builder.restore(); + } + auto expected_display_list = expected_builder.Build(); + + layer->Paint(display_list_paint_context()); + EXPECT_TRUE(DisplayListsEQ_Verbose(display_list(), expected_display_list)); +} + TEST_F(ImageFilterLayerTest, SimpleFilterBounds) { const SkMatrix initial_transform = SkMatrix::Translate(0.5f, 1.0f); const SkRect child_bounds = SkRect::MakeLTRB(5.0f, 6.0f, 20.5f, 21.5f); diff --git a/engine/src/flutter/lib/ui/compositing.dart b/engine/src/flutter/lib/ui/compositing.dart index 30104639e7..b039e67ff1 100644 --- a/engine/src/flutter/lib/ui/compositing.dart +++ b/engine/src/flutter/lib/ui/compositing.dart @@ -520,6 +520,7 @@ class SceneBuilder extends NativeFieldWrapperClass1 { /// See [pop] for details about the operation stack. ImageFilterEngineLayer pushImageFilter( ImageFilter filter, { + Offset offset = Offset.zero, ImageFilterEngineLayer? oldLayer, }) { assert(filter != null); @@ -527,14 +528,14 @@ class SceneBuilder extends NativeFieldWrapperClass1 { final _ImageFilter nativeFilter = filter._toNativeImageFilter(); assert(nativeFilter != null); final EngineLayer engineLayer = EngineLayer._(); - _pushImageFilter(engineLayer, nativeFilter, oldLayer?._nativeLayer); + _pushImageFilter(engineLayer, nativeFilter, offset.dx, offset.dy, oldLayer?._nativeLayer); final ImageFilterEngineLayer layer = ImageFilterEngineLayer._(engineLayer); assert(_debugPushLayer(layer)); return layer; } - @FfiNative, Handle, Pointer, Handle)>('SceneBuilder::pushImageFilter') - external void _pushImageFilter(EngineLayer outEngineLayer, _ImageFilter filter, EngineLayer? oldLayer); + @FfiNative, Handle, Pointer, Double, Double, Handle)>('SceneBuilder::pushImageFilter') + external void _pushImageFilter(EngineLayer outEngineLayer, _ImageFilter filter, double dx, double dy, EngineLayer? oldLayer); /// Pushes a backdrop filter operation onto the operation stack. /// diff --git a/engine/src/flutter/lib/ui/compositing/scene_builder.cc b/engine/src/flutter/lib/ui/compositing/scene_builder.cc index 71419b22f2..6f1c718fa1 100644 --- a/engine/src/flutter/lib/ui/compositing/scene_builder.cc +++ b/engine/src/flutter/lib/ui/compositing/scene_builder.cc @@ -151,9 +151,11 @@ void SceneBuilder::pushColorFilter(Dart_Handle layer_handle, void SceneBuilder::pushImageFilter(Dart_Handle layer_handle, const ImageFilter* image_filter, + double dx, + double dy, const fml::RefPtr& oldLayer) { - auto layer = - std::make_shared(image_filter->filter()); + auto layer = std::make_shared( + image_filter->filter(), SkPoint::Make(dx, dy)); PushLayer(layer); EngineLayer::MakeRetained(layer_handle, layer); diff --git a/engine/src/flutter/lib/ui/compositing/scene_builder.h b/engine/src/flutter/lib/ui/compositing/scene_builder.h index 1ef7e0773e..f2de29c480 100644 --- a/engine/src/flutter/lib/ui/compositing/scene_builder.h +++ b/engine/src/flutter/lib/ui/compositing/scene_builder.h @@ -74,6 +74,8 @@ class SceneBuilder : public RefCountedDartWrappable { const fml::RefPtr& oldLayer); void pushImageFilter(Dart_Handle layer_handle, const ImageFilter* image_filter, + double dx, + double dy, const fml::RefPtr& oldLayer); void pushBackdropFilter(Dart_Handle layer_handle, ImageFilter* filter, diff --git a/engine/src/flutter/lib/web_ui/lib/compositing.dart b/engine/src/flutter/lib/web_ui/lib/compositing.dart index 11154ad660..d3f0acddd6 100644 --- a/engine/src/flutter/lib/web_ui/lib/compositing.dart +++ b/engine/src/flutter/lib/web_ui/lib/compositing.dart @@ -71,6 +71,7 @@ abstract class SceneBuilder { }); ImageFilterEngineLayer pushImageFilter( ImageFilter filter, { + Offset offset = Offset.zero, ImageFilterEngineLayer? oldLayer, }); BackdropFilterEngineLayer pushBackdropFilter( diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/layer.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/layer.dart index fa067f049f..3f09e27e0b 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/layer.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/layer.dart @@ -394,18 +394,22 @@ class OffsetEngineLayer extends TransformEngineLayer /// A layer that applies an [ui.ImageFilter] to its children. class ImageFilterEngineLayer extends ContainerLayer implements ui.ImageFilterEngineLayer { - ImageFilterEngineLayer(this._filter); + ImageFilterEngineLayer(this._filter, this._offset); + final ui.Offset _offset; final ui.ImageFilter _filter; @override void paint(PaintContext paintContext) { assert(needsPainting); + paintContext.internalNodesCanvas.save(); + paintContext.internalNodesCanvas.translate(_offset.dx, _offset.dy); final CkPaint paint = CkPaint(); paint.imageFilter = _filter; paintContext.internalNodesCanvas.saveLayer(paintBounds, paint); paintChildren(paintContext); paintContext.internalNodesCanvas.restore(); + paintContext.internalNodesCanvas.restore(); } // TODO(dnfield): dispose of the _filter diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/layer_scene_builder.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/layer_scene_builder.dart index bca671f680..addd3aa5b9 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/layer_scene_builder.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/layer_scene_builder.dart @@ -155,9 +155,10 @@ class LayerSceneBuilder implements ui.SceneBuilder { ImageFilterEngineLayer pushImageFilter( ui.ImageFilter filter, { ui.ImageFilterEngineLayer? oldLayer, + ui.Offset offset = ui.Offset.zero, }) { assert(filter != null); - return pushLayer(ImageFilterEngineLayer(filter)); + return pushLayer(ImageFilterEngineLayer(filter, offset)); } @override diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/html/image_filter.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/html/image_filter.dart index 7fe37f641b..13de22ab99 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/html/image_filter.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/html/image_filter.dart @@ -7,22 +7,54 @@ import 'package:ui/ui.dart' as ui; import '../color_filter.dart'; import '../dom.dart'; import '../embedder.dart'; +import '../util.dart'; +import '../vector_math.dart'; import 'shaders/shader.dart'; import 'surface.dart'; +import 'surface_stats.dart'; /// A surface that applies an [imageFilter] to its children. class PersistedImageFilter extends PersistedContainerSurface implements ui.ImageFilterEngineLayer { - PersistedImageFilter(PersistedImageFilter? super.oldLayer, this.filter); + PersistedImageFilter(PersistedImageFilter? super.oldLayer, this.filter, this.offset); final ui.ImageFilter filter; + final ui.Offset offset; + + @override + void recomputeTransformAndClip() { + transform = parent!.transform; + + final double dx = offset.dx; + final double dy = offset.dy; + + if (dx != 0.0 || dy != 0.0) { + transform = transform!.clone(); + transform!.translate(dx, dy); + } + projectedClip = null; + } + + /// Cached inverse of transform on this node. Unlike transform, this + /// Matrix only contains local transform (not chain multiplied since root). + Matrix4? _localTransformInverse; + + @override + Matrix4 get localTransformInverse => _localTransformInverse ??= + Matrix4.translationValues(-offset.dx, -offset.dy, 0); DomElement? _svgFilter; + @override + DomElement? get childContainer => _childContainer; + DomElement? _childContainer; @override void adoptElements(PersistedImageFilter oldSurface) { super.adoptElements(oldSurface); _svgFilter = oldSurface._svgFilter; + _childContainer = oldSurface._childContainer; + oldSurface._svgFilter = null; + oldSurface._childContainer = null; } @override @@ -30,11 +62,26 @@ class PersistedImageFilter extends PersistedContainerSurface super.discard(); flutterViewEmbedder.removeResource(_svgFilter); _svgFilter = null; + _childContainer = null; } @override DomElement createElement() { - return defaultCreateElement('flt-image-filter'); + final DomElement element = defaultCreateElement('flt-image-filter'); + final DomElement container = defaultCreateElement('flt-image-filter-interior'); + if (debugExplainSurfaceStats) { + // This creates an additional interior element. Count it too. + surfaceStatsFor(this).allocatedDomNodeCount++; + } + + setElementStyle(container, 'position', 'absolute'); + setElementStyle(container, 'transform-origin', '0 0 0'); + setElementStyle(element, 'position', 'absolute'); + setElementStyle(element, 'transform-origin', '0 0 0'); + + _childContainer = container; + element.appendChild(container); + return element; } @override @@ -57,15 +104,18 @@ class PersistedImageFilter extends PersistedContainerSurface _svgFilter = backendFilter.makeSvgFilter(rootElement); } - rootElement!.style.filter = backendFilter.filterAttribute; - rootElement!.style.transform = backendFilter.transformAttribute; + _childContainer!.style.filter = backendFilter.filterAttribute; + _childContainer!.style.transform = backendFilter.transformAttribute; + rootElement!.style + ..left = '${offset.dx}px' + ..top = '${offset.dy}px'; } @override void update(PersistedImageFilter oldSurface) { super.update(oldSurface); - if (oldSurface.filter != filter) { + if (oldSurface.filter != filter || oldSurface.offset != offset) { apply(); } } diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/html/scene_builder.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/html/scene_builder.dart index 1e552b15b5..bd84aeaf9a 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/html/scene_builder.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/html/scene_builder.dart @@ -223,11 +223,12 @@ class SurfaceSceneBuilder implements ui.SceneBuilder { @override ui.ImageFilterEngineLayer pushImageFilter( ui.ImageFilter filter, { + ui.Offset offset = ui.Offset.zero, ui.ImageFilterEngineLayer? oldLayer, }) { assert(filter != null); return _pushSurface( - PersistedImageFilter(oldLayer as PersistedImageFilter?, filter)); + PersistedImageFilter(oldLayer as PersistedImageFilter?, filter, offset)); } /// Pushes a backdrop filter operation onto the operation stack.