Return EngineLayer for all SceneBuilder pushXXX (#6814)

Most changes are trivial except pushTransform. In pushTransform, matrix4 is an Dart object and it has to be released before we can return a new Dart object (EngineLayer).

lib/ui/compositing.dart

@@ -56,14 +56,14 @@ class SceneBuilder extends NativeFieldWrapperClass2 {
   /// The objects are transformed by the given matrix before rasterization.
   ///
   /// See [pop] for details about the operation stack.
-  void pushTransform(Float64List matrix4) {
+  EngineLayer pushTransform(Float64List matrix4) {
     if (matrix4 == null)
       throw new ArgumentError('"matrix4" argument cannot be null');
     if (matrix4.length != 16)
       throw new ArgumentError('"matrix4" must have 16 entries.');
-    _pushTransform(matrix4);
+    return _pushTransform(matrix4);
   }
-  void _pushTransform(Float64List matrix4) native 'SceneBuilder_pushTransform';
+  EngineLayer _pushTransform(Float64List matrix4) native 'SceneBuilder_pushTransform';
 
   /// Pushes an offset operation onto the operation stack.
   ///
@@ -78,16 +78,16 @@ class SceneBuilder extends NativeFieldWrapperClass2 {
   ///
   /// 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]).
-  void pushClipRect(Rect rect, {Clip clipBehavior = Clip.antiAlias}) {
+  EngineLayer pushClipRect(Rect rect, {Clip clipBehavior = Clip.antiAlias}) {
     assert(clipBehavior != null);
     assert(clipBehavior != Clip.none);
-    _pushClipRect(rect.left, rect.right, rect.top, rect.bottom, clipBehavior.index);
+    return _pushClipRect(rect.left, rect.right, rect.top, rect.bottom, clipBehavior.index);
   }
-  void _pushClipRect(double left,
-                     double right,
-                     double top,
-                     double bottom,
-                     int clipBehavior) native 'SceneBuilder_pushClipRect';
+  EngineLayer _pushClipRect(double left,
+                            double right,
+                            double top,
+                            double bottom,
+                            int clipBehavior) native 'SceneBuilder_pushClipRect';
 
   /// Pushes a rounded-rectangular clip operation onto the operation stack.
   ///
@@ -95,12 +95,12 @@ class SceneBuilder extends NativeFieldWrapperClass2 {
   ///
   /// 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]).
-  void pushClipRRect(RRect rrect, {Clip clipBehavior = Clip.antiAlias}) {
+  EngineLayer pushClipRRect(RRect rrect, {Clip clipBehavior = Clip.antiAlias}) {
     assert(clipBehavior != null);
     assert(clipBehavior != Clip.none);
-    _pushClipRRect(rrect._value, clipBehavior.index);
+    return _pushClipRRect(rrect._value, clipBehavior.index);
   }
-  void _pushClipRRect(Float32List rrect, int clipBehavior) native 'SceneBuilder_pushClipRRect';
+  EngineLayer _pushClipRRect(Float32List rrect, int clipBehavior) native 'SceneBuilder_pushClipRRect';
 
   /// Pushes a path clip operation onto the operation stack.
   ///
@@ -108,12 +108,12 @@ class SceneBuilder extends NativeFieldWrapperClass2 {
   ///
   /// 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]).
-  void pushClipPath(Path path, {Clip clipBehavior = Clip.antiAlias}) {
+  EngineLayer pushClipPath(Path path, {Clip clipBehavior = Clip.antiAlias}) {
     assert(clipBehavior != null);
     assert(clipBehavior != Clip.none);
-    _pushClipPath(path, clipBehavior.index);
+    return _pushClipPath(path, clipBehavior.index);
   }
-  void _pushClipPath(Path path, int clipBehavior) native 'SceneBuilder_pushClipPath';
+  EngineLayer _pushClipPath(Path path, int clipBehavior) native 'SceneBuilder_pushClipPath';
 
   /// Pushes an opacity operation onto the operation stack.
   ///
@@ -123,10 +123,10 @@ class SceneBuilder extends NativeFieldWrapperClass2 {
   /// opacity).
   ///
   /// See [pop] for details about the operation stack.
-  void pushOpacity(int alpha, {Offset offset = Offset.zero}) {
-    _pushOpacity(alpha, offset.dx, offset.dy);
+  EngineLayer pushOpacity(int alpha, {Offset offset = Offset.zero}) {
+    return _pushOpacity(alpha, offset.dx, offset.dy);
   }
-  void _pushOpacity(int alpha, double dx, double dy) native 'SceneBuilder_pushOpacity';
+  EngineLayer _pushOpacity(int alpha, double dx, double dy) native 'SceneBuilder_pushOpacity';
 
   /// Pushes a color filter operation onto the operation stack.
   ///
@@ -134,10 +134,10 @@ class SceneBuilder extends NativeFieldWrapperClass2 {
   /// blend mode.
   ///
   /// See [pop] for details about the operation stack.
-  void pushColorFilter(Color color, BlendMode blendMode) {
-    _pushColorFilter(color.value, blendMode.index);
+  EngineLayer pushColorFilter(Color color, BlendMode blendMode) {
+    return _pushColorFilter(color.value, blendMode.index);
   }
-  void _pushColorFilter(int color, int blendMode) native 'SceneBuilder_pushColorFilter';
+  EngineLayer _pushColorFilter(int color, int blendMode) native 'SceneBuilder_pushColorFilter';
 
   /// Pushes a backdrop filter operation onto the operation stack.
   ///
@@ -145,7 +145,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 {
   /// rasterizing the given objects.
   ///
   /// See [pop] for details about the operation stack.
-  void pushBackdropFilter(ImageFilter filter) native 'SceneBuilder_pushBackdropFilter';
+  EngineLayer pushBackdropFilter(ImageFilter filter) native 'SceneBuilder_pushBackdropFilter';
 
   /// Pushes a shader mask operation onto the operation stack.
   ///
@@ -153,20 +153,20 @@ class SceneBuilder extends NativeFieldWrapperClass2 {
   /// rectangle using the given blend mode.
   ///
   /// See [pop] for details about the operation stack.
-  void pushShaderMask(Shader shader, Rect maskRect, BlendMode blendMode) {
-    _pushShaderMask(shader,
-                    maskRect.left,
-                    maskRect.right,
-                    maskRect.top,
-                    maskRect.bottom,
-                    blendMode.index);
+  EngineLayer pushShaderMask(Shader shader, Rect maskRect, BlendMode blendMode) {
+    return _pushShaderMask(shader,
+                           maskRect.left,
+                           maskRect.right,
+                           maskRect.top,
+                           maskRect.bottom,
+                           blendMode.index);
   }
-  void _pushShaderMask(Shader shader,
-                       double maskRectLeft,
-                       double maskRectRight,
-                       double maskRectTop,
-                       double maskRectBottom,
-                       int blendMode) native 'SceneBuilder_pushShaderMask';
+  EngineLayer _pushShaderMask(Shader shader,
+                              double maskRectLeft,
+                              double maskRectRight,
+                              double maskRectTop,
+                              double maskRectBottom,
+                              int blendMode) native 'SceneBuilder_pushShaderMask';
 
   /// Pushes a physical layer operation for an arbitrary shape onto the
   /// operation stack.

lib/ui/compositing/scene_builder.cc

@@ -76,11 +76,15 @@ void SceneBuilder::RegisterNatives(tonic::DartLibraryNatives* natives) {
 SceneBuilder::SceneBuilder() = default;
 SceneBuilder::~SceneBuilder() = default;
 
-void SceneBuilder::pushTransform(const tonic::Float64List& matrix4) {
+fml::RefPtr<EngineLayer> SceneBuilder::pushTransform(
+    tonic::Float64List& matrix4) {
   SkMatrix sk_matrix = ToSkMatrix(matrix4);
-  auto layer = std::make_unique<flow::TransformLayer>();
+  auto layer = std::make_shared<flow::TransformLayer>();
   layer->set_transform(sk_matrix);
-  PushLayer(std::move(layer));
+  PushLayer(layer);
+  // matrix4 has to be released before we can return another Dart object
+  matrix4.Release();
+  return EngineLayer::MakeRetained(layer);
 }
 
 fml::RefPtr<EngineLayer> SceneBuilder::pushOffset(double dx, double dy) {
@@ -91,66 +95,78 @@ fml::RefPtr<EngineLayer> SceneBuilder::pushOffset(double dx, double dy) {
   return EngineLayer::MakeRetained(layer);
 }
 
-void SceneBuilder::pushClipRect(double left,
-                                double right,
-                                double top,
-                                double bottom,
-                                int clipBehavior) {
+fml::RefPtr<EngineLayer> SceneBuilder::pushClipRect(double left,
+                                                    double right,
+                                                    double top,
+                                                    double bottom,
+                                                    int clipBehavior) {
   SkRect clipRect = SkRect::MakeLTRB(left, top, right, bottom);
   flow::Clip clip_behavior = static_cast<flow::Clip>(clipBehavior);
-  auto layer = std::make_unique<flow::ClipRectLayer>(clip_behavior);
+  auto layer = std::make_shared<flow::ClipRectLayer>(clip_behavior);
   layer->set_clip_rect(clipRect);
-  PushLayer(std::move(layer));
+  PushLayer(layer);
+  return EngineLayer::MakeRetained(layer);
 }
 
-void SceneBuilder::pushClipRRect(const RRect& rrect, int clipBehavior) {
+fml::RefPtr<EngineLayer> SceneBuilder::pushClipRRect(const RRect& rrect,
+                                                     int clipBehavior) {
   flow::Clip clip_behavior = static_cast<flow::Clip>(clipBehavior);
-  auto layer = std::make_unique<flow::ClipRRectLayer>(clip_behavior);
+  auto layer = std::make_shared<flow::ClipRRectLayer>(clip_behavior);
   layer->set_clip_rrect(rrect.sk_rrect);
-  PushLayer(std::move(layer));
+  PushLayer(layer);
+  return EngineLayer::MakeRetained(layer);
 }
 
-void SceneBuilder::pushClipPath(const CanvasPath* path, int clipBehavior) {
+fml::RefPtr<EngineLayer> SceneBuilder::pushClipPath(const CanvasPath* path,
+                                                    int clipBehavior) {
   flow::Clip clip_behavior = static_cast<flow::Clip>(clipBehavior);
   FML_DCHECK(clip_behavior != flow::Clip::none);
-  auto layer = std::make_unique<flow::ClipPathLayer>(clip_behavior);
+  auto layer = std::make_shared<flow::ClipPathLayer>(clip_behavior);
   layer->set_clip_path(path->path());
-  PushLayer(std::move(layer));
+  PushLayer(layer);
+  return EngineLayer::MakeRetained(layer);
 }
 
-void SceneBuilder::pushOpacity(int alpha, double dx, double dy) {
-  auto layer = std::make_unique<flow::OpacityLayer>();
+fml::RefPtr<EngineLayer> SceneBuilder::pushOpacity(int alpha,
+                                                   double dx,
+                                                   double dy) {
+  auto layer = std::make_shared<flow::OpacityLayer>();
   layer->set_alpha(alpha);
   layer->set_offset(SkPoint::Make(dx, dy));
-  PushLayer(std::move(layer));
+  PushLayer(layer);
+  return EngineLayer::MakeRetained(layer);
 }
 
-void SceneBuilder::pushColorFilter(int color, int blendMode) {
-  auto layer = std::make_unique<flow::ColorFilterLayer>();
+fml::RefPtr<EngineLayer> SceneBuilder::pushColorFilter(int color,
+                                                       int blendMode) {
+  auto layer = std::make_shared<flow::ColorFilterLayer>();
   layer->set_color(static_cast<SkColor>(color));
   layer->set_blend_mode(static_cast<SkBlendMode>(blendMode));
-  PushLayer(std::move(layer));
+  PushLayer(layer);
+  return EngineLayer::MakeRetained(layer);
 }
 
-void SceneBuilder::pushBackdropFilter(ImageFilter* filter) {
-  auto layer = std::make_unique<flow::BackdropFilterLayer>();
+fml::RefPtr<EngineLayer> SceneBuilder::pushBackdropFilter(ImageFilter* filter) {
+  auto layer = std::make_shared<flow::BackdropFilterLayer>();
   layer->set_filter(filter->filter());
-  PushLayer(std::move(layer));
+  PushLayer(layer);
+  return EngineLayer::MakeRetained(layer);
 }
 
-void SceneBuilder::pushShaderMask(Shader* shader,
-                                  double maskRectLeft,
-                                  double maskRectRight,
-                                  double maskRectTop,
-                                  double maskRectBottom,
-                                  int blendMode) {
+fml::RefPtr<EngineLayer> SceneBuilder::pushShaderMask(Shader* shader,
+                                                      double maskRectLeft,
+                                                      double maskRectRight,
+                                                      double maskRectTop,
+                                                      double maskRectBottom,
+                                                      int blendMode) {
   SkRect rect = SkRect::MakeLTRB(maskRectLeft, maskRectTop, maskRectRight,
                                  maskRectBottom);
-  auto layer = std::make_unique<flow::ShaderMaskLayer>();
+  auto layer = std::make_shared<flow::ShaderMaskLayer>();
   layer->set_shader(shader->shader());
   layer->set_mask_rect(rect);
   layer->set_blend_mode(static_cast<SkBlendMode>(blendMode));
-  PushLayer(std::move(layer));
+  PushLayer(layer);
+  return EngineLayer::MakeRetained(layer);
 }
 
 fml::RefPtr<EngineLayer> SceneBuilder::pushPhysicalShape(const CanvasPath* path,

lib/ui/compositing/scene_builder.h

@@ -33,24 +33,25 @@ class SceneBuilder : public RefCountedDartWrappable<SceneBuilder> {
 
   ~SceneBuilder() override;
 
-  void pushTransform(const tonic::Float64List& matrix4);
+  fml::RefPtr<EngineLayer> pushTransform(tonic::Float64List& matrix4);
   fml::RefPtr<EngineLayer> pushOffset(double dx, double dy);
-  void pushClipRect(double left,
-                    double right,
-                    double top,
-                    double bottom,
-                    int clipBehavior);
-  void pushClipRRect(const RRect& rrect, int clipBehavior);
-  void pushClipPath(const CanvasPath* path, int clipBehavior);
-  void pushOpacity(int alpha, double dx = 0, double dy = 0);
-  void pushColorFilter(int color, int blendMode);
-  void pushBackdropFilter(ImageFilter* filter);
-  void pushShaderMask(Shader* shader,
-                      double maskRectLeft,
-                      double maskRectRight,
-                      double maskRectTop,
-                      double maskRectBottom,
-                      int blendMode);
+  fml::RefPtr<EngineLayer> pushClipRect(double left,
+                                        double right,
+                                        double top,
+                                        double bottom,
+                                        int clipBehavior);
+  fml::RefPtr<EngineLayer> pushClipRRect(const RRect& rrect, int clipBehavior);
+  fml::RefPtr<EngineLayer> pushClipPath(const CanvasPath* path,
+                                        int clipBehavior);
+  fml::RefPtr<EngineLayer> pushOpacity(int alpha, double dx = 0, double dy = 0);
+  fml::RefPtr<EngineLayer> pushColorFilter(int color, int blendMode);
+  fml::RefPtr<EngineLayer> pushBackdropFilter(ImageFilter* filter);
+  fml::RefPtr<EngineLayer> pushShaderMask(Shader* shader,
+                                          double maskRectLeft,
+                                          double maskRectRight,
+                                          double maskRectTop,
+                                          double maskRectBottom,
+                                          int blendMode);
   fml::RefPtr<EngineLayer> pushPhysicalShape(const CanvasPath* path,
                                              double elevation,
                                              int color,