From e05c2ced6bb07108920a6a29dcc3c2726be289a4 Mon Sep 17 00:00:00 2001 From: Yegor Jbanov Date: Thu, 20 Jun 2019 13:40:08 -0700 Subject: [PATCH 01/10] Make layers more tightly typed; introduce oldLayer --- lib/stub_ui/lib/src/ui/compositing.dart | 22 +-- lib/ui/compositing.dart | 235 ++++++++++++++++++++++-- 2 files changed, 226 insertions(+), 31 deletions(-) diff --git a/lib/stub_ui/lib/src/ui/compositing.dart b/lib/stub_ui/lib/src/ui/compositing.dart index f990c5bffbf1f..57b11e0a523ea 100644 --- a/lib/stub_ui/lib/src/ui/compositing.dart +++ b/lib/stub_ui/lib/src/ui/compositing.dart @@ -108,7 +108,7 @@ class SceneBuilder { /// This is equivalent to [pushTransform] with a matrix with only translation. /// /// See [pop] for details about the operation stack. - EngineLayer pushOffset(double dx, double dy) { + EngineLayer pushOffset(double dx, double dy, { EngineLayer oldLayer }) { return _pushSurface(engine.PersistedOffset(null, dx, dy)); } @@ -117,7 +117,7 @@ class SceneBuilder { /// The objects are transformed by the given matrix before rasterization. /// /// See [pop] for details about the operation stack. - EngineLayer pushTransform(Float64List matrix4) { + EngineLayer pushTransform(Float64List matrix4, { EngineLayer oldLayer }) { if (matrix4 == null) { throw new ArgumentError('"matrix4" argument cannot be null'); } @@ -134,7 +134,7 @@ class SceneBuilder { /// See [pop] for details about the operation stack, and [Clip] for different clip modes. /// By default, the clip will be anti-aliased (clip = [Clip.antiAlias]). EngineLayer pushClipRect(Rect rect, - {Clip clipBehavior = Clip.antiAlias}) { + {Clip clipBehavior = Clip.antiAlias, EngineLayer oldLayer }) { assert(clipBehavior != null); assert(clipBehavior != Clip.none); return _pushSurface(engine.PersistedClipRect(null, rect)); @@ -146,7 +146,7 @@ class SceneBuilder { /// /// See [pop] for details about the operation stack. EngineLayer pushClipRRect(RRect rrect, - {Clip clipBehavior}) { + {Clip clipBehavior, EngineLayer oldLayer }) { return _pushSurface( engine.PersistedClipRRect(null, rrect, clipBehavior)); } @@ -157,7 +157,7 @@ class SceneBuilder { /// /// See [pop] for details about the operation stack. EngineLayer pushClipPath(Path path, - {Clip clipBehavior = Clip.antiAlias}) { + {Clip clipBehavior = Clip.antiAlias, EngineLayer oldLayer }) { assert(clipBehavior != null); assert(clipBehavior != Clip.none); return _pushSurface( @@ -173,9 +173,8 @@ class SceneBuilder { /// /// See [pop] for details about the operation stack. EngineLayer pushOpacity(int alpha, - {Offset offset = Offset.zero}) { - return _pushSurface( - engine.PersistedOpacity(null, alpha, offset)); + {Offset offset = Offset.zero, EngineLayer oldLayer}) { + return _pushSurface(engine.PersistedOpacity(null, alpha, offset)); } /// Pushes a color filter operation onto the operation stack. @@ -184,7 +183,7 @@ class SceneBuilder { /// blend mode. /// /// See [pop] for details about the operation stack. - EngineLayer pushColorFilter(Color color, BlendMode blendMode) { + EngineLayer pushColorFilter(Color color, BlendMode blendMode, { EngineLayer oldLayer }) { throw new UnimplementedError(); } @@ -194,7 +193,7 @@ class SceneBuilder { /// rasterizing the given objects. /// /// See [pop] for details about the operation stack. - EngineLayer pushBackdropFilter(ImageFilter filter) { + EngineLayer pushBackdropFilter(ImageFilter filter, { EngineLayer oldLayer }) { throw new UnimplementedError(); } @@ -204,7 +203,7 @@ class SceneBuilder { /// rectangle using the given blend mode. /// /// See [pop] for details about the operation stack. - EngineLayer pushShaderMask(Shader shader, Rect maskRect, BlendMode blendMode) { + EngineLayer pushShaderMask(Shader shader, Rect maskRect, BlendMode blendMode, { EngineLayer oldLayer }) { throw new UnimplementedError(); } @@ -226,6 +225,7 @@ class SceneBuilder { Color color, Color shadowColor, Clip clipBehavior = Clip.none, + EngineLayer oldLayer, }) { return _pushSurface(engine.PersistedPhysicalShape( null, diff --git a/lib/ui/compositing.dart b/lib/ui/compositing.dart index 6cdd8e260cd28..9f5a77d35959a 100644 --- a/lib/ui/compositing.dart +++ b/lib/ui/compositing.dart @@ -38,6 +38,112 @@ class Scene extends NativeFieldWrapperClass2 { void dispose() native 'Scene_dispose'; } +// Wraps a native layer object. +// +// This is used to provide a typed API for engine layers to prevent +// incompatible layers from being passed to [SceneBuilder]'s push methods. +// For example, this prevents a layer returned from `pushOpacity` from being +// passed as `oldLayer` to `pushTransform`. +abstract class _EngineLayerWrapper implements EngineLayer { + _EngineLayerWrapper._(this._nativeLayer); + + EngineLayer _nativeLayer; +} + +/// An opaque handle to a transform engine layer. +/// +/// Instances of this class are created by [SceneBuilder.pushTransform]. +/// +/// {@template dart.ui.sceneBuilder.oldLayerCompatibility} +/// `oldLayer` parameter in [SceneBuilder] methods only accepts objects created +/// by the engine. [SceneBuilder] will throw an [AssertionError] if you pass it +/// a custom implementation of this class. +/// {@endtemplate} +class TransformEngineLayer extends _EngineLayerWrapper { + TransformEngineLayer._(EngineLayer nativeLayer) : super._(nativeLayer); +} + +/// An opaque handle to an offset engine layer. +/// +/// Instances of this class are created by [SceneBuilder.pushOffset]. +/// +/// {@macro dart.ui.sceneBuilder.oldLayerCompatibility} +class OffsetEngineLayer extends _EngineLayerWrapper { + OffsetEngineLayer._(EngineLayer nativeLayer) : super._(nativeLayer); +} + +/// An opaque handle to a clip rect engine layer. +/// +/// Instances of this class are created by [SceneBuilder.pushClipRect]. +/// +/// {@macro dart.ui.sceneBuilder.oldLayerCompatibility} +class ClipRectEngineLayer extends _EngineLayerWrapper { + ClipRectEngineLayer._(EngineLayer nativeLayer) : super._(nativeLayer); +} + +/// An opaque handle to a clip rounded rect engine layer. +/// +/// Instances of this class are created by [SceneBuilder.pushClipRRect]. +/// +/// {@macro dart.ui.sceneBuilder.oldLayerCompatibility} +class ClipRRectEngineLayer extends _EngineLayerWrapper { + ClipRRectEngineLayer._(EngineLayer nativeLayer) : super._(nativeLayer); +} + +/// An opaque handle to a clip path engine layer. +/// +/// Instances of this class are created by [SceneBuilder.pushClipPath]. +/// +/// {@macro dart.ui.sceneBuilder.oldLayerCompatibility} +class ClipPathEngineLayer extends _EngineLayerWrapper { + ClipPathEngineLayer._(EngineLayer nativeLayer) : super._(nativeLayer); +} + +/// An opaque handle to an opacity engine layer. +/// +/// Instances of this class are created by [SceneBuilder.pushOpacity]. +/// +/// {@macro dart.ui.sceneBuilder.oldLayerCompatibility} +class OpacityEngineLayer extends _EngineLayerWrapper { + OpacityEngineLayer._(EngineLayer nativeLayer) : super._(nativeLayer); +} + +/// An opaque handle to a color filter engine layer. +/// +/// Instances of this class are created by [SceneBuilder.pushColorFilter]. +/// +/// {@macro dart.ui.sceneBuilder.oldLayerCompatibility} +class ColorFilterEngineLayer extends _EngineLayerWrapper { + ColorFilterEngineLayer._(EngineLayer nativeLayer) : super._(nativeLayer); +} + +/// An opaque handle to a backdrop filter engine layer. +/// +/// Instances of this class are created by [SceneBuilder.pushBackdropFilter]. +/// +/// {@macro dart.ui.sceneBuilder.oldLayerCompatibility} +class BackdropFilterEngineLayer extends _EngineLayerWrapper { + BackdropFilterEngineLayer._(EngineLayer nativeLayer) : super._(nativeLayer); +} + +/// An opaque handle to a shader mask engine layer. +/// +/// Instances of this class are created by [SceneBuilder.pushShaderMask]. +/// +/// {@macro dart.ui.sceneBuilder.oldLayerCompatibility} +class ShaderMaskEngineLayer extends _EngineLayerWrapper { + ShaderMaskEngineLayer._(EngineLayer nativeLayer) : super._(nativeLayer); +} + +/// An opaque handle to a physical shape engine layer. +/// +/// Instances of this class are created by [SceneBuilder.pushPhysicalShape]. +/// +/// {@macro dart.ui.sceneBuilder.oldLayerCompatibility} +class PhysicalShapeEngineLayer extends _EngineLayerWrapper { + PhysicalShapeEngineLayer._(EngineLayer nativeLayer) : super._(nativeLayer); +} + /// Builds a [Scene] containing the given visuals. /// /// A [Scene] can then be rendered using [Window.render]. @@ -51,34 +157,71 @@ class SceneBuilder extends NativeFieldWrapperClass2 { SceneBuilder() { _constructor(); } void _constructor() native 'SceneBuilder_constructor'; + static void _debugAssertCompatibleLayerImplementation(EngineLayer layer, Type compatibleType, String methodName) { + if (layer == null) { + return; + } + assert( + layer.runtimeType != compatibleType, + 'Unsupported implementation of $compatibleType passed to SceneBuilder.$methodName as oldLayer argument.\n' + 'Only instances of $compatibleType returned by the SceneBuilder.$methodName are accepted, ' + 'but the passed implementation had type ${layer.runtimeType}.\n', + ); + } + /// Pushes a transform operation onto the operation stack. /// /// The objects are transformed by the given matrix before rasterization. + /// + /// {@template dart.ui.sceneBuilder.oldLayer} + /// If `oldLayer` is not null the engine will attempt to reuse the resources + /// allocated for the old layer when rendering the new layer. This is purely + /// an optimization. It has no effect on the correctness of rendering. + /// {@endtemplate} /// /// See [pop] for details about the operation stack. - EngineLayer pushTransform(Float64List matrix4) { + TransformEngineLayer pushTransform(Float64List matrix4, { TransformEngineLayer oldLayer }) { assert(_matrix4IsValid(matrix4)); - return _pushTransform(matrix4); + assert(() { + _debugAssertCompatibleLayerImplementation(oldLayer, TransformEngineLayer, 'pushTransform'); + return true; + }()); + return TransformEngineLayer._(_pushTransform(matrix4)); } EngineLayer _pushTransform(Float64List matrix4) native 'SceneBuilder_pushTransform'; /// Pushes an offset operation onto the operation stack. /// /// This is equivalent to [pushTransform] with a matrix with only translation. + /// + /// {@macro dart.ui.sceneBuilder.oldLayer} /// /// See [pop] for details about the operation stack. - EngineLayer pushOffset(double dx, double dy) native 'SceneBuilder_pushOffset'; + OffsetEngineLayer pushOffset(double dx, double dy, { OffsetEngineLayer oldLayer }) { + assert(() { + _debugAssertCompatibleLayerImplementation(oldLayer, OffsetEngineLayer, 'pushOffset'); + return true; + }()); + return OffsetEngineLayer._(_pushOffset(dx, dy)); + } + EngineLayer _pushOffset(double dx, double dy) native 'SceneBuilder_pushOffset'; /// Pushes a rectangular clip operation onto the operation stack. /// /// Rasterization outside the given rectangle is discarded. + /// + /// {@macro dart.ui.sceneBuilder.oldLayer} /// /// See [pop] for details about the operation stack, and [Clip] for different clip modes. /// By default, the clip will be anti-aliased (clip = [Clip.antiAlias]). - EngineLayer pushClipRect(Rect rect, {Clip clipBehavior = Clip.antiAlias}) { + ClipRectEngineLayer pushClipRect(Rect rect, {Clip clipBehavior = Clip.antiAlias, ClipRectEngineLayer oldLayer }) { assert(clipBehavior != null); assert(clipBehavior != Clip.none); - return _pushClipRect(rect.left, rect.right, rect.top, rect.bottom, clipBehavior.index); + assert(() { + _debugAssertCompatibleLayerImplementation(oldLayer, ClipRectEngineLayer, 'pushClipRect'); + return true; + }()); + return ClipRectEngineLayer._(_pushClipRect(rect.left, rect.right, rect.top, rect.bottom, clipBehavior.index)); } EngineLayer _pushClipRect(double left, double right, @@ -89,26 +232,38 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// Pushes a rounded-rectangular clip operation onto the operation stack. /// /// Rasterization outside the given rounded rectangle is discarded. + /// + /// {@macro dart.ui.sceneBuilder.oldLayer} /// /// See [pop] for details about the operation stack, and [Clip] for different clip modes. /// By default, the clip will be anti-aliased (clip = [Clip.antiAlias]). - EngineLayer pushClipRRect(RRect rrect, {Clip clipBehavior = Clip.antiAlias}) { + ClipRRectEngineLayer pushClipRRect(RRect rrect, {Clip clipBehavior = Clip.antiAlias, ClipRRectEngineLayer oldLayer}) { assert(clipBehavior != null); assert(clipBehavior != Clip.none); - return _pushClipRRect(rrect._value32, clipBehavior.index); + assert(() { + _debugAssertCompatibleLayerImplementation(oldLayer, ClipRRectEngineLayer, 'pushClipRRect'); + return true; + }()); + return ClipRRectEngineLayer._(_pushClipRRect(rrect._value32, clipBehavior.index)); } EngineLayer _pushClipRRect(Float32List rrect, int clipBehavior) native 'SceneBuilder_pushClipRRect'; /// Pushes a path clip operation onto the operation stack. /// /// Rasterization outside the given path is discarded. + /// + /// {@macro dart.ui.sceneBuilder.oldLayer} /// /// See [pop] for details about the operation stack. See [Clip] for different clip modes. /// By default, the clip will be anti-aliased (clip = [Clip.antiAlias]). - EngineLayer pushClipPath(Path path, {Clip clipBehavior = Clip.antiAlias}) { + ClipPathEngineLayer pushClipPath(Path path, {Clip clipBehavior = Clip.antiAlias, ClipPathEngineLayer oldLayer}) { assert(clipBehavior != null); assert(clipBehavior != Clip.none); - return _pushClipPath(path, clipBehavior.index); + assert(() { + _debugAssertCompatibleLayerImplementation(oldLayer, ClipPathEngineLayer, 'pushClipPath'); + return true; + }()); + return ClipPathEngineLayer._(_pushClipPath(path, clipBehavior.index)); } EngineLayer _pushClipPath(Path path, int clipBehavior) native 'SceneBuilder_pushClipPath'; @@ -118,10 +273,16 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// rasterization. An alpha value of 0 makes the objects entirely invisible. /// An alpha value of 255 has no effect (i.e., the objects retain the current /// opacity). + /// + /// {@macro dart.ui.sceneBuilder.oldLayer} /// /// See [pop] for details about the operation stack. - EngineLayer pushOpacity(int alpha, {Offset offset = Offset.zero}) { - return _pushOpacity(alpha, offset.dx, offset.dy); + OpacityEngineLayer pushOpacity(int alpha, {Offset offset = Offset.zero, OpacityEngineLayer oldLayer}) { + assert(() { + _debugAssertCompatibleLayerImplementation(oldLayer, OpacityEngineLayer, 'pushOpacity'); + return true; + }()); + return OpacityEngineLayer._(_pushOpacity(alpha, offset.dx, offset.dy)); } EngineLayer _pushOpacity(int alpha, double dx, double dy) native 'SceneBuilder_pushOpacity'; @@ -129,10 +290,16 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// /// The given color is applied to the objects' rasterization using the given /// blend mode. + /// + /// {@macro dart.ui.sceneBuilder.oldLayer} /// /// See [pop] for details about the operation stack. - EngineLayer pushColorFilter(Color color, BlendMode blendMode) { - return _pushColorFilter(color.value, blendMode.index); + ColorFilterEngineLayer pushColorFilter(Color color, BlendMode blendMode, ColorFilterEngineLayer oldLayer) { + assert(() { + _debugAssertCompatibleLayerImplementation(oldLayer, ColorFilterEngineLayer, 'pushColorFilter'); + return true; + }()); + return ColorFilterEngineLayer._(_pushColorFilter(color.value, blendMode.index)); } EngineLayer _pushColorFilter(int color, int blendMode) native 'SceneBuilder_pushColorFilter'; @@ -140,23 +307,38 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// /// The given filter is applied to the current contents of the scene prior to /// rasterizing the given objects. + /// + /// {@macro dart.ui.sceneBuilder.oldLayer} /// /// See [pop] for details about the operation stack. - EngineLayer pushBackdropFilter(ImageFilter filter) native 'SceneBuilder_pushBackdropFilter'; + BackdropFilterEngineLayer pushBackdropFilter(ImageFilter filter, { BackdropFilterEngineLayer oldLayer }) { + assert(() { + _debugAssertCompatibleLayerImplementation(oldLayer, BackdropFilterEngineLayer, 'pushBackdropFilter'); + return true; + }()); + return BackdropFilterEngineLayer._(_pushBackdropFilter(filter)); + } + EngineLayer _pushBackdropFilter(ImageFilter filter) native 'SceneBuilder_pushBackdropFilter'; /// Pushes a shader mask operation onto the operation stack. /// /// The given shader is applied to the object's rasterization in the given /// rectangle using the given blend mode. + /// + /// {@macro dart.ui.sceneBuilder.oldLayer} /// /// See [pop] for details about the operation stack. - EngineLayer pushShaderMask(Shader shader, Rect maskRect, BlendMode blendMode) { - return _pushShaderMask(shader, + ShaderMaskEngineLayer pushShaderMask(Shader shader, Rect maskRect, BlendMode blendMode, { ShaderMaskEngineLayer oldLayer }) { + assert(() { + _debugAssertCompatibleLayerImplementation(oldLayer, ShaderMaskEngineLayer, 'pushShaderMask'); + return true; + }()); + return ShaderMaskEngineLayer._(_pushShaderMask(shader, maskRect.left, maskRect.right, maskRect.top, maskRect.bottom, - blendMode.index); + blendMode.index)); } EngineLayer _pushShaderMask(Shader shader, double maskRectLeft, @@ -175,11 +357,17 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// If [elevation] is greater than 0.0, then a shadow is drawn around the layer. /// [shadowColor] defines the color of the shadow if present and [color] defines the /// color of the layer background. + /// + /// {@macro dart.ui.sceneBuilder.oldLayer} /// /// See [pop] for details about the operation stack, and [Clip] for different clip modes. // ignore: deprecated_member_use - EngineLayer pushPhysicalShape({ Path path, double elevation, Color color, Color shadowColor, Clip clipBehavior = Clip.none}) { - return _pushPhysicalShape(path, elevation, color.value, shadowColor?.value ?? 0xFF000000, clipBehavior.index); + PhysicalShapeEngineLayer pushPhysicalShape({ Path path, double elevation, Color color, Color shadowColor, Clip clipBehavior = Clip.none, PhysicalShapeEngineLayer oldLayer }) { + assert(() { + _debugAssertCompatibleLayerImplementation(oldLayer, PhysicalShapeEngineLayer, 'pushPhysicalShape'); + return true; + }()); + return PhysicalShapeEngineLayer._(_pushPhysicalShape(path, elevation, color.value, shadowColor?.value ?? 0xFF000000, clipBehavior.index)); } EngineLayer _pushPhysicalShape(Path path, double elevation, int color, int shadowColor, int clipBehavior) native 'SceneBuilder_pushPhysicalShape'; @@ -200,7 +388,14 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// Therefore, when implementing a subclass of the [Layer] concept defined in /// the rendering layer of Flutter's framework, once this is called, there's /// no need to call [addToScene] for its children layers. - void addRetained(EngineLayer retainedLayer) native 'SceneBuilder_addRetained'; + void addRetained(EngineLayer retainedLayer) { + if (retainedLayer is _EngineLayerWrapper) { + _addRetained(retainedLayer._nativeLayer); + } else { + _addRetained(retainedLayer); + } + } + void _addRetained(EngineLayer retainedLayer) native 'SceneBuilder_addRetained'; /// Adds an object to the scene that displays performance statistics. /// From 65278594597cfc2d56a3b77305377ac9195fbef8 Mon Sep 17 00:00:00 2001 From: Yegor Jbanov Date: Mon, 24 Jun 2019 12:57:50 -0700 Subject: [PATCH 02/10] validation and tests --- lib/ui/compositing.dart | 90 ++++++++++++------------------ testing/dart/compositing_test.dart | 88 +++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+), 54 deletions(-) diff --git a/lib/ui/compositing.dart b/lib/ui/compositing.dart index 9f5a77d35959a..9d8fc31c4f54d 100644 --- a/lib/ui/compositing.dart +++ b/lib/ui/compositing.dart @@ -38,12 +38,13 @@ class Scene extends NativeFieldWrapperClass2 { void dispose() native 'Scene_dispose'; } -// Wraps a native layer object. +// Lightweight wrapper of a native layer object. // // This is used to provide a typed API for engine layers to prevent // incompatible layers from being passed to [SceneBuilder]'s push methods. // For example, this prevents a layer returned from `pushOpacity` from being -// passed as `oldLayer` to `pushTransform`. +// passed as `oldLayer` to `pushTransform`. This is achieved by having one +// concrete subclass of this class per push method. abstract class _EngineLayerWrapper implements EngineLayer { _EngineLayerWrapper._(this._nativeLayer); @@ -157,16 +158,28 @@ class SceneBuilder extends NativeFieldWrapperClass2 { SceneBuilder() { _constructor(); } void _constructor() native 'SceneBuilder_constructor'; - static void _debugAssertCompatibleLayerImplementation(EngineLayer layer, Type compatibleType, String methodName) { + // Layers used in this scene. + // + // The key is the layer used. The value is the description of what the layer + // is used for, e.g. "pushOpacity" or "addRetained". + Map _usedLayers = {}; + + // In debug mode checks that the `layer` is only used once in a given scene. + bool _debugCheckUsedOnce(EngineLayer layer, String usage) { if (layer == null) { - return; + return true; } + assert( - layer.runtimeType != compatibleType, - 'Unsupported implementation of $compatibleType passed to SceneBuilder.$methodName as oldLayer argument.\n' - 'Only instances of $compatibleType returned by the SceneBuilder.$methodName are accepted, ' - 'but the passed implementation had type ${layer.runtimeType}.\n', + !_usedLayers.containsKey(layer), + 'Layer ${layer.runtimeType} already used.\n' + 'The layer is already being used for ${_usedLayers[layer]} in this scene.\n' + 'A layer may only be used once in a given scene.' ); + + _usedLayers[layer] = usage; + + return true; } /// Pushes a transform operation onto the operation stack. @@ -182,10 +195,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// See [pop] for details about the operation stack. TransformEngineLayer pushTransform(Float64List matrix4, { TransformEngineLayer oldLayer }) { assert(_matrix4IsValid(matrix4)); - assert(() { - _debugAssertCompatibleLayerImplementation(oldLayer, TransformEngineLayer, 'pushTransform'); - return true; - }()); + assert(_debugCheckUsedOnce(oldLayer, 'oldLayer in pushTransform')); return TransformEngineLayer._(_pushTransform(matrix4)); } EngineLayer _pushTransform(Float64List matrix4) native 'SceneBuilder_pushTransform'; @@ -198,10 +208,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// /// See [pop] for details about the operation stack. OffsetEngineLayer pushOffset(double dx, double dy, { OffsetEngineLayer oldLayer }) { - assert(() { - _debugAssertCompatibleLayerImplementation(oldLayer, OffsetEngineLayer, 'pushOffset'); - return true; - }()); + assert(_debugCheckUsedOnce(oldLayer, 'oldLayer in pushOffset')); return OffsetEngineLayer._(_pushOffset(dx, dy)); } EngineLayer _pushOffset(double dx, double dy) native 'SceneBuilder_pushOffset'; @@ -217,10 +224,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { ClipRectEngineLayer pushClipRect(Rect rect, {Clip clipBehavior = Clip.antiAlias, ClipRectEngineLayer oldLayer }) { assert(clipBehavior != null); assert(clipBehavior != Clip.none); - assert(() { - _debugAssertCompatibleLayerImplementation(oldLayer, ClipRectEngineLayer, 'pushClipRect'); - return true; - }()); + assert(_debugCheckUsedOnce(oldLayer, 'oldLayer in pushClipRect')); return ClipRectEngineLayer._(_pushClipRect(rect.left, rect.right, rect.top, rect.bottom, clipBehavior.index)); } EngineLayer _pushClipRect(double left, @@ -240,10 +244,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { ClipRRectEngineLayer pushClipRRect(RRect rrect, {Clip clipBehavior = Clip.antiAlias, ClipRRectEngineLayer oldLayer}) { assert(clipBehavior != null); assert(clipBehavior != Clip.none); - assert(() { - _debugAssertCompatibleLayerImplementation(oldLayer, ClipRRectEngineLayer, 'pushClipRRect'); - return true; - }()); + assert(_debugCheckUsedOnce(oldLayer, 'oldLayer in pushClipRRect')); return ClipRRectEngineLayer._(_pushClipRRect(rrect._value32, clipBehavior.index)); } EngineLayer _pushClipRRect(Float32List rrect, int clipBehavior) native 'SceneBuilder_pushClipRRect'; @@ -259,10 +260,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { ClipPathEngineLayer pushClipPath(Path path, {Clip clipBehavior = Clip.antiAlias, ClipPathEngineLayer oldLayer}) { assert(clipBehavior != null); assert(clipBehavior != Clip.none); - assert(() { - _debugAssertCompatibleLayerImplementation(oldLayer, ClipPathEngineLayer, 'pushClipPath'); - return true; - }()); + assert(_debugCheckUsedOnce(oldLayer, 'oldLayer in pushClipPath')); return ClipPathEngineLayer._(_pushClipPath(path, clipBehavior.index)); } EngineLayer _pushClipPath(Path path, int clipBehavior) native 'SceneBuilder_pushClipPath'; @@ -278,10 +276,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// /// See [pop] for details about the operation stack. OpacityEngineLayer pushOpacity(int alpha, {Offset offset = Offset.zero, OpacityEngineLayer oldLayer}) { - assert(() { - _debugAssertCompatibleLayerImplementation(oldLayer, OpacityEngineLayer, 'pushOpacity'); - return true; - }()); + assert(_debugCheckUsedOnce(oldLayer, 'oldLayer in pushOpacity')); return OpacityEngineLayer._(_pushOpacity(alpha, offset.dx, offset.dy)); } EngineLayer _pushOpacity(int alpha, double dx, double dy) native 'SceneBuilder_pushOpacity'; @@ -294,11 +289,8 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// {@macro dart.ui.sceneBuilder.oldLayer} /// /// See [pop] for details about the operation stack. - ColorFilterEngineLayer pushColorFilter(Color color, BlendMode blendMode, ColorFilterEngineLayer oldLayer) { - assert(() { - _debugAssertCompatibleLayerImplementation(oldLayer, ColorFilterEngineLayer, 'pushColorFilter'); - return true; - }()); + ColorFilterEngineLayer pushColorFilter(Color color, BlendMode blendMode, { ColorFilterEngineLayer oldLayer }) { + assert(_debugCheckUsedOnce(oldLayer, 'oldLayer in pushColorFilter')); return ColorFilterEngineLayer._(_pushColorFilter(color.value, blendMode.index)); } EngineLayer _pushColorFilter(int color, int blendMode) native 'SceneBuilder_pushColorFilter'; @@ -312,10 +304,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// /// See [pop] for details about the operation stack. BackdropFilterEngineLayer pushBackdropFilter(ImageFilter filter, { BackdropFilterEngineLayer oldLayer }) { - assert(() { - _debugAssertCompatibleLayerImplementation(oldLayer, BackdropFilterEngineLayer, 'pushBackdropFilter'); - return true; - }()); + assert(_debugCheckUsedOnce(oldLayer, 'oldLayer in pushBackdropFilter')); return BackdropFilterEngineLayer._(_pushBackdropFilter(filter)); } EngineLayer _pushBackdropFilter(ImageFilter filter) native 'SceneBuilder_pushBackdropFilter'; @@ -329,10 +318,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// /// See [pop] for details about the operation stack. ShaderMaskEngineLayer pushShaderMask(Shader shader, Rect maskRect, BlendMode blendMode, { ShaderMaskEngineLayer oldLayer }) { - assert(() { - _debugAssertCompatibleLayerImplementation(oldLayer, ShaderMaskEngineLayer, 'pushShaderMask'); - return true; - }()); + assert(_debugCheckUsedOnce(oldLayer, 'oldLayer in pushShaderMask')); return ShaderMaskEngineLayer._(_pushShaderMask(shader, maskRect.left, maskRect.right, @@ -363,10 +349,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// See [pop] for details about the operation stack, and [Clip] for different clip modes. // ignore: deprecated_member_use PhysicalShapeEngineLayer pushPhysicalShape({ Path path, double elevation, Color color, Color shadowColor, Clip clipBehavior = Clip.none, PhysicalShapeEngineLayer oldLayer }) { - assert(() { - _debugAssertCompatibleLayerImplementation(oldLayer, PhysicalShapeEngineLayer, 'pushPhysicalShape'); - return true; - }()); + assert(_debugCheckUsedOnce(oldLayer, 'oldLayer in pushPhysicalShape')); return PhysicalShapeEngineLayer._(_pushPhysicalShape(path, elevation, color.value, shadowColor?.value ?? 0xFF000000, clipBehavior.index)); } EngineLayer _pushPhysicalShape(Path path, double elevation, int color, int shadowColor, int clipBehavior) native @@ -389,11 +372,10 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// the rendering layer of Flutter's framework, once this is called, there's /// no need to call [addToScene] for its children layers. void addRetained(EngineLayer retainedLayer) { - if (retainedLayer is _EngineLayerWrapper) { - _addRetained(retainedLayer._nativeLayer); - } else { - _addRetained(retainedLayer); - } + assert(retainedLayer is _EngineLayerWrapper); + assert(_debugCheckUsedOnce(retainedLayer, 'addRetained')); + final _EngineLayerWrapper wrapper = retainedLayer; + _addRetained(wrapper._nativeLayer); } void _addRetained(EngineLayer retainedLayer) native 'SceneBuilder_addRetained'; diff --git a/testing/dart/compositing_test.dart b/testing/dart/compositing_test.dart index 6135260c6e90f..18511f832d4cb 100644 --- a/testing/dart/compositing_test.dart +++ b/testing/dart/compositing_test.dart @@ -51,4 +51,92 @@ void main() { throwsA(const TypeMatcher()), ); }); + + test('SceneBuilder accepts typed layers', () { + final SceneBuilder builder1 = SceneBuilder(); + final OpacityEngineLayer opacity1 = builder1.pushOpacity(100); + expect(opacity1, isNotNull); + builder1.pop(); + builder1.build(); + + final SceneBuilder builder2 = SceneBuilder(); + final OpacityEngineLayer opacity2 = builder2.pushOpacity(200, oldLayer: opacity1); + expect(opacity2, isNotNull); + builder2.pop(); + builder2.build(); + }); + + void testNoSharing(_TestNoSharingFunction pushFunction) { + final SceneBuilder builder1 = SceneBuilder(); + final EngineLayer layer = pushFunction(builder1, null); + builder1.pop(); + builder1.build(); + + // Test: first push then attempt illegal addRetained + final SceneBuilder builder2 = SceneBuilder(); + pushFunction(builder2, layer); + builder2.pop(); + try { + builder2.addRetained(layer); + fail('Expected addRetained to throw AssertionError but it returned successully'); + } on AssertionError catch (error) { + expect(error.toString(), contains('The layer is already being used')); + } + builder2.build(); + + // Test: first addRetained then attempt illegal push + final SceneBuilder builder3 = SceneBuilder(); + builder3.addRetained(layer); + try { + pushFunction(builder3, layer); + fail('Expected push to throw AssertionError but it returned successully'); + } on AssertionError catch (error) { + expect(error.toString(), contains('The layer is already being used')); + } + builder3.build(); + } + + test('SceneBuilder does not share a layer between addRetained and push*', () { + testNoSharing((SceneBuilder builder, EngineLayer oldLayer) { + return builder.pushOffset(0, 0, oldLayer: oldLayer); + }); + testNoSharing((SceneBuilder builder, EngineLayer oldLayer) { + return builder.pushTransform(Float64List(16), oldLayer: oldLayer); + }); + testNoSharing((SceneBuilder builder, EngineLayer oldLayer) { + return builder.pushClipRect(Rect.zero, oldLayer: oldLayer); + }); + testNoSharing((SceneBuilder builder, EngineLayer oldLayer) { + return builder.pushClipRRect(RRect.zero, oldLayer: oldLayer); + }); + testNoSharing((SceneBuilder builder, EngineLayer oldLayer) { + return builder.pushClipPath(Path(), oldLayer: oldLayer); + }); + testNoSharing((SceneBuilder builder, EngineLayer oldLayer) { + return builder.pushOpacity(100, oldLayer: oldLayer); + }); + testNoSharing((SceneBuilder builder, EngineLayer oldLayer) { + return builder.pushColorFilter(const Color.fromARGB(0, 0, 0, 0), BlendMode.color, oldLayer: oldLayer); + }); + testNoSharing((SceneBuilder builder, EngineLayer oldLayer) { + return builder.pushBackdropFilter(ImageFilter.blur(), oldLayer: oldLayer); + }); + testNoSharing((SceneBuilder builder, EngineLayer oldLayer) { + return builder.pushShaderMask( + Gradient.radial( + const Offset(0, 0), + 10, + const [Color.fromARGB(0, 0, 0, 0), Color.fromARGB(0, 255, 255, 255)], + ), + Rect.zero, + BlendMode.color, + oldLayer: oldLayer, + ); + }); + testNoSharing((SceneBuilder builder, EngineLayer oldLayer) { + return builder.pushPhysicalShape(path: Path(), color: const Color.fromARGB(0, 0, 0, 0), oldLayer: oldLayer); + }); + }); } + +typedef _TestNoSharingFunction = EngineLayer Function(SceneBuilder builder, EngineLayer oldLayer); From c9fb3a3b1a5005968e8d0f8613ea7487ee953411 Mon Sep 17 00:00:00 2001 From: Yegor Jbanov Date: Mon, 24 Jun 2019 13:35:46 -0700 Subject: [PATCH 03/10] remove trailing whitespace --- lib/ui/compositing.dart | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/lib/ui/compositing.dart b/lib/ui/compositing.dart index 9d8fc31c4f54d..4e7aee12c4c6d 100644 --- a/lib/ui/compositing.dart +++ b/lib/ui/compositing.dart @@ -119,27 +119,27 @@ class ColorFilterEngineLayer extends _EngineLayerWrapper { } /// An opaque handle to a backdrop filter engine layer. -/// +/// /// Instances of this class are created by [SceneBuilder.pushBackdropFilter]. -/// +/// /// {@macro dart.ui.sceneBuilder.oldLayerCompatibility} class BackdropFilterEngineLayer extends _EngineLayerWrapper { BackdropFilterEngineLayer._(EngineLayer nativeLayer) : super._(nativeLayer); } /// An opaque handle to a shader mask engine layer. -/// +/// /// Instances of this class are created by [SceneBuilder.pushShaderMask]. -/// +/// /// {@macro dart.ui.sceneBuilder.oldLayerCompatibility} class ShaderMaskEngineLayer extends _EngineLayerWrapper { ShaderMaskEngineLayer._(EngineLayer nativeLayer) : super._(nativeLayer); } /// An opaque handle to a physical shape engine layer. -/// +/// /// Instances of this class are created by [SceneBuilder.pushPhysicalShape]. -/// +/// /// {@macro dart.ui.sceneBuilder.oldLayerCompatibility} class PhysicalShapeEngineLayer extends _EngineLayerWrapper { PhysicalShapeEngineLayer._(EngineLayer nativeLayer) : super._(nativeLayer); @@ -185,7 +185,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// Pushes a transform operation onto the operation stack. /// /// The objects are transformed by the given matrix before rasterization. - /// + /// /// {@template dart.ui.sceneBuilder.oldLayer} /// If `oldLayer` is not null the engine will attempt to reuse the resources /// allocated for the old layer when rendering the new layer. This is purely @@ -203,7 +203,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// Pushes an offset operation onto the operation stack. /// /// This is equivalent to [pushTransform] with a matrix with only translation. - /// + /// /// {@macro dart.ui.sceneBuilder.oldLayer} /// /// See [pop] for details about the operation stack. @@ -216,7 +216,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// Pushes a rectangular clip operation onto the operation stack. /// /// Rasterization outside the given rectangle is discarded. - /// + /// /// {@macro dart.ui.sceneBuilder.oldLayer} /// /// See [pop] for details about the operation stack, and [Clip] for different clip modes. @@ -236,7 +236,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// Pushes a rounded-rectangular clip operation onto the operation stack. /// /// Rasterization outside the given rounded rectangle is discarded. - /// + /// /// {@macro dart.ui.sceneBuilder.oldLayer} /// /// See [pop] for details about the operation stack, and [Clip] for different clip modes. @@ -252,7 +252,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// Pushes a path clip operation onto the operation stack. /// /// Rasterization outside the given path is discarded. - /// + /// /// {@macro dart.ui.sceneBuilder.oldLayer} /// /// See [pop] for details about the operation stack. See [Clip] for different clip modes. @@ -271,7 +271,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// rasterization. An alpha value of 0 makes the objects entirely invisible. /// An alpha value of 255 has no effect (i.e., the objects retain the current /// opacity). - /// + /// /// {@macro dart.ui.sceneBuilder.oldLayer} /// /// See [pop] for details about the operation stack. @@ -285,7 +285,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// /// The given color is applied to the objects' rasterization using the given /// blend mode. - /// + /// /// {@macro dart.ui.sceneBuilder.oldLayer} /// /// See [pop] for details about the operation stack. @@ -299,7 +299,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// /// The given filter is applied to the current contents of the scene prior to /// rasterizing the given objects. - /// + /// /// {@macro dart.ui.sceneBuilder.oldLayer} /// /// See [pop] for details about the operation stack. @@ -313,7 +313,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// /// The given shader is applied to the object's rasterization in the given /// rectangle using the given blend mode. - /// + /// /// {@macro dart.ui.sceneBuilder.oldLayer} /// /// See [pop] for details about the operation stack. @@ -343,7 +343,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// If [elevation] is greater than 0.0, then a shadow is drawn around the layer. /// [shadowColor] defines the color of the shadow if present and [color] defines the /// color of the layer background. - /// + /// /// {@macro dart.ui.sceneBuilder.oldLayer} /// /// See [pop] for details about the operation stack, and [Clip] for different clip modes. From 43dc4b311b8228c9b6197e89915c300785baf2f3 Mon Sep 17 00:00:00 2001 From: Yegor Jbanov Date: Tue, 25 Jun 2019 10:08:06 -0700 Subject: [PATCH 04/10] fix Web public API --- lib/stub_ui/lib/src/engine/surface/clip.dart | 8 +- .../lib/src/engine/surface/offset.dart | 2 +- .../lib/src/engine/surface/opacity.dart | 2 +- .../lib/src/engine/surface/transform.dart | 2 +- lib/stub_ui/lib/src/ui/compositing.dart | 104 +++++++++++++++--- 5 files changed, 96 insertions(+), 22 deletions(-) diff --git a/lib/stub_ui/lib/src/engine/surface/clip.dart b/lib/stub_ui/lib/src/engine/surface/clip.dart index b9cbb6e671b61..d0633c3322cc6 100644 --- a/lib/stub_ui/lib/src/engine/surface/clip.dart +++ b/lib/stub_ui/lib/src/engine/surface/clip.dart @@ -56,7 +56,7 @@ mixin _DomClip on PersistedContainerSurface { } /// A surface that creates a rectangular clip. -class PersistedClipRect extends PersistedContainerSurface with _DomClip { +class PersistedClipRect extends PersistedContainerSurface with _DomClip implements ui.ClipRectEngineLayer { PersistedClipRect(Object paintedBy, this.rect) : super(paintedBy); final ui.Rect rect; @@ -99,7 +99,7 @@ class PersistedClipRect extends PersistedContainerSurface with _DomClip { } /// A surface that creates a rounded rectangular clip. -class PersistedClipRRect extends PersistedContainerSurface with _DomClip { +class PersistedClipRRect extends PersistedContainerSurface with _DomClip implements ui.ClipRRectEngineLayer { PersistedClipRRect(Object paintedBy, this.rrect, this.clipBehavior) : super(paintedBy); @@ -148,7 +148,7 @@ class PersistedClipRRect extends PersistedContainerSurface with _DomClip { } } -class PersistedPhysicalShape extends PersistedContainerSurface with _DomClip { +class PersistedPhysicalShape extends PersistedContainerSurface with _DomClip implements ui.PhysicalShapeEngineLayer { PersistedPhysicalShape(Object paintedBy, this.path, this.elevation, int color, int shadowColor, this.clipBehavior) : this.color = ui.Color(color), @@ -313,7 +313,7 @@ class PersistedPhysicalShape extends PersistedContainerSurface with _DomClip { } /// A surface that clips it's children. -class PersistedClipPath extends PersistedContainerSurface { +class PersistedClipPath extends PersistedContainerSurface implements ui.ClipPathEngineLayer { PersistedClipPath(Object paintedBy, this.clipPath, this.clipBehavior) : super(paintedBy); diff --git a/lib/stub_ui/lib/src/engine/surface/offset.dart b/lib/stub_ui/lib/src/engine/surface/offset.dart index 8df344a34a60e..392f6dd63d958 100644 --- a/lib/stub_ui/lib/src/engine/surface/offset.dart +++ b/lib/stub_ui/lib/src/engine/surface/offset.dart @@ -5,7 +5,7 @@ part of engine; /// A surface that translates its children using CSS transform and translate. -class PersistedOffset extends PersistedContainerSurface { +class PersistedOffset extends PersistedContainerSurface implements ui.OffsetEngineLayer { PersistedOffset(Object paintedBy, this.dx, this.dy) : super(paintedBy); /// Horizontal displacement. diff --git a/lib/stub_ui/lib/src/engine/surface/opacity.dart b/lib/stub_ui/lib/src/engine/surface/opacity.dart index 55639706db957..0da7ac05492b3 100644 --- a/lib/stub_ui/lib/src/engine/surface/opacity.dart +++ b/lib/stub_ui/lib/src/engine/surface/opacity.dart @@ -5,7 +5,7 @@ part of engine; /// A surface that makes its children transparent. -class PersistedOpacity extends PersistedContainerSurface { +class PersistedOpacity extends PersistedContainerSurface implements ui.OpacityEngineLayer { PersistedOpacity(Object paintedBy, this.alpha, this.offset) : super(paintedBy); diff --git a/lib/stub_ui/lib/src/engine/surface/transform.dart b/lib/stub_ui/lib/src/engine/surface/transform.dart index 08b37c869eeb8..da0b1aa454772 100644 --- a/lib/stub_ui/lib/src/engine/surface/transform.dart +++ b/lib/stub_ui/lib/src/engine/surface/transform.dart @@ -5,7 +5,7 @@ part of engine; /// A surface that transforms its children using CSS transform. -class PersistedTransform extends PersistedContainerSurface { +class PersistedTransform extends PersistedContainerSurface implements ui.TransformEngineLayer { PersistedTransform(Object paintedBy, this.matrix4) : super(paintedBy); final Float64List matrix4; diff --git a/lib/stub_ui/lib/src/ui/compositing.dart b/lib/stub_ui/lib/src/ui/compositing.dart index 57b11e0a523ea..d70865673b303 100644 --- a/lib/stub_ui/lib/src/ui/compositing.dart +++ b/lib/stub_ui/lib/src/ui/compositing.dart @@ -39,6 +39,80 @@ class Scene { void dispose() {} } +/// An opaque handle to a transform engine layer. +/// +/// Instances of this class are created by [SceneBuilder.pushTransform]. +/// +/// {@template dart.ui.sceneBuilder.oldLayerCompatibility} +/// `oldLayer` parameter in [SceneBuilder] methods only accepts objects created +/// by the engine. [SceneBuilder] will throw an [AssertionError] if you pass it +/// a custom implementation of this class. +/// {@endtemplate} +abstract class TransformEngineLayer implements EngineLayer {} + +/// An opaque handle to an offset engine layer. +/// +/// Instances of this class are created by [SceneBuilder.pushOffset]. +/// +/// {@macro dart.ui.sceneBuilder.oldLayerCompatibility} +abstract class OffsetEngineLayer implements EngineLayer {} + +/// An opaque handle to a clip rect engine layer. +/// +/// Instances of this class are created by [SceneBuilder.pushClipRect]. +/// +/// {@macro dart.ui.sceneBuilder.oldLayerCompatibility} +abstract class ClipRectEngineLayer implements EngineLayer {} + +/// An opaque handle to a clip rounded rect engine layer. +/// +/// Instances of this class are created by [SceneBuilder.pushClipRRect]. +/// +/// {@macro dart.ui.sceneBuilder.oldLayerCompatibility} +abstract class ClipRRectEngineLayer implements EngineLayer {} + +/// An opaque handle to a clip path engine layer. +/// +/// Instances of this class are created by [SceneBuilder.pushClipPath]. +/// +/// {@macro dart.ui.sceneBuilder.oldLayerCompatibility} +abstract class ClipPathEngineLayer implements EngineLayer {} + +/// An opaque handle to an opacity engine layer. +/// +/// Instances of this class are created by [SceneBuilder.pushOpacity]. +/// +/// {@macro dart.ui.sceneBuilder.oldLayerCompatibility} +abstract class OpacityEngineLayer implements EngineLayer {} + +/// An opaque handle to a color filter engine layer. +/// +/// Instances of this class are created by [SceneBuilder.pushColorFilter]. +/// +/// {@macro dart.ui.sceneBuilder.oldLayerCompatibility} +abstract class ColorFilterEngineLayer implements EngineLayer {} + +/// An opaque handle to a backdrop filter engine layer. +/// +/// Instances of this class are created by [SceneBuilder.pushBackdropFilter]. +/// +/// {@macro dart.ui.sceneBuilder.oldLayerCompatibility} +abstract class BackdropFilterEngineLayer implements EngineLayer {} + +/// An opaque handle to a shader mask engine layer. +/// +/// Instances of this class are created by [SceneBuilder.pushShaderMask]. +/// +/// {@macro dart.ui.sceneBuilder.oldLayerCompatibility} +abstract class ShaderMaskEngineLayer implements EngineLayer {} + +/// An opaque handle to a physical shape engine layer. +/// +/// Instances of this class are created by [SceneBuilder.pushPhysicalShape]. +/// +/// {@macro dart.ui.sceneBuilder.oldLayerCompatibility} +abstract class PhysicalShapeEngineLayer implements EngineLayer {} + /// Builds a [Scene] containing the given visuals. /// /// A [Scene] can then be rendered using [Window.render]. @@ -108,7 +182,7 @@ class SceneBuilder { /// This is equivalent to [pushTransform] with a matrix with only translation. /// /// See [pop] for details about the operation stack. - EngineLayer pushOffset(double dx, double dy, { EngineLayer oldLayer }) { + OffsetEngineLayer pushOffset(double dx, double dy, { OffsetEngineLayer oldLayer }) { return _pushSurface(engine.PersistedOffset(null, dx, dy)); } @@ -117,7 +191,7 @@ class SceneBuilder { /// The objects are transformed by the given matrix before rasterization. /// /// See [pop] for details about the operation stack. - EngineLayer pushTransform(Float64List matrix4, { EngineLayer oldLayer }) { + TransformEngineLayer pushTransform(Float64List matrix4, { TransformEngineLayer oldLayer }) { if (matrix4 == null) { throw new ArgumentError('"matrix4" argument cannot be null'); } @@ -133,8 +207,8 @@ class SceneBuilder { /// /// See [pop] for details about the operation stack, and [Clip] for different clip modes. /// By default, the clip will be anti-aliased (clip = [Clip.antiAlias]). - EngineLayer pushClipRect(Rect rect, - {Clip clipBehavior = Clip.antiAlias, EngineLayer oldLayer }) { + ClipRectEngineLayer pushClipRect(Rect rect, + {Clip clipBehavior = Clip.antiAlias, ClipRectEngineLayer oldLayer }) { assert(clipBehavior != null); assert(clipBehavior != Clip.none); return _pushSurface(engine.PersistedClipRect(null, rect)); @@ -145,8 +219,8 @@ class SceneBuilder { /// Rasterization outside the given rounded rectangle is discarded. /// /// See [pop] for details about the operation stack. - EngineLayer pushClipRRect(RRect rrect, - {Clip clipBehavior, EngineLayer oldLayer }) { + ClipRRectEngineLayer pushClipRRect(RRect rrect, + {Clip clipBehavior, ClipRRectEngineLayer oldLayer }) { return _pushSurface( engine.PersistedClipRRect(null, rrect, clipBehavior)); } @@ -156,8 +230,8 @@ class SceneBuilder { /// Rasterization outside the given path is discarded. /// /// See [pop] for details about the operation stack. - EngineLayer pushClipPath(Path path, - {Clip clipBehavior = Clip.antiAlias, EngineLayer oldLayer }) { + ClipPathEngineLayer pushClipPath(Path path, + {Clip clipBehavior = Clip.antiAlias, ClipPathEngineLayer oldLayer }) { assert(clipBehavior != null); assert(clipBehavior != Clip.none); return _pushSurface( @@ -172,8 +246,8 @@ class SceneBuilder { /// opacity). /// /// See [pop] for details about the operation stack. - EngineLayer pushOpacity(int alpha, - {Offset offset = Offset.zero, EngineLayer oldLayer}) { + OpacityEngineLayer pushOpacity(int alpha, + {Offset offset = Offset.zero, OpacityEngineLayer oldLayer}) { return _pushSurface(engine.PersistedOpacity(null, alpha, offset)); } @@ -183,7 +257,7 @@ class SceneBuilder { /// blend mode. /// /// See [pop] for details about the operation stack. - EngineLayer pushColorFilter(Color color, BlendMode blendMode, { EngineLayer oldLayer }) { + ColorFilterEngineLayer pushColorFilter(Color color, BlendMode blendMode, { ColorFilterEngineLayer oldLayer }) { throw new UnimplementedError(); } @@ -193,7 +267,7 @@ class SceneBuilder { /// rasterizing the given objects. /// /// See [pop] for details about the operation stack. - EngineLayer pushBackdropFilter(ImageFilter filter, { EngineLayer oldLayer }) { + BackdropFilterEngineLayer pushBackdropFilter(ImageFilter filter, { BackdropFilterEngineLayer oldLayer }) { throw new UnimplementedError(); } @@ -203,7 +277,7 @@ class SceneBuilder { /// rectangle using the given blend mode. /// /// See [pop] for details about the operation stack. - EngineLayer pushShaderMask(Shader shader, Rect maskRect, BlendMode blendMode, { EngineLayer oldLayer }) { + ShaderMaskEngineLayer pushShaderMask(Shader shader, Rect maskRect, BlendMode blendMode, { ShaderMaskEngineLayer oldLayer }) { throw new UnimplementedError(); } @@ -219,13 +293,13 @@ class SceneBuilder { /// color of the layer background. /// /// See [pop] for details about the operation stack, and [Clip] for different clip modes. - EngineLayer pushPhysicalShape({ + PhysicalShapeEngineLayer pushPhysicalShape({ Path path, double elevation, Color color, Color shadowColor, Clip clipBehavior = Clip.none, - EngineLayer oldLayer, + PhysicalShapeEngineLayer oldLayer, }) { return _pushSurface(engine.PersistedPhysicalShape( null, From 91ec97941a538c9bda64029940da650158291cec Mon Sep 17 00:00:00 2001 From: Yegor Jbanov Date: Tue, 25 Jun 2019 13:09:00 -0700 Subject: [PATCH 05/10] same changes in the CanvasKit prototype --- .../lib/src/engine/compositor/layer.dart | 6 +- .../compositor/layer_scene_builder.dart | 55 +++++++++---------- 2 files changed, 28 insertions(+), 33 deletions(-) diff --git a/lib/stub_ui/lib/src/engine/compositor/layer.dart b/lib/stub_ui/lib/src/engine/compositor/layer.dart index c03b52e175209..e400fb7b52c44 100644 --- a/lib/stub_ui/lib/src/engine/compositor/layer.dart +++ b/lib/stub_ui/lib/src/engine/compositor/layer.dart @@ -8,7 +8,7 @@ part of engine; /// /// A layer is the lowest-level rendering primitive. It represents an atomic /// painting command. -abstract class Layer { +abstract class Layer implements ui.EngineLayer { /// The layer that contains us as a child. ContainerLayer parent; @@ -173,7 +173,7 @@ class ClipRRectLayer extends ContainerLayer { } /// A layer that transforms its child layers by the given transform matrix. -class TransformLayer extends ContainerLayer { +class TransformLayer extends ContainerLayer implements ui.OffsetEngineLayer, ui.TransformEngineLayer { /// The matrix with which to transform the child layers. final Matrix4 _transform; @@ -287,7 +287,7 @@ class PictureLayer extends Layer { /// /// The shape clips its children to a given [Path], and casts a shadow based /// on the given elevation. -class PhysicalShapeLayer extends ContainerLayer { +class PhysicalShapeLayer extends ContainerLayer implements ui.PhysicalShapeEngineLayer { final double _elevation; final ui.Color _color; final ui.Color _shadowColor; diff --git a/lib/stub_ui/lib/src/engine/compositor/layer_scene_builder.dart b/lib/stub_ui/lib/src/engine/compositor/layer_scene_builder.dart index b0f2693acb6ae..31769a5c968e9 100644 --- a/lib/stub_ui/lib/src/engine/compositor/layer_scene_builder.dart +++ b/lib/stub_ui/lib/src/engine/compositor/layer_scene_builder.dart @@ -4,12 +4,6 @@ part of engine; -class EngineLayerImpl extends ui.EngineLayer { - final ContainerLayer _layer; - - EngineLayerImpl(this._layer); -} - class LayerScene implements ui.Scene { final LayerTree layerTree; @@ -60,7 +54,7 @@ class LayerSceneBuilder implements ui.SceneBuilder { @override void addRetained(ui.EngineLayer retainedLayer) { if (currentLayer == null) return; - currentLayer.add((retainedLayer as EngineLayerImpl)._layer); + currentLayer.add(retainedLayer); } @override @@ -95,80 +89,81 @@ class LayerSceneBuilder implements ui.SceneBuilder { } @override - ui.EngineLayer pushBackdropFilter(ui.ImageFilter filter, - {Object webOnlyPaintedBy}) { + ui.BackdropFilterEngineLayer pushBackdropFilter(ui.ImageFilter filter, + {ui.BackdropFilterEngineLayer oldLayer}) { throw new UnimplementedError(); } @override - ui.EngineLayer pushClipPath(ui.Path path, - {ui.Clip clipBehavior = ui.Clip.antiAlias, Object webOnlyPaintedBy}) { + ui.ClipPathEngineLayer pushClipPath(ui.Path path, + {ui.Clip clipBehavior = ui.Clip.antiAlias, ui.ClipPathEngineLayer oldLayer}) { pushLayer(ClipPathLayer(path)); return null; } @override - ui.EngineLayer pushClipRRect(ui.RRect rrect, - {ui.Clip clipBehavior, Object webOnlyPaintedBy}) { + ui.ClipRRectEngineLayer pushClipRRect(ui.RRect rrect, + {ui.Clip clipBehavior, ui.ClipRRectEngineLayer oldLayer}) { pushLayer(ClipRRectLayer(rrect)); return null; } @override - ui.EngineLayer pushClipRect(ui.Rect rect, - {ui.Clip clipBehavior = ui.Clip.antiAlias, Object webOnlyPaintedBy}) { + ui.ClipRectEngineLayer pushClipRect(ui.Rect rect, + {ui.Clip clipBehavior = ui.Clip.antiAlias, ui.ClipRectEngineLayer oldLayer}) { pushLayer(ClipRectLayer(rect)); return null; } @override - ui.EngineLayer pushColorFilter(ui.Color color, ui.BlendMode blendMode, - {Object webOnlyPaintedBy}) { + ui.ColorFilterEngineLayer pushColorFilter(ui.Color color, ui.BlendMode blendMode, + {ui.ColorFilterEngineLayer oldLayer}) { throw new UnimplementedError(); } @override - ui.EngineLayer pushOffset(double dx, double dy, {Object webOnlyPaintedBy}) { + ui.OffsetEngineLayer pushOffset(double dx, double dy, {ui.OffsetEngineLayer oldLayer}) { final matrix = Matrix4.translationValues(dx, dy, 0.0); final layer = TransformLayer(matrix); pushLayer(layer); - return EngineLayerImpl(layer); + return layer; } @override - ui.EngineLayer pushOpacity(int alpha, - {Object webOnlyPaintedBy, ui.Offset offset = ui.Offset.zero}) { + ui.OpacityEngineLayer pushOpacity(int alpha, + {ui.OpacityEngineLayer oldLayer, ui.Offset offset = ui.Offset.zero}) { // TODO(het): Implement opacity pushOffset(0.0, 0.0); return null; } @override - ui.EngineLayer pushPhysicalShape( + ui.PhysicalShapeEngineLayer pushPhysicalShape( {ui.Path path, double elevation, ui.Color color, ui.Color shadowColor, ui.Clip clipBehavior = ui.Clip.none, - Object webOnlyPaintedBy}) { + ui.PhysicalShapeEngineLayer oldLayer}) { final layer = PhysicalShapeLayer(elevation, color, shadowColor, path, clipBehavior); pushLayer(layer); - return EngineLayerImpl(layer); + return layer; } @override - ui.EngineLayer pushShaderMask( + ui.ShaderMaskEngineLayer pushShaderMask( ui.Shader shader, ui.Rect maskRect, ui.BlendMode blendMode, - {Object webOnlyPaintedBy}) { + {ui.ShaderMaskEngineLayer oldLayer}) { throw new UnimplementedError(); } @override - ui.EngineLayer pushTransform(Float64List matrix4, {Object webOnlyPaintedBy}) { - final matrix = Matrix4.fromList(matrix4); - pushLayer(TransformLayer(matrix)); - return null; + ui.TransformEngineLayer pushTransform(Float64List matrix4, {ui.TransformEngineLayer oldLayer}) { + final Matrix4 matrix = Matrix4.fromList(matrix4); + final TransformLayer layer = TransformLayer(matrix); + pushLayer(layer); + return layer; } @override From bd2eb940280b437378b8cb79bf086950f604be43 Mon Sep 17 00:00:00 2001 From: Yegor Jbanov Date: Wed, 26 Jun 2019 08:00:23 -0700 Subject: [PATCH 06/10] test double addRetained and double push --- testing/dart/compositing_test.dart | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/testing/dart/compositing_test.dart b/testing/dart/compositing_test.dart index 18511f832d4cb..ddc1bdbf0413a 100644 --- a/testing/dart/compositing_test.dart +++ b/testing/dart/compositing_test.dart @@ -94,6 +94,28 @@ void main() { expect(error.toString(), contains('The layer is already being used')); } builder3.build(); + + // Test: addRetained twice + final SceneBuilder builder4 = SceneBuilder(); + builder4.addRetained(layer); + try { + builder4.addRetained(layer); + fail('Expected second addRetained to throw AssertionError but it returned successully'); + } on AssertionError catch (error) { + expect(error.toString(), contains('The layer is already being used')); + } + builder4.build(); + + // Test: push twice + final SceneBuilder builder5 = SceneBuilder(); + pushFunction(builder5, layer); + try { + pushFunction(builder5, layer); + fail('Expected push to throw AssertionError but it returned successully'); + } on AssertionError catch (error) { + expect(error.toString(), contains('The layer is already being used')); + } + builder5.build(); } test('SceneBuilder does not share a layer between addRetained and push*', () { From caabbed04dd00e778ffdbe423948820d7fa18fe6 Mon Sep 17 00:00:00 2001 From: Yegor Jbanov Date: Wed, 26 Jun 2019 16:27:55 -0700 Subject: [PATCH 07/10] prevent double-use of retained descendant layers --- lib/ui/compositing.dart | 113 +++++++++++++++++++++++------ testing/dart/compositing_test.dart | 25 +++++++ 2 files changed, 116 insertions(+), 22 deletions(-) diff --git a/lib/ui/compositing.dart b/lib/ui/compositing.dart index 4e7aee12c4c6d..bf33d2b6e6979 100644 --- a/lib/ui/compositing.dart +++ b/lib/ui/compositing.dart @@ -49,6 +49,12 @@ abstract class _EngineLayerWrapper implements EngineLayer { _EngineLayerWrapper._(this._nativeLayer); EngineLayer _nativeLayer; + + // Children of this layer. + // + // Null if this layer has no children. This field is populated only in debug + // mode. + List<_EngineLayerWrapper> _debugChildren; } /// An opaque handle to a transform engine layer. @@ -166,19 +172,39 @@ class SceneBuilder extends NativeFieldWrapperClass2 { // In debug mode checks that the `layer` is only used once in a given scene. bool _debugCheckUsedOnce(EngineLayer layer, String usage) { - if (layer == null) { + assert(() { + if (layer == null) { + return true; + } + + assert( + !_usedLayers.containsKey(layer), + 'Layer ${layer.runtimeType} already used.\n' + 'The layer is already being used for ${_usedLayers[layer]} in this scene.\n' + 'A layer may only be used once in a given scene.' + ); + + _usedLayers[layer] = usage; return true; - } - - assert( - !_usedLayers.containsKey(layer), - 'Layer ${layer.runtimeType} already used.\n' - 'The layer is already being used for ${_usedLayers[layer]} in this scene.\n' - 'A layer may only be used once in a given scene.' - ); + }()); - _usedLayers[layer] = usage; + return true; + } + final List<_EngineLayerWrapper> _layerStack = <_EngineLayerWrapper>[]; + + // Pushes the `newLayer` onto the `_layerStack` and adds it to the + // `_debugChildren` of the current layer in the stack, if any. + bool _debugPushLayer(_EngineLayerWrapper newLayer) { + assert(() { + if (_layerStack.isNotEmpty) { + final _EngineLayerWrapper currentLayer = _layerStack.last; + currentLayer._debugChildren ??= <_EngineLayerWrapper>[]; + currentLayer._debugChildren.add(newLayer); + } + _layerStack.add(newLayer); + return true; + }()); return true; } @@ -196,7 +222,9 @@ class SceneBuilder extends NativeFieldWrapperClass2 { TransformEngineLayer pushTransform(Float64List matrix4, { TransformEngineLayer oldLayer }) { assert(_matrix4IsValid(matrix4)); assert(_debugCheckUsedOnce(oldLayer, 'oldLayer in pushTransform')); - return TransformEngineLayer._(_pushTransform(matrix4)); + final TransformEngineLayer layer = TransformEngineLayer._(_pushTransform(matrix4)); + assert(_debugPushLayer(layer)); + return layer; } EngineLayer _pushTransform(Float64List matrix4) native 'SceneBuilder_pushTransform'; @@ -209,7 +237,9 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// See [pop] for details about the operation stack. OffsetEngineLayer pushOffset(double dx, double dy, { OffsetEngineLayer oldLayer }) { assert(_debugCheckUsedOnce(oldLayer, 'oldLayer in pushOffset')); - return OffsetEngineLayer._(_pushOffset(dx, dy)); + final OffsetEngineLayer layer = OffsetEngineLayer._(_pushOffset(dx, dy)); + assert(_debugPushLayer(layer)); + return layer; } EngineLayer _pushOffset(double dx, double dy) native 'SceneBuilder_pushOffset'; @@ -225,7 +255,9 @@ class SceneBuilder extends NativeFieldWrapperClass2 { assert(clipBehavior != null); assert(clipBehavior != Clip.none); assert(_debugCheckUsedOnce(oldLayer, 'oldLayer in pushClipRect')); - return ClipRectEngineLayer._(_pushClipRect(rect.left, rect.right, rect.top, rect.bottom, clipBehavior.index)); + final ClipRectEngineLayer layer = ClipRectEngineLayer._(_pushClipRect(rect.left, rect.right, rect.top, rect.bottom, clipBehavior.index)); + assert(_debugPushLayer(layer)); + return layer; } EngineLayer _pushClipRect(double left, double right, @@ -245,7 +277,9 @@ class SceneBuilder extends NativeFieldWrapperClass2 { assert(clipBehavior != null); assert(clipBehavior != Clip.none); assert(_debugCheckUsedOnce(oldLayer, 'oldLayer in pushClipRRect')); - return ClipRRectEngineLayer._(_pushClipRRect(rrect._value32, clipBehavior.index)); + final ClipRRectEngineLayer layer = ClipRRectEngineLayer._(_pushClipRRect(rrect._value32, clipBehavior.index)); + assert(_debugPushLayer(layer)); + return layer; } EngineLayer _pushClipRRect(Float32List rrect, int clipBehavior) native 'SceneBuilder_pushClipRRect'; @@ -261,7 +295,9 @@ class SceneBuilder extends NativeFieldWrapperClass2 { assert(clipBehavior != null); assert(clipBehavior != Clip.none); assert(_debugCheckUsedOnce(oldLayer, 'oldLayer in pushClipPath')); - return ClipPathEngineLayer._(_pushClipPath(path, clipBehavior.index)); + final ClipPathEngineLayer layer = ClipPathEngineLayer._(_pushClipPath(path, clipBehavior.index)); + assert(_debugPushLayer(layer)); + return layer; } EngineLayer _pushClipPath(Path path, int clipBehavior) native 'SceneBuilder_pushClipPath'; @@ -277,7 +313,9 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// See [pop] for details about the operation stack. OpacityEngineLayer pushOpacity(int alpha, {Offset offset = Offset.zero, OpacityEngineLayer oldLayer}) { assert(_debugCheckUsedOnce(oldLayer, 'oldLayer in pushOpacity')); - return OpacityEngineLayer._(_pushOpacity(alpha, offset.dx, offset.dy)); + final OpacityEngineLayer layer = OpacityEngineLayer._(_pushOpacity(alpha, offset.dx, offset.dy)); + assert(_debugPushLayer(layer)); + return layer; } EngineLayer _pushOpacity(int alpha, double dx, double dy) native 'SceneBuilder_pushOpacity'; @@ -291,7 +329,9 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// See [pop] for details about the operation stack. ColorFilterEngineLayer pushColorFilter(Color color, BlendMode blendMode, { ColorFilterEngineLayer oldLayer }) { assert(_debugCheckUsedOnce(oldLayer, 'oldLayer in pushColorFilter')); - return ColorFilterEngineLayer._(_pushColorFilter(color.value, blendMode.index)); + final ColorFilterEngineLayer layer = ColorFilterEngineLayer._(_pushColorFilter(color.value, blendMode.index)); + assert(_debugPushLayer(layer)); + return layer; } EngineLayer _pushColorFilter(int color, int blendMode) native 'SceneBuilder_pushColorFilter'; @@ -305,7 +345,9 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// See [pop] for details about the operation stack. BackdropFilterEngineLayer pushBackdropFilter(ImageFilter filter, { BackdropFilterEngineLayer oldLayer }) { assert(_debugCheckUsedOnce(oldLayer, 'oldLayer in pushBackdropFilter')); - return BackdropFilterEngineLayer._(_pushBackdropFilter(filter)); + final BackdropFilterEngineLayer layer = BackdropFilterEngineLayer._(_pushBackdropFilter(filter)); + assert(_debugPushLayer(layer)); + return layer; } EngineLayer _pushBackdropFilter(ImageFilter filter) native 'SceneBuilder_pushBackdropFilter'; @@ -319,12 +361,14 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// See [pop] for details about the operation stack. ShaderMaskEngineLayer pushShaderMask(Shader shader, Rect maskRect, BlendMode blendMode, { ShaderMaskEngineLayer oldLayer }) { assert(_debugCheckUsedOnce(oldLayer, 'oldLayer in pushShaderMask')); - return ShaderMaskEngineLayer._(_pushShaderMask(shader, + final ShaderMaskEngineLayer layer = ShaderMaskEngineLayer._(_pushShaderMask(shader, maskRect.left, maskRect.right, maskRect.top, maskRect.bottom, blendMode.index)); + assert(_debugPushLayer(layer)); + return layer; } EngineLayer _pushShaderMask(Shader shader, double maskRectLeft, @@ -350,7 +394,9 @@ class SceneBuilder extends NativeFieldWrapperClass2 { // ignore: deprecated_member_use PhysicalShapeEngineLayer pushPhysicalShape({ Path path, double elevation, Color color, Color shadowColor, Clip clipBehavior = Clip.none, PhysicalShapeEngineLayer oldLayer }) { assert(_debugCheckUsedOnce(oldLayer, 'oldLayer in pushPhysicalShape')); - return PhysicalShapeEngineLayer._(_pushPhysicalShape(path, elevation, color.value, shadowColor?.value ?? 0xFF000000, clipBehavior.index)); + final PhysicalShapeEngineLayer layer = PhysicalShapeEngineLayer._(_pushPhysicalShape(path, elevation, color.value, shadowColor?.value ?? 0xFF000000, clipBehavior.index)); + assert(_debugPushLayer(layer)); + return layer; } EngineLayer _pushPhysicalShape(Path path, double elevation, int color, int shadowColor, int clipBehavior) native 'SceneBuilder_pushPhysicalShape'; @@ -361,7 +407,13 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// operations in the stack applies to each of the objects added to the scene. /// Calling this function removes the most recently added operation from the /// stack. - void pop() native 'SceneBuilder_pop'; + void pop() { + if (_layerStack.isNotEmpty) { + _layerStack.removeLast(); + } + _pop(); + } + void _pop() native 'SceneBuilder_pop'; /// Add a retained engine layer subtree from previous frames. /// @@ -373,7 +425,24 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// no need to call [addToScene] for its children layers. void addRetained(EngineLayer retainedLayer) { assert(retainedLayer is _EngineLayerWrapper); - assert(_debugCheckUsedOnce(retainedLayer, 'addRetained')); + assert(() { + _debugCheckUsedOnce(retainedLayer, 'addRetained'); + + void recursivelyCheckChildrenUsedOnce(_EngineLayerWrapper parentLayer) { + if (parentLayer._debugChildren == null || parentLayer._debugChildren.isEmpty) { + return; + } + + for (_EngineLayerWrapper childLayer in parentLayer._debugChildren) { + _debugCheckUsedOnce(childLayer, 'retaining an ancestor layer'); + } + } + + recursivelyCheckChildrenUsedOnce(retainedLayer); + + return true; + }()); + final _EngineLayerWrapper wrapper = retainedLayer; _addRetained(wrapper._nativeLayer); } diff --git a/testing/dart/compositing_test.dart b/testing/dart/compositing_test.dart index ddc1bdbf0413a..2210fdd3fb682 100644 --- a/testing/dart/compositing_test.dart +++ b/testing/dart/compositing_test.dart @@ -69,6 +69,8 @@ void main() { void testNoSharing(_TestNoSharingFunction pushFunction) { final SceneBuilder builder1 = SceneBuilder(); final EngineLayer layer = pushFunction(builder1, null); + final EngineLayer childLayer = builder1.pushOpacity(123); + builder1.pop(); builder1.pop(); builder1.build(); @@ -116,6 +118,29 @@ void main() { expect(error.toString(), contains('The layer is already being used')); } builder5.build(); + + // Test: child layer of a retained layer also pushed + final SceneBuilder builder6 = SceneBuilder(); + builder6.addRetained(layer); + try { + builder6.pushOpacity(321, oldLayer: childLayer); + fail('Expected pushOpacity to throw AssertionError but it returned successully'); + } on AssertionError catch (error) { + expect(error.toString(), contains('The layer is already being used')); + } + builder6.build(); + + // Test: pushed layer's parent being also added as retained + final SceneBuilder builder7 = SceneBuilder(); + builder7.pushOpacity(234, oldLayer: childLayer); + builder7.pop(); + try { + builder7.addRetained(layer); + fail('Expected addRetained to throw AssertionError but it returned successully'); + } on AssertionError catch (error) { + expect(error.toString(), contains('The layer is already being used')); + } + builder7.build(); } test('SceneBuilder does not share a layer between addRetained and push*', () { From a368d6e6a2230e0231901687006817f347bebd7c Mon Sep 17 00:00:00 2001 From: Yegor Jbanov Date: Thu, 27 Jun 2019 10:04:41 -0700 Subject: [PATCH 08/10] check that oldLayers are not reused across frames --- lib/ui/compositing.dart | 64 ++++++++--- testing/dart/compositing_test.dart | 173 +++++++++++++++++++++++------ 2 files changed, 190 insertions(+), 47 deletions(-) diff --git a/lib/ui/compositing.dart b/lib/ui/compositing.dart index bf33d2b6e6979..0f6e95e8c77cd 100644 --- a/lib/ui/compositing.dart +++ b/lib/ui/compositing.dart @@ -55,6 +55,24 @@ abstract class _EngineLayerWrapper implements EngineLayer { // Null if this layer has no children. This field is populated only in debug // mode. List<_EngineLayerWrapper> _debugChildren; + + // Whether this layer was used as `oldLayer` in a past frame. + // + // It is illegal to use a layer object again after it is passed as an + // `oldLayer` argument. + bool _debugWasUsedAsOldLayer = false; + + bool _debugCheckNotUsedAsOldLayer() { + assert( + !_debugWasUsedAsOldLayer, + 'Layer $runtimeType was previously used as oldLayer.\n' + 'Once a layer is used as oldLayer, it may not be used again. Instead, ' + 'after calling one of the SceneBuilder.push* methods and passing an oldLayer ' + 'to it, use the layer returned by the method as oldLayer in subsequent ' + 'frames.' + ); + return true; + } } /// An opaque handle to a transform engine layer. @@ -180,7 +198,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { assert( !_usedLayers.containsKey(layer), 'Layer ${layer.runtimeType} already used.\n' - 'The layer is already being used for ${_usedLayers[layer]} in this scene.\n' + 'The layer is already being used as ${_usedLayers[layer]} in this scene.\n' 'A layer may only be used once in a given scene.' ); @@ -191,6 +209,19 @@ class SceneBuilder extends NativeFieldWrapperClass2 { return true; } + bool _debugCheckCanBeUsedAsOldLayer(_EngineLayerWrapper layer, String methodName) { + assert(() { + if (layer == null) { + return true; + } + layer._debugCheckNotUsedAsOldLayer(); + assert(_debugCheckUsedOnce(layer, 'oldLayer in $methodName')); + layer._debugWasUsedAsOldLayer = true; + return true; + }()); + return true; + } + final List<_EngineLayerWrapper> _layerStack = <_EngineLayerWrapper>[]; // Pushes the `newLayer` onto the `_layerStack` and adds it to the @@ -221,7 +252,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// See [pop] for details about the operation stack. TransformEngineLayer pushTransform(Float64List matrix4, { TransformEngineLayer oldLayer }) { assert(_matrix4IsValid(matrix4)); - assert(_debugCheckUsedOnce(oldLayer, 'oldLayer in pushTransform')); + assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushTransform')); final TransformEngineLayer layer = TransformEngineLayer._(_pushTransform(matrix4)); assert(_debugPushLayer(layer)); return layer; @@ -236,7 +267,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// /// See [pop] for details about the operation stack. OffsetEngineLayer pushOffset(double dx, double dy, { OffsetEngineLayer oldLayer }) { - assert(_debugCheckUsedOnce(oldLayer, 'oldLayer in pushOffset')); + assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushOffset')); final OffsetEngineLayer layer = OffsetEngineLayer._(_pushOffset(dx, dy)); assert(_debugPushLayer(layer)); return layer; @@ -254,7 +285,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { ClipRectEngineLayer pushClipRect(Rect rect, {Clip clipBehavior = Clip.antiAlias, ClipRectEngineLayer oldLayer }) { assert(clipBehavior != null); assert(clipBehavior != Clip.none); - assert(_debugCheckUsedOnce(oldLayer, 'oldLayer in pushClipRect')); + assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushClipRect')); final ClipRectEngineLayer layer = ClipRectEngineLayer._(_pushClipRect(rect.left, rect.right, rect.top, rect.bottom, clipBehavior.index)); assert(_debugPushLayer(layer)); return layer; @@ -276,7 +307,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { ClipRRectEngineLayer pushClipRRect(RRect rrect, {Clip clipBehavior = Clip.antiAlias, ClipRRectEngineLayer oldLayer}) { assert(clipBehavior != null); assert(clipBehavior != Clip.none); - assert(_debugCheckUsedOnce(oldLayer, 'oldLayer in pushClipRRect')); + assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushClipRRect')); final ClipRRectEngineLayer layer = ClipRRectEngineLayer._(_pushClipRRect(rrect._value32, clipBehavior.index)); assert(_debugPushLayer(layer)); return layer; @@ -294,7 +325,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { ClipPathEngineLayer pushClipPath(Path path, {Clip clipBehavior = Clip.antiAlias, ClipPathEngineLayer oldLayer}) { assert(clipBehavior != null); assert(clipBehavior != Clip.none); - assert(_debugCheckUsedOnce(oldLayer, 'oldLayer in pushClipPath')); + assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushClipPath')); final ClipPathEngineLayer layer = ClipPathEngineLayer._(_pushClipPath(path, clipBehavior.index)); assert(_debugPushLayer(layer)); return layer; @@ -312,7 +343,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// /// See [pop] for details about the operation stack. OpacityEngineLayer pushOpacity(int alpha, {Offset offset = Offset.zero, OpacityEngineLayer oldLayer}) { - assert(_debugCheckUsedOnce(oldLayer, 'oldLayer in pushOpacity')); + assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushOpacity')); final OpacityEngineLayer layer = OpacityEngineLayer._(_pushOpacity(alpha, offset.dx, offset.dy)); assert(_debugPushLayer(layer)); return layer; @@ -328,7 +359,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// /// See [pop] for details about the operation stack. ColorFilterEngineLayer pushColorFilter(Color color, BlendMode blendMode, { ColorFilterEngineLayer oldLayer }) { - assert(_debugCheckUsedOnce(oldLayer, 'oldLayer in pushColorFilter')); + assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushColorFilter')); final ColorFilterEngineLayer layer = ColorFilterEngineLayer._(_pushColorFilter(color.value, blendMode.index)); assert(_debugPushLayer(layer)); return layer; @@ -344,7 +375,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// /// See [pop] for details about the operation stack. BackdropFilterEngineLayer pushBackdropFilter(ImageFilter filter, { BackdropFilterEngineLayer oldLayer }) { - assert(_debugCheckUsedOnce(oldLayer, 'oldLayer in pushBackdropFilter')); + assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushBackdropFilter')); final BackdropFilterEngineLayer layer = BackdropFilterEngineLayer._(_pushBackdropFilter(filter)); assert(_debugPushLayer(layer)); return layer; @@ -360,7 +391,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// /// See [pop] for details about the operation stack. ShaderMaskEngineLayer pushShaderMask(Shader shader, Rect maskRect, BlendMode blendMode, { ShaderMaskEngineLayer oldLayer }) { - assert(_debugCheckUsedOnce(oldLayer, 'oldLayer in pushShaderMask')); + assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushShaderMask')); final ShaderMaskEngineLayer layer = ShaderMaskEngineLayer._(_pushShaderMask(shader, maskRect.left, maskRect.right, @@ -393,7 +424,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// See [pop] for details about the operation stack, and [Clip] for different clip modes. // ignore: deprecated_member_use PhysicalShapeEngineLayer pushPhysicalShape({ Path path, double elevation, Color color, Color shadowColor, Clip clipBehavior = Clip.none, PhysicalShapeEngineLayer oldLayer }) { - assert(_debugCheckUsedOnce(oldLayer, 'oldLayer in pushPhysicalShape')); + assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushPhysicalShape')); final PhysicalShapeEngineLayer layer = PhysicalShapeEngineLayer._(_pushPhysicalShape(path, elevation, color.value, shadowColor?.value ?? 0xFF000000, clipBehavior.index)); assert(_debugPushLayer(layer)); return layer; @@ -426,19 +457,20 @@ class SceneBuilder extends NativeFieldWrapperClass2 { void addRetained(EngineLayer retainedLayer) { assert(retainedLayer is _EngineLayerWrapper); assert(() { - _debugCheckUsedOnce(retainedLayer, 'addRetained'); + final _EngineLayerWrapper layer = retainedLayer; void recursivelyCheckChildrenUsedOnce(_EngineLayerWrapper parentLayer) { + _debugCheckUsedOnce(parentLayer, 'retained layer'); + parentLayer._debugCheckNotUsedAsOldLayer(); + if (parentLayer._debugChildren == null || parentLayer._debugChildren.isEmpty) { return; } - for (_EngineLayerWrapper childLayer in parentLayer._debugChildren) { - _debugCheckUsedOnce(childLayer, 'retaining an ancestor layer'); - } + parentLayer._debugChildren.forEach(recursivelyCheckChildrenUsedOnce); } - recursivelyCheckChildrenUsedOnce(retainedLayer); + recursivelyCheckChildrenUsedOnce(layer); return true; }()); diff --git a/testing/dart/compositing_test.dart b/testing/dart/compositing_test.dart index 2210fdd3fb682..478f0aef6b35f 100644 --- a/testing/dart/compositing_test.dart +++ b/testing/dart/compositing_test.dart @@ -66,15 +66,13 @@ void main() { builder2.build(); }); - void testNoSharing(_TestNoSharingFunction pushFunction) { + // Attempts to use the same layer first as `oldLayer` then in `addRetained`. + void testPushThenIllegalRetain(_TestNoSharingFunction pushFunction) { final SceneBuilder builder1 = SceneBuilder(); final EngineLayer layer = pushFunction(builder1, null); - final EngineLayer childLayer = builder1.pushOpacity(123); - builder1.pop(); builder1.pop(); builder1.build(); - // Test: first push then attempt illegal addRetained final SceneBuilder builder2 = SceneBuilder(); pushFunction(builder2, layer); builder2.pop(); @@ -85,62 +83,175 @@ void main() { expect(error.toString(), contains('The layer is already being used')); } builder2.build(); + } - // Test: first addRetained then attempt illegal push - final SceneBuilder builder3 = SceneBuilder(); - builder3.addRetained(layer); + // Attempts to use the same layer first in `addRetained` then as `oldLayer`. + void testAddRetainedThenIllegalPush(_TestNoSharingFunction pushFunction) { + final SceneBuilder builder1 = SceneBuilder(); + final EngineLayer layer = pushFunction(builder1, null); + builder1.pop(); + builder1.build(); + + final SceneBuilder builder2 = SceneBuilder(); + builder2.addRetained(layer); try { - pushFunction(builder3, layer); + pushFunction(builder2, layer); fail('Expected push to throw AssertionError but it returned successully'); } on AssertionError catch (error) { expect(error.toString(), contains('The layer is already being used')); } - builder3.build(); + builder2.build(); + } - // Test: addRetained twice - final SceneBuilder builder4 = SceneBuilder(); - builder4.addRetained(layer); + // Attempts to retain the same layer twice in the same scene. + void testDoubleAddRetained(_TestNoSharingFunction pushFunction) { + final SceneBuilder builder1 = SceneBuilder(); + final EngineLayer layer = pushFunction(builder1, null); + builder1.pop(); + builder1.build(); + + final SceneBuilder builder2 = SceneBuilder(); + builder2.addRetained(layer); try { - builder4.addRetained(layer); + builder2.addRetained(layer); fail('Expected second addRetained to throw AssertionError but it returned successully'); } on AssertionError catch (error) { expect(error.toString(), contains('The layer is already being used')); } - builder4.build(); + builder2.build(); + } - // Test: push twice - final SceneBuilder builder5 = SceneBuilder(); - pushFunction(builder5, layer); + // Attempts to use the same layer as `oldLayer` twice in the same scene. + void testPushOldLayerTwice(_TestNoSharingFunction pushFunction) { + final SceneBuilder builder1 = SceneBuilder(); + final EngineLayer layer = pushFunction(builder1, null); + builder1.pop(); + builder1.build(); + + final SceneBuilder builder2 = SceneBuilder(); + pushFunction(builder2, layer); try { - pushFunction(builder5, layer); + pushFunction(builder2, layer); fail('Expected push to throw AssertionError but it returned successully'); } on AssertionError catch (error) { - expect(error.toString(), contains('The layer is already being used')); + expect(error.toString(), contains('was previously used as oldLayer')); } - builder5.build(); + builder2.build(); + } + + // Attempts to use a child of a retained layer as an `oldLayer`. + void testPushChildLayerOfRetainedLayer(_TestNoSharingFunction pushFunction) { + final SceneBuilder builder1 = SceneBuilder(); + final EngineLayer layer = pushFunction(builder1, null); + final EngineLayer childLayer = builder1.pushOpacity(123); + builder1.pop(); + builder1.pop(); + builder1.build(); - // Test: child layer of a retained layer also pushed - final SceneBuilder builder6 = SceneBuilder(); - builder6.addRetained(layer); + final SceneBuilder builder2 = SceneBuilder(); + builder2.addRetained(layer); try { - builder6.pushOpacity(321, oldLayer: childLayer); + builder2.pushOpacity(321, oldLayer: childLayer); fail('Expected pushOpacity to throw AssertionError but it returned successully'); } on AssertionError catch (error) { expect(error.toString(), contains('The layer is already being used')); } - builder6.build(); + builder2.build(); + } + + // Attempts to retain a layer whose child is already used as `oldLayer` elsewhere in the scene. + void testRetainParentLayerOfPushedChild(_TestNoSharingFunction pushFunction) { + final SceneBuilder builder1 = SceneBuilder(); + final EngineLayer layer = pushFunction(builder1, null); + final EngineLayer childLayer = builder1.pushOpacity(123); + builder1.pop(); + builder1.pop(); + builder1.build(); - // Test: pushed layer's parent being also added as retained - final SceneBuilder builder7 = SceneBuilder(); - builder7.pushOpacity(234, oldLayer: childLayer); - builder7.pop(); + final SceneBuilder builder2 = SceneBuilder(); + builder2.pushOpacity(234, oldLayer: childLayer); + builder2.pop(); try { - builder7.addRetained(layer); + builder2.addRetained(layer); fail('Expected addRetained to throw AssertionError but it returned successully'); } on AssertionError catch (error) { expect(error.toString(), contains('The layer is already being used')); } - builder7.build(); + builder2.build(); + } + + // Attempts to retain a layer that has been used as `oldLayer` in a previous frame. + void testRetainOldLayer(_TestNoSharingFunction pushFunction) { + final SceneBuilder builder1 = SceneBuilder(); + final EngineLayer layer = pushFunction(builder1, null); + builder1.pop(); + builder1.build(); + + final SceneBuilder builder2 = SceneBuilder(); + pushFunction(builder2, layer); + builder2.pop(); + try { + final SceneBuilder builder3 = SceneBuilder(); + builder3.addRetained(layer); + fail('Expected addRetained to throw AssertionError but it returned successully'); + } on AssertionError catch (error) { + expect(error.toString(), contains('was previously used as oldLayer')); + } + builder2.build(); + } + + // Attempts to pass layer as `oldLayer` that has been used as `oldLayer` in a previous frame. + void testPushOldLayer(_TestNoSharingFunction pushFunction) { + final SceneBuilder builder1 = SceneBuilder(); + final EngineLayer layer = pushFunction(builder1, null); + builder1.pop(); + builder1.build(); + + final SceneBuilder builder2 = SceneBuilder(); + pushFunction(builder2, layer); + builder2.pop(); + try { + final SceneBuilder builder3 = SceneBuilder(); + pushFunction(builder3, layer); + fail('Expected addRetained to throw AssertionError but it returned successully'); + } on AssertionError catch (error) { + expect(error.toString(), contains('was previously used as oldLayer')); + } + builder2.build(); + } + + // Attempts to retain a parent of a layer used as `oldLayer` in a previous frame. + void testRetainsParentOfOldLayer(_TestNoSharingFunction pushFunction) { + final SceneBuilder builder1 = SceneBuilder(); + final EngineLayer parentLayer = pushFunction(builder1, null); + final OpacityEngineLayer childLayer = builder1.pushOpacity(123); + builder1.pop(); + builder1.pop(); + builder1.build(); + + final SceneBuilder builder2 = SceneBuilder(); + builder2.pushOpacity(321, oldLayer: childLayer); + builder2.pop(); + try { + final SceneBuilder builder3 = SceneBuilder(); + builder3.addRetained(parentLayer); + fail('Expected addRetained to throw AssertionError but it returned successully'); + } on AssertionError catch (error) { + expect(error.toString(), contains('was previously used as oldLayer')); + } + builder2.build(); + } + + void testNoSharing(_TestNoSharingFunction pushFunction) { + testPushThenIllegalRetain(pushFunction); + testAddRetainedThenIllegalPush(pushFunction); + testDoubleAddRetained(pushFunction); + testPushOldLayerTwice(pushFunction); + testPushChildLayerOfRetainedLayer(pushFunction); + testRetainParentLayerOfPushedChild(pushFunction); + testRetainOldLayer(pushFunction); + testPushOldLayer(pushFunction); + testRetainsParentOfOldLayer(pushFunction); } test('SceneBuilder does not share a layer between addRetained and push*', () { From 809686a93493222db7aba405097ac7cd05943e8c Mon Sep 17 00:00:00 2001 From: Yegor Jbanov Date: Thu, 27 Jun 2019 12:45:34 -0700 Subject: [PATCH 09/10] more docs --- lib/ui/compositing.dart | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/lib/ui/compositing.dart b/lib/ui/compositing.dart index 0f6e95e8c77cd..1a73bbebf4346 100644 --- a/lib/ui/compositing.dart +++ b/lib/ui/compositing.dart @@ -249,6 +249,23 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// an optimization. It has no effect on the correctness of rendering. /// {@endtemplate} /// + /// {@template dart.ui.sceneBuilder.oldLayerVsRetained} + /// Passing a layer to [addRetained] or as `oldLayer` argument to a push + /// method counts as _usage_. A layer be used no more than once in a scene. + /// For example, it may not be passed simultaneously to two push methods, or + /// to a push method and to `addRetained`. + /// + /// When a layer is passed to [addRetained] all descendant layers are also + /// considered as used in this scene. The same single-usage restriction + /// applies to descendants. + /// + /// When a layer is passed as an `oldLayer` argument to a push method, it may + /// no longer be used in subsequent frames. If you would like to continue + /// reusing the resources associated layer, store the layer object returned + /// by the push method and use that in the next frame instead of the original + /// object. + /// {@endtemplate} + /// /// See [pop] for details about the operation stack. TransformEngineLayer pushTransform(Float64List matrix4, { TransformEngineLayer oldLayer }) { assert(_matrix4IsValid(matrix4)); @@ -265,6 +282,8 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// /// {@macro dart.ui.sceneBuilder.oldLayer} /// + /// {@macro dart.ui.sceneBuilder.oldLayerVsRetained} + /// /// See [pop] for details about the operation stack. OffsetEngineLayer pushOffset(double dx, double dy, { OffsetEngineLayer oldLayer }) { assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushOffset')); @@ -280,6 +299,8 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// /// {@macro dart.ui.sceneBuilder.oldLayer} /// + /// {@macro dart.ui.sceneBuilder.oldLayerVsRetained} + /// /// See [pop] for details about the operation stack, and [Clip] for different clip modes. /// By default, the clip will be anti-aliased (clip = [Clip.antiAlias]). ClipRectEngineLayer pushClipRect(Rect rect, {Clip clipBehavior = Clip.antiAlias, ClipRectEngineLayer oldLayer }) { @@ -302,6 +323,8 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// /// {@macro dart.ui.sceneBuilder.oldLayer} /// + /// {@macro dart.ui.sceneBuilder.oldLayerVsRetained} + /// /// See [pop] for details about the operation stack, and [Clip] for different clip modes. /// By default, the clip will be anti-aliased (clip = [Clip.antiAlias]). ClipRRectEngineLayer pushClipRRect(RRect rrect, {Clip clipBehavior = Clip.antiAlias, ClipRRectEngineLayer oldLayer}) { @@ -320,6 +343,8 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// /// {@macro dart.ui.sceneBuilder.oldLayer} /// + /// {@macro dart.ui.sceneBuilder.oldLayerVsRetained} + /// /// See [pop] for details about the operation stack. See [Clip] for different clip modes. /// By default, the clip will be anti-aliased (clip = [Clip.antiAlias]). ClipPathEngineLayer pushClipPath(Path path, {Clip clipBehavior = Clip.antiAlias, ClipPathEngineLayer oldLayer}) { @@ -341,6 +366,8 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// /// {@macro dart.ui.sceneBuilder.oldLayer} /// + /// {@macro dart.ui.sceneBuilder.oldLayerVsRetained} + /// /// See [pop] for details about the operation stack. OpacityEngineLayer pushOpacity(int alpha, {Offset offset = Offset.zero, OpacityEngineLayer oldLayer}) { assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushOpacity')); @@ -357,6 +384,8 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// /// {@macro dart.ui.sceneBuilder.oldLayer} /// + /// {@macro dart.ui.sceneBuilder.oldLayerVsRetained} + /// /// See [pop] for details about the operation stack. ColorFilterEngineLayer pushColorFilter(Color color, BlendMode blendMode, { ColorFilterEngineLayer oldLayer }) { assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushColorFilter')); @@ -373,6 +402,8 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// /// {@macro dart.ui.sceneBuilder.oldLayer} /// + /// {@macro dart.ui.sceneBuilder.oldLayerVsRetained} + /// /// See [pop] for details about the operation stack. BackdropFilterEngineLayer pushBackdropFilter(ImageFilter filter, { BackdropFilterEngineLayer oldLayer }) { assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushBackdropFilter')); @@ -389,6 +420,8 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// /// {@macro dart.ui.sceneBuilder.oldLayer} /// + /// {@macro dart.ui.sceneBuilder.oldLayerVsRetained} + /// /// See [pop] for details about the operation stack. ShaderMaskEngineLayer pushShaderMask(Shader shader, Rect maskRect, BlendMode blendMode, { ShaderMaskEngineLayer oldLayer }) { assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushShaderMask')); @@ -421,6 +454,8 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// /// {@macro dart.ui.sceneBuilder.oldLayer} /// + /// {@macro dart.ui.sceneBuilder.oldLayerVsRetained} + /// /// See [pop] for details about the operation stack, and [Clip] for different clip modes. // ignore: deprecated_member_use PhysicalShapeEngineLayer pushPhysicalShape({ Path path, double elevation, Color color, Color shadowColor, Clip clipBehavior = Clip.none, PhysicalShapeEngineLayer oldLayer }) { @@ -454,6 +489,8 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// Therefore, when implementing a subclass of the [Layer] concept defined in /// the rendering layer of Flutter's framework, once this is called, there's /// no need to call [addToScene] for its children layers. + /// + /// {@macro dart.ui.sceneBuilder.oldLayerVsRetained} void addRetained(EngineLayer retainedLayer) { assert(retainedLayer is _EngineLayerWrapper); assert(() { From 0e62b95841e15223961e24d2563f53291370dcda Mon Sep 17 00:00:00 2001 From: Yegor Jbanov Date: Thu, 27 Jun 2019 15:47:53 -0700 Subject: [PATCH 10/10] doc fixes --- lib/ui/compositing.dart | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/ui/compositing.dart b/lib/ui/compositing.dart index 1a73bbebf4346..357952087e2ff 100644 --- a/lib/ui/compositing.dart +++ b/lib/ui/compositing.dart @@ -251,7 +251,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// /// {@template dart.ui.sceneBuilder.oldLayerVsRetained} /// Passing a layer to [addRetained] or as `oldLayer` argument to a push - /// method counts as _usage_. A layer be used no more than once in a scene. + /// method counts as _usage_. A layer can be used no more than once in a scene. /// For example, it may not be passed simultaneously to two push methods, or /// to a push method and to `addRetained`. /// @@ -261,9 +261,9 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// /// When a layer is passed as an `oldLayer` argument to a push method, it may /// no longer be used in subsequent frames. If you would like to continue - /// reusing the resources associated layer, store the layer object returned - /// by the push method and use that in the next frame instead of the original - /// object. + /// reusing the resources associated with the layer, store the layer object + /// returned by the push method and use that in the next frame instead of the + /// original object. /// {@endtemplate} /// /// See [pop] for details about the operation stack.