From 9394ba2cb2125096629d6545fe32bfc192785699 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 20 Feb 2020 12:10:04 -0800 Subject: [PATCH 1/8] Reland --- lib/ui/compositing.dart | 95 +++++++++++++++++++--------- lib/ui/compositing/scene.cc | 12 ++-- lib/ui/compositing/scene.h | 9 +-- lib/ui/compositing/scene_builder.cc | 98 +++++++++++++++-------------- lib/ui/compositing/scene_builder.h | 63 +++++++++++-------- lib/ui/painting.dart | 14 ++++- lib/ui/painting/engine_layer.h | 7 +++ lib/ui/painting/picture.cc | 6 +- lib/ui/painting/picture.h | 3 +- lib/ui/painting/picture_recorder.cc | 8 ++- lib/ui/painting/picture_recorder.h | 2 +- third_party/tonic/dart_state.cc | 8 +++ third_party/tonic/dart_state.h | 6 ++ third_party/tonic/dart_wrappable.cc | 31 +++++++-- third_party/tonic/dart_wrappable.h | 4 ++ 15 files changed, 244 insertions(+), 122 deletions(-) diff --git a/lib/ui/compositing.dart b/lib/ui/compositing.dart index 01a3550ae9dce..bf223d9ef9d51 100644 --- a/lib/ui/compositing.dart +++ b/lib/ui/compositing.dart @@ -280,12 +280,14 @@ class SceneBuilder extends NativeFieldWrapperClass2 { }) { assert(_matrix4IsValid(matrix4)); assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushTransform')); - final TransformEngineLayer layer = TransformEngineLayer._(_pushTransform(matrix4)); + final EngineLayer engineLayer = EngineLayer._(); + _pushTransform(engineLayer, matrix4); + final TransformEngineLayer layer = TransformEngineLayer._(engineLayer); assert(_debugPushLayer(layer)); return layer; } - EngineLayer _pushTransform(Float64List matrix4) native 'SceneBuilder_pushTransform'; + void _pushTransform(EngineLayer layer, Float64List matrix4) native 'SceneBuilder_pushTransform'; /// Pushes an offset operation onto the operation stack. /// @@ -302,12 +304,14 @@ class SceneBuilder extends NativeFieldWrapperClass2 { OffsetEngineLayer oldLayer, }) { assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushOffset')); - final OffsetEngineLayer layer = OffsetEngineLayer._(_pushOffset(dx, dy)); + final EngineLayer engineLayer = EngineLayer._(); + _pushOffset(engineLayer, dx, dy); + final OffsetEngineLayer layer = OffsetEngineLayer._(engineLayer); assert(_debugPushLayer(layer)); return layer; } - EngineLayer _pushOffset(double dx, double dy) native 'SceneBuilder_pushOffset'; + void _pushOffset(EngineLayer layer, double dx, double dy) native 'SceneBuilder_pushOffset'; /// Pushes a rectangular clip operation onto the operation stack. /// @@ -327,13 +331,14 @@ class SceneBuilder extends NativeFieldWrapperClass2 { assert(clipBehavior != null); assert(clipBehavior != Clip.none); assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushClipRect')); - final ClipRectEngineLayer layer = ClipRectEngineLayer._( - _pushClipRect(rect.left, rect.right, rect.top, rect.bottom, clipBehavior.index)); + final EngineLayer engineLayer = EngineLayer._(); + _pushClipRect(engineLayer, rect.left, rect.right, rect.top, rect.bottom, clipBehavior.index); + final ClipRectEngineLayer layer = ClipRectEngineLayer._(engineLayer); assert(_debugPushLayer(layer)); return layer; } - EngineLayer _pushClipRect(double left, double right, double top, double bottom, int clipBehavior) + void _pushClipRect(EngineLayer engineLayer, double left, double right, double top, double bottom, int clipBehavior) native 'SceneBuilder_pushClipRect'; /// Pushes a rounded-rectangular clip operation onto the operation stack. @@ -354,13 +359,14 @@ class SceneBuilder extends NativeFieldWrapperClass2 { assert(clipBehavior != null); assert(clipBehavior != Clip.none); assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushClipRRect')); - final ClipRRectEngineLayer layer = - ClipRRectEngineLayer._(_pushClipRRect(rrect._value32, clipBehavior.index)); + final EngineLayer engineLayer = EngineLayer._(); + _pushClipRRect(engineLayer, rrect._value32, clipBehavior.index); + final ClipRRectEngineLayer layer = ClipRRectEngineLayer._(engineLayer); assert(_debugPushLayer(layer)); return layer; } - EngineLayer _pushClipRRect(Float32List rrect, int clipBehavior) + void _pushClipRRect(EngineLayer layer, Float32List rrect, int clipBehavior) native 'SceneBuilder_pushClipRRect'; /// Pushes a path clip operation onto the operation stack. @@ -381,13 +387,14 @@ class SceneBuilder extends NativeFieldWrapperClass2 { assert(clipBehavior != null); assert(clipBehavior != Clip.none); assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushClipPath')); - final ClipPathEngineLayer layer = - ClipPathEngineLayer._(_pushClipPath(path, clipBehavior.index)); + final EngineLayer engineLayer = EngineLayer._(); + _pushClipPath(engineLayer, path, clipBehavior.index); + final ClipPathEngineLayer layer = ClipPathEngineLayer._(engineLayer); assert(_debugPushLayer(layer)); return layer; } - EngineLayer _pushClipPath(Path path, int clipBehavior) native 'SceneBuilder_pushClipPath'; + void _pushClipPath(EngineLayer layer, Path path, int clipBehavior) native 'SceneBuilder_pushClipPath'; /// Pushes an opacity operation onto the operation stack. /// @@ -407,13 +414,14 @@ class SceneBuilder extends NativeFieldWrapperClass2 { OpacityEngineLayer oldLayer, }) { assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushOpacity')); - final OpacityEngineLayer layer = - OpacityEngineLayer._(_pushOpacity(alpha, offset.dx, offset.dy)); + final EngineLayer engineLayer = EngineLayer._(); + _pushOpacity(engineLayer, alpha, offset.dx, offset.dy); + final OpacityEngineLayer layer = OpacityEngineLayer._(engineLayer); assert(_debugPushLayer(layer)); return layer; } - EngineLayer _pushOpacity(int alpha, double dx, double dy) native 'SceneBuilder_pushOpacity'; + void _pushOpacity(EngineLayer layer, int alpha, double dx, double dy) native 'SceneBuilder_pushOpacity'; /// Pushes a color filter operation onto the operation stack. /// @@ -433,12 +441,14 @@ class SceneBuilder extends NativeFieldWrapperClass2 { assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushColorFilter')); final _ColorFilter nativeFilter = filter._toNativeColorFilter(); assert(nativeFilter != null); - final ColorFilterEngineLayer layer = ColorFilterEngineLayer._(_pushColorFilter(nativeFilter)); + final EngineLayer engineLayer = EngineLayer._(); + _pushColorFilter(engineLayer, nativeFilter); + final ColorFilterEngineLayer layer = ColorFilterEngineLayer._(engineLayer); assert(_debugPushLayer(layer)); return layer; } - EngineLayer _pushColorFilter(_ColorFilter filter) native 'SceneBuilder_pushColorFilter'; + void _pushColorFilter(EngineLayer layer, _ColorFilter filter) native 'SceneBuilder_pushColorFilter'; /// Pushes an image filter operation onto the operation stack. /// @@ -458,12 +468,14 @@ class SceneBuilder extends NativeFieldWrapperClass2 { assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushImageFilter')); final _ImageFilter nativeFilter = filter._toNativeImageFilter(); assert(nativeFilter != null); - final ImageFilterEngineLayer layer = ImageFilterEngineLayer._(_pushImageFilter(nativeFilter)); + final EngineLayer engineLayer = EngineLayer._(); + _pushImageFilter(engineLayer, nativeFilter); + final ImageFilterEngineLayer layer = ImageFilterEngineLayer._(engineLayer); assert(_debugPushLayer(layer)); return layer; } - EngineLayer _pushImageFilter(_ImageFilter filter) native 'SceneBuilder_pushImageFilter'; + void _pushImageFilter(EngineLayer engineLayer, _ImageFilter filter) native 'SceneBuilder_pushImageFilter'; /// Pushes a backdrop filter operation onto the operation stack. /// @@ -480,13 +492,14 @@ class SceneBuilder extends NativeFieldWrapperClass2 { BackdropFilterEngineLayer oldLayer, }) { assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushBackdropFilter')); - final BackdropFilterEngineLayer layer = - BackdropFilterEngineLayer._(_pushBackdropFilter(filter._toNativeImageFilter())); + final EngineLayer engineLayer = EngineLayer._(); + _pushBackdropFilter(engineLayer, filter._toNativeImageFilter()); + final BackdropFilterEngineLayer layer = BackdropFilterEngineLayer._(engineLayer); assert(_debugPushLayer(layer)); return layer; } - EngineLayer _pushBackdropFilter(_ImageFilter filter) native 'SceneBuilder_pushBackdropFilter'; + void _pushBackdropFilter(EngineLayer engineLayer, _ImageFilter filter) native 'SceneBuilder_pushBackdropFilter'; /// Pushes a shader mask operation onto the operation stack. /// @@ -505,13 +518,23 @@ class SceneBuilder extends NativeFieldWrapperClass2 { ShaderMaskEngineLayer oldLayer, }) { assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushShaderMask')); - final ShaderMaskEngineLayer layer = ShaderMaskEngineLayer._(_pushShaderMask( - shader, maskRect.left, maskRect.right, maskRect.top, maskRect.bottom, blendMode.index)); + final EngineLayer engineLayer = EngineLayer._(); + _pushShaderMask( + engineLayer, + shader, + maskRect.left, + maskRect.right, + maskRect.top, + maskRect.bottom, + blendMode.index, + ); + final ShaderMaskEngineLayer layer = ShaderMaskEngineLayer._(engineLayer); assert(_debugPushLayer(layer)); return layer; } EngineLayer _pushShaderMask( + EngineLayer engineLayer, Shader shader, double maskRectLeft, double maskRectRight, @@ -545,13 +568,21 @@ class SceneBuilder extends NativeFieldWrapperClass2 { PhysicalShapeEngineLayer oldLayer, }) { assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushPhysicalShape')); - final PhysicalShapeEngineLayer layer = PhysicalShapeEngineLayer._(_pushPhysicalShape( - path, elevation, color.value, shadowColor?.value ?? 0xFF000000, clipBehavior.index)); + final EngineLayer engineLayer = EngineLayer._(); + _pushPhysicalShape( + engineLayer, + path, + elevation, + color.value, + shadowColor?.value ?? 0xFF000000, + clipBehavior.index, + ); + final PhysicalShapeEngineLayer layer = PhysicalShapeEngineLayer._(engineLayer); assert(_debugPushLayer(layer)); return layer; } - EngineLayer _pushPhysicalShape(Path path, double elevation, int color, int shadowColor, + EngineLayer _pushPhysicalShape(EngineLayer engineLayer, Path path, double elevation, int color, int shadowColor, int clipBehavior) native 'SceneBuilder_pushPhysicalShape'; /// Ends the effect of the most recently pushed operation. @@ -769,7 +800,13 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// /// After calling this function, the scene builder object is invalid and /// cannot be used further. - Scene build() native 'SceneBuilder_build'; + Scene build() { + final Scene scene = Scene._(); + _build(scene); + return scene; + } + + void _build(Scene scene) native 'SceneBuilder_build'; } /// (Fuchsia-only) Hosts content provided by another application. diff --git a/lib/ui/compositing/scene.cc b/lib/ui/compositing/scene.cc index d02a232ac0433..fe2b12d13738a 100644 --- a/lib/ui/compositing/scene.cc +++ b/lib/ui/compositing/scene.cc @@ -26,13 +26,15 @@ IMPLEMENT_WRAPPERTYPEINFO(ui, Scene); DART_BIND_ALL(Scene, FOR_EACH_BINDING) -fml::RefPtr Scene::create(std::shared_ptr rootLayer, - uint32_t rasterizerTracingThreshold, - bool checkerboardRasterCacheImages, - bool checkerboardOffscreenLayers) { - return fml::MakeRefCounted( +void Scene::create(Dart_Handle scene_handle, + std::shared_ptr rootLayer, + uint32_t rasterizerTracingThreshold, + bool checkerboardRasterCacheImages, + bool checkerboardOffscreenLayers) { + auto scene = fml::MakeRefCounted( std::move(rootLayer), rasterizerTracingThreshold, checkerboardRasterCacheImages, checkerboardOffscreenLayers); + scene->ClaimDartHandle(std::move(scene_handle)); } Scene::Scene(std::shared_ptr rootLayer, diff --git a/lib/ui/compositing/scene.h b/lib/ui/compositing/scene.h index 42c972d8d4ee3..d46ca5d3d4281 100644 --- a/lib/ui/compositing/scene.h +++ b/lib/ui/compositing/scene.h @@ -24,10 +24,11 @@ class Scene : public RefCountedDartWrappable { public: ~Scene() override; - static fml::RefPtr create(std::shared_ptr rootLayer, - uint32_t rasterizerTracingThreshold, - bool checkerboardRasterCacheImages, - bool checkerboardOffscreenLayers); + static void create(Dart_Handle scene_handle, + std::shared_ptr rootLayer, + uint32_t rasterizerTracingThreshold, + bool checkerboardRasterCacheImages, + bool checkerboardOffscreenLayers); std::unique_ptr takeLayerTree(); diff --git a/lib/ui/compositing/scene_builder.cc b/lib/ui/compositing/scene_builder.cc index 467f187400255..0594c471e4100 100644 --- a/lib/ui/compositing/scene_builder.cc +++ b/lib/ui/compositing/scene_builder.cc @@ -88,111 +88,118 @@ SceneBuilder::SceneBuilder() { SceneBuilder::~SceneBuilder() = default; -fml::RefPtr SceneBuilder::pushTransform( - tonic::Float64List& matrix4) { +void SceneBuilder::pushTransform(Dart_Handle layer_handle, + tonic::Float64List& matrix4) { SkMatrix sk_matrix = ToSkMatrix(matrix4); auto layer = std::make_shared(sk_matrix); PushLayer(layer); // matrix4 has to be released before we can return another Dart object matrix4.Release(); - return EngineLayer::MakeRetained(layer); + EngineLayer::MakeRetained(std::move(layer_handle), layer); } -fml::RefPtr SceneBuilder::pushOffset(double dx, double dy) { +void SceneBuilder::pushOffset(Dart_Handle layer_handle, double dx, double dy) { SkMatrix sk_matrix = SkMatrix::MakeTrans(dx, dy); auto layer = std::make_shared(sk_matrix); PushLayer(layer); - return EngineLayer::MakeRetained(layer); + EngineLayer::MakeRetained(std::move(layer_handle), layer); } -fml::RefPtr SceneBuilder::pushClipRect(double left, - double right, - double top, - double bottom, - int clipBehavior) { +void SceneBuilder::pushClipRect(Dart_Handle layer_handle, + double left, + double right, + double top, + double bottom, + int clipBehavior) { SkRect clipRect = SkRect::MakeLTRB(left, top, right, bottom); flutter::Clip clip_behavior = static_cast(clipBehavior); auto layer = std::make_shared(clipRect, clip_behavior); PushLayer(layer); - return EngineLayer::MakeRetained(layer); + EngineLayer::MakeRetained(std::move(layer_handle), layer); } -fml::RefPtr SceneBuilder::pushClipRRect(const RRect& rrect, - int clipBehavior) { +void SceneBuilder::pushClipRRect(Dart_Handle layer_handle, + const RRect& rrect, + int clipBehavior) { flutter::Clip clip_behavior = static_cast(clipBehavior); auto layer = std::make_shared(rrect.sk_rrect, clip_behavior); PushLayer(layer); - return EngineLayer::MakeRetained(layer); + EngineLayer::MakeRetained(std::move(layer_handle), layer); } -fml::RefPtr SceneBuilder::pushClipPath(const CanvasPath* path, - int clipBehavior) { +void SceneBuilder::pushClipPath(Dart_Handle layer_handle, + const CanvasPath* path, + int clipBehavior) { flutter::Clip clip_behavior = static_cast(clipBehavior); FML_DCHECK(clip_behavior != flutter::Clip::none); auto layer = std::make_shared(path->path(), clip_behavior); PushLayer(layer); - return EngineLayer::MakeRetained(layer); + EngineLayer::MakeRetained(std::move(layer_handle), layer); } -fml::RefPtr SceneBuilder::pushOpacity(int alpha, - double dx, - double dy) { +void SceneBuilder::pushOpacity(Dart_Handle layer_handle, + int alpha, + double dx, + double dy) { auto layer = std::make_shared(alpha, SkPoint::Make(dx, dy)); PushLayer(layer); - return EngineLayer::MakeRetained(layer); + EngineLayer::MakeRetained(std::move(layer_handle), layer); } -fml::RefPtr SceneBuilder::pushColorFilter( - const ColorFilter* color_filter) { +void SceneBuilder::pushColorFilter(Dart_Handle layer_handle, + const ColorFilter* color_filter) { auto layer = std::make_shared(color_filter->filter()); PushLayer(layer); - return EngineLayer::MakeRetained(layer); + EngineLayer::MakeRetained(std::move(layer_handle), layer); } -fml::RefPtr SceneBuilder::pushImageFilter( - const ImageFilter* image_filter) { +void SceneBuilder::pushImageFilter(Dart_Handle layer_handle, + const ImageFilter* image_filter) { auto layer = std::make_shared(image_filter->filter()); PushLayer(layer); - return EngineLayer::MakeRetained(layer); + EngineLayer::MakeRetained(std::move(layer_handle), layer); } -fml::RefPtr SceneBuilder::pushBackdropFilter(ImageFilter* filter) { +void SceneBuilder::pushBackdropFilter(Dart_Handle layer_handle, + ImageFilter* filter) { auto layer = std::make_shared(filter->filter()); PushLayer(layer); - return EngineLayer::MakeRetained(layer); + EngineLayer::MakeRetained(std::move(layer_handle), layer); } -fml::RefPtr SceneBuilder::pushShaderMask(Shader* shader, - double maskRectLeft, - double maskRectRight, - double maskRectTop, - double maskRectBottom, - int blendMode) { +void SceneBuilder::pushShaderMask(Dart_Handle layer_handle, + Shader* shader, + double maskRectLeft, + double maskRectRight, + double maskRectTop, + double maskRectBottom, + int blendMode) { SkRect rect = SkRect::MakeLTRB(maskRectLeft, maskRectTop, maskRectRight, maskRectBottom); auto layer = std::make_shared( shader->shader(), rect, static_cast(blendMode)); PushLayer(layer); - return EngineLayer::MakeRetained(layer); + EngineLayer::MakeRetained(std::move(layer_handle), layer); } -fml::RefPtr SceneBuilder::pushPhysicalShape(const CanvasPath* path, - double elevation, - int color, - int shadow_color, - int clipBehavior) { +void SceneBuilder::pushPhysicalShape(Dart_Handle layer_handle, + const CanvasPath* path, + double elevation, + int color, + int shadow_color, + int clipBehavior) { auto layer = std::make_shared( static_cast(color), static_cast(shadow_color), static_cast(elevation), path->path(), static_cast(clipBehavior)); PushLayer(layer); - return EngineLayer::MakeRetained(layer); + EngineLayer::MakeRetained(std::move(layer_handle), layer); } void SceneBuilder::addRetained(fml::RefPtr retainedLayer) { @@ -275,14 +282,13 @@ void SceneBuilder::setCheckerboardOffscreenLayers(bool checkerboard) { checkerboard_offscreen_layers_ = checkerboard; } -fml::RefPtr SceneBuilder::build() { +void SceneBuilder::build(Dart_Handle scene_handle) { FML_DCHECK(layer_stack_.size() >= 1); - fml::RefPtr scene = Scene::create( - layer_stack_[0], rasterizer_tracing_threshold_, + Scene::create( + std::move(scene_handle), layer_stack_[0], rasterizer_tracing_threshold_, checkerboard_raster_cache_images_, checkerboard_offscreen_layers_); ClearDartWrapper(); // may delete this object. - return scene; } void SceneBuilder::AddLayer(std::shared_ptr layer) { diff --git a/lib/ui/compositing/scene_builder.h b/lib/ui/compositing/scene_builder.h index 51ae76e651ce0..11c04951226d0 100644 --- a/lib/ui/compositing/scene_builder.h +++ b/lib/ui/compositing/scene_builder.h @@ -38,31 +38,42 @@ class SceneBuilder : public RefCountedDartWrappable { } ~SceneBuilder() override; - fml::RefPtr pushTransform(tonic::Float64List& matrix4); - fml::RefPtr pushOffset(double dx, double dy); - fml::RefPtr pushClipRect(double left, - double right, - double top, - double bottom, - int clipBehavior); - fml::RefPtr pushClipRRect(const RRect& rrect, int clipBehavior); - fml::RefPtr pushClipPath(const CanvasPath* path, - int clipBehavior); - fml::RefPtr pushOpacity(int alpha, double dx = 0, double dy = 0); - fml::RefPtr pushColorFilter(const ColorFilter* color_filter); - fml::RefPtr pushImageFilter(const ImageFilter* image_filter); - fml::RefPtr pushBackdropFilter(ImageFilter* filter); - fml::RefPtr pushShaderMask(Shader* shader, - double maskRectLeft, - double maskRectRight, - double maskRectTop, - double maskRectBottom, - int blendMode); - fml::RefPtr pushPhysicalShape(const CanvasPath* path, - double elevation, - int color, - int shadowColor, - int clipBehavior); + void pushTransform(Dart_Handle layer_handle, tonic::Float64List& matrix4); + void pushOffset(Dart_Handle layer_handle, double dx, double dy); + void pushClipRect(Dart_Handle layer_handle, + double left, + double right, + double top, + double bottom, + int clipBehavior); + void pushClipRRect(Dart_Handle layer_handle, + const RRect& rrect, + int clipBehavior); + void pushClipPath(Dart_Handle layer_handle, + const CanvasPath* path, + int clipBehavior); + void pushOpacity(Dart_Handle layer_handle, + int alpha, + double dx = 0, + double dy = 0); + void pushColorFilter(Dart_Handle layer_handle, + const ColorFilter* color_filter); + void pushImageFilter(Dart_Handle layer_handle, + const ImageFilter* image_filter); + void pushBackdropFilter(Dart_Handle layer_handle, ImageFilter* filter); + void pushShaderMask(Dart_Handle layer_handle, + Shader* shader, + double maskRectLeft, + double maskRectRight, + double maskRectTop, + double maskRectBottom, + int blendMode); + void pushPhysicalShape(Dart_Handle layer_handle, + const CanvasPath* path, + double elevation, + int color, + int shadowColor, + int clipBehavior); void addRetained(fml::RefPtr retainedLayer); @@ -102,7 +113,7 @@ class SceneBuilder : public RefCountedDartWrappable { void setCheckerboardRasterCacheImages(bool checkerboard); void setCheckerboardOffscreenLayers(bool checkerboard); - fml::RefPtr build(); + void build(Dart_Handle scene_handle); static void RegisterNatives(tonic::DartLibraryNatives* natives); diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index e44287a96a573..2669afab64ac3 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -1892,6 +1892,12 @@ class Path extends NativeFieldWrapperClass2 { Path() { _constructor(); } void _constructor() native 'Path_constructor'; + // Workaround for tonic, which expects classes with native fields to have a + // private constructor. + // TODO(dnfield): rework this to use ClaimNativeField - https://github.com/flutter/flutter/issues/50997 + @pragma('vm:entry-point') + Path._() { _constructor(); } + /// Creates a copy of another [Path]. /// /// This copy is fast and does not require additional memory unless either @@ -4158,7 +4164,13 @@ class PictureRecorder extends NativeFieldWrapperClass2 { /// and the canvas objects are invalid and cannot be used further. /// /// Returns null if the PictureRecorder is not associated with a canvas. - Picture endRecording() native 'PictureRecorder_endRecording'; + Picture endRecording() { + final Picture picture = Picture._(); + _endRecording(picture); + return picture; + } + + void _endRecording(Picture picture) native 'PictureRecorder_endRecording'; } /// A single shadow. diff --git a/lib/ui/painting/engine_layer.h b/lib/ui/painting/engine_layer.h index a679ef2fe50f3..09ba7d4b09e6d 100644 --- a/lib/ui/painting/engine_layer.h +++ b/lib/ui/painting/engine_layer.h @@ -30,6 +30,13 @@ class EngineLayer : public RefCountedDartWrappable { return fml::MakeRefCounted(layer); } + static void MakeRetained( + Dart_Handle dart_handle, + std::shared_ptr layer) { + auto engine_layer = fml::MakeRefCounted(layer); + engine_layer->ClaimDartHandle(std::move(dart_handle)); + } + static void RegisterNatives(tonic::DartLibraryNatives* natives); std::shared_ptr Layer() const { return layer_; } diff --git a/lib/ui/painting/picture.cc b/lib/ui/painting/picture.cc index 98c37edc115f7..3027017853644 100644 --- a/lib/ui/painting/picture.cc +++ b/lib/ui/painting/picture.cc @@ -27,8 +27,12 @@ IMPLEMENT_WRAPPERTYPEINFO(ui, Picture); DART_BIND_ALL(Picture, FOR_EACH_BINDING) fml::RefPtr Picture::Create( + Dart_Handle dart_handle, flutter::SkiaGPUObject picture) { - return fml::MakeRefCounted(std::move(picture)); + auto canvas_picture = fml::MakeRefCounted(std::move(picture)); + + canvas_picture->ClaimDartHandle(std::move(dart_handle)); + return canvas_picture; } Picture::Picture(flutter::SkiaGPUObject picture) diff --git a/lib/ui/painting/picture.h b/lib/ui/painting/picture.h index a800f920c48a0..f6dd98887d264 100644 --- a/lib/ui/painting/picture.h +++ b/lib/ui/painting/picture.h @@ -23,7 +23,8 @@ class Picture : public RefCountedDartWrappable { public: ~Picture() override; - static fml::RefPtr Create(flutter::SkiaGPUObject picture); + static fml::RefPtr Create(Dart_Handle dart_handle, + flutter::SkiaGPUObject picture); sk_sp picture() const { return picture_.get(); } diff --git a/lib/ui/painting/picture_recorder.cc b/lib/ui/painting/picture_recorder.cc index 711ea93b5252a..fa44efc77af08 100644 --- a/lib/ui/painting/picture_recorder.cc +++ b/lib/ui/painting/picture_recorder.cc @@ -47,12 +47,14 @@ SkCanvas* PictureRecorder::BeginRecording(SkRect bounds) { return picture_recorder_.beginRecording(bounds, &rtree_factory_); } -fml::RefPtr PictureRecorder::endRecording() { +fml::RefPtr PictureRecorder::endRecording(Dart_Handle dart_picture) { if (!isRecording()) return nullptr; - fml::RefPtr picture = Picture::Create(UIDartState::CreateGPUObject( - picture_recorder_.finishRecordingAsPicture())); + fml::RefPtr picture = + Picture::Create(std::move(dart_picture), + UIDartState::CreateGPUObject( + picture_recorder_.finishRecordingAsPicture())); canvas_->Clear(); canvas_->ClearDartWrapper(); canvas_ = nullptr; diff --git a/lib/ui/painting/picture_recorder.h b/lib/ui/painting/picture_recorder.h index 58835fb034059..7033b54a1d226 100644 --- a/lib/ui/painting/picture_recorder.h +++ b/lib/ui/painting/picture_recorder.h @@ -26,7 +26,7 @@ class PictureRecorder : public RefCountedDartWrappable { ~PictureRecorder() override; SkCanvas* BeginRecording(SkRect bounds); - fml::RefPtr endRecording(); + fml::RefPtr endRecording(Dart_Handle dart_picture); bool isRecording(); void set_canvas(fml::RefPtr canvas) { canvas_ = std::move(canvas); } diff --git a/third_party/tonic/dart_state.cc b/third_party/tonic/dart_state.cc index a3430d87004d2..48c9afa969b21 100644 --- a/third_party/tonic/dart_state.cc +++ b/third_party/tonic/dart_state.cc @@ -22,6 +22,7 @@ DartState::Scope::~Scope() {} DartState::DartState(int dirfd, std::function message_epilogue) : isolate_(nullptr), + private_constructor_name_(), class_library_(new DartClassLibrary), message_handler_(new DartMessageHandler()), file_loader_(new FileLoader(dirfd)), @@ -32,8 +33,15 @@ DartState::~DartState() {} void DartState::SetIsolate(Dart_Isolate isolate) { isolate_ = isolate; + if (!isolate_) return; + + private_constructor_name_.Clear(); + Dart_EnterScope(); + private_constructor_name_.Set(this, Dart_NewPersistentHandle(Dart_NewStringFromCString("_"))); + Dart_ExitScope(); + DidSetIsolate(); } diff --git a/third_party/tonic/dart_state.h b/third_party/tonic/dart_state.h index d2c6e03cb7cc9..d72514cfc38f1 100644 --- a/third_party/tonic/dart_state.h +++ b/third_party/tonic/dart_state.h @@ -49,6 +49,11 @@ class DartState : public std::enable_shared_from_this { Dart_Isolate isolate() { return isolate_; } void SetIsolate(Dart_Isolate isolate); + // TODO(dnfield): delete this https://github.com/flutter/flutter/issues/50997 + Dart_PersistentHandle private_constructor_name() { + return private_constructor_name_.Get(); + } + DartClassLibrary& class_library() { return *class_library_; } DartMessageHandler& message_handler() { return *message_handler_; } FileLoader& file_loader() { return *file_loader_; } @@ -70,6 +75,7 @@ class DartState : public std::enable_shared_from_this { private: Dart_Isolate isolate_; + DartPersistentValue private_constructor_name_; std::unique_ptr class_library_; std::unique_ptr message_handler_; std::unique_ptr file_loader_; diff --git a/third_party/tonic/dart_wrappable.cc b/third_party/tonic/dart_wrappable.cc index 96b5e44ed80fd..60398b9533bdd 100644 --- a/third_party/tonic/dart_wrappable.cc +++ b/third_party/tonic/dart_wrappable.cc @@ -15,6 +15,7 @@ DartWrappable::~DartWrappable() { TONIC_CHECK(!dart_wrapper_); } +// TODO(dnfield): Delete this. https://github.com/flutter/flutter/issues/50997 Dart_Handle DartWrappable::CreateDartWrapper(DartState* dart_state) { TONIC_DCHECK(!dart_wrapper_); const DartWrapperInfo& info = GetDartWrapperInfo(); @@ -22,13 +23,17 @@ Dart_Handle DartWrappable::CreateDartWrapper(DartState* dart_state) { Dart_PersistentHandle type = dart_state->class_library().GetClass(info); TONIC_DCHECK(!LogIfError(type)); - intptr_t native_fields[kNumberOfNativeFields]; - native_fields[kPeerIndex] = reinterpret_cast(this); - native_fields[kWrapperInfoIndex] = reinterpret_cast(&info); - Dart_Handle wrapper = - Dart_AllocateWithNativeFields(type, kNumberOfNativeFields, native_fields); + Dart_Handle wrapper = Dart_New(type, dart_state->private_constructor_name(), 0, nullptr); + TONIC_DCHECK(!LogIfError(wrapper)); + Dart_Handle res = Dart_SetNativeInstanceField( + wrapper, kPeerIndex, reinterpret_cast(this)); + TONIC_DCHECK(!LogIfError(res)); + res = Dart_SetNativeInstanceField(wrapper, kWrapperInfoIndex, + reinterpret_cast(&info)); + TONIC_DCHECK(!LogIfError(res)); + this->RetainDartWrappableReference(); // Balanced in FinalizeDartWrapper. dart_wrapper_ = Dart_NewWeakPersistentHandle( wrapper, this, GetAllocationSize(), &FinalizeDartWrapper); @@ -36,6 +41,22 @@ Dart_Handle DartWrappable::CreateDartWrapper(DartState* dart_state) { return wrapper; } +void DartWrappable::ClaimDartHandle(Dart_Handle wrapper) { + TONIC_DCHECK(!dart_wrapper_); + TONIC_CHECK(!LogIfError(wrapper)); + + const DartWrapperInfo& info = GetDartWrapperInfo(); + + TONIC_CHECK(!LogIfError(Dart_SetNativeInstanceField( + wrapper, kPeerIndex, reinterpret_cast(this)))); + TONIC_CHECK(!LogIfError(Dart_SetNativeInstanceField( + wrapper, kWrapperInfoIndex, reinterpret_cast(&info)))); + + this->RetainDartWrappableReference(); // Balanced in FinalizeDartWrapper. + dart_wrapper_ = Dart_NewWeakPersistentHandle( + wrapper, this, GetAllocationSize(), &FinalizeDartWrapper); +} + void DartWrappable::AssociateWithDartWrapper(Dart_NativeArguments args) { TONIC_DCHECK(!dart_wrapper_); diff --git a/third_party/tonic/dart_wrappable.h b/third_party/tonic/dart_wrappable.h index d6823c1ef92bc..da93476db26a6 100644 --- a/third_party/tonic/dart_wrappable.h +++ b/third_party/tonic/dart_wrappable.h @@ -43,7 +43,11 @@ class DartWrappable { virtual void ReleaseDartWrappableReference() const = 0; + + // Use this method sparingly. It follows a slower path using Dart_New. + // Prefer constructing the object in Dart code and using ClaimDartHandle. Dart_Handle CreateDartWrapper(DartState* dart_state); + void ClaimDartHandle(Dart_Handle wrappable); void AssociateWithDartWrapper(Dart_NativeArguments args); void ClearDartWrapper(); // Warning: Might delete this. Dart_WeakPersistentHandle dart_wrapper() const { return dart_wrapper_; } From 54a30aea1a4f62285aa03fe7111e98b42a553f6c Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 20 Feb 2020 13:34:44 -0800 Subject: [PATCH 2/8] out --- lib/ui/compositing.dart | 10 +++++----- lib/ui/painting.dart | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/ui/compositing.dart b/lib/ui/compositing.dart index bf223d9ef9d51..29058227cd323 100644 --- a/lib/ui/compositing.dart +++ b/lib/ui/compositing.dart @@ -338,7 +338,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { return layer; } - void _pushClipRect(EngineLayer engineLayer, double left, double right, double top, double bottom, int clipBehavior) + void _pushClipRect(EngineLayer outEngineLayer, double left, double right, double top, double bottom, int clipBehavior) native 'SceneBuilder_pushClipRect'; /// Pushes a rounded-rectangular clip operation onto the operation stack. @@ -475,7 +475,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { return layer; } - void _pushImageFilter(EngineLayer engineLayer, _ImageFilter filter) native 'SceneBuilder_pushImageFilter'; + void _pushImageFilter(EngineLayer outEngineLayer, _ImageFilter filter) native 'SceneBuilder_pushImageFilter'; /// Pushes a backdrop filter operation onto the operation stack. /// @@ -499,7 +499,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { return layer; } - void _pushBackdropFilter(EngineLayer engineLayer, _ImageFilter filter) native 'SceneBuilder_pushBackdropFilter'; + void _pushBackdropFilter(EngineLayer outEngineLayer, _ImageFilter filter) native 'SceneBuilder_pushBackdropFilter'; /// Pushes a shader mask operation onto the operation stack. /// @@ -582,7 +582,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { return layer; } - EngineLayer _pushPhysicalShape(EngineLayer engineLayer, Path path, double elevation, int color, int shadowColor, + EngineLayer _pushPhysicalShape(EngineLayer outEngineLayer, Path path, double elevation, int color, int shadowColor, int clipBehavior) native 'SceneBuilder_pushPhysicalShape'; /// Ends the effect of the most recently pushed operation. @@ -806,7 +806,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { return scene; } - void _build(Scene scene) native 'SceneBuilder_build'; + void _build(Scene outScene) native 'SceneBuilder_build'; } /// (Fuchsia-only) Hosts content provided by another application. diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 2669afab64ac3..590e9265ba06c 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -4170,7 +4170,7 @@ class PictureRecorder extends NativeFieldWrapperClass2 { return picture; } - void _endRecording(Picture picture) native 'PictureRecorder_endRecording'; + void _endRecording(Picture outPicture) native 'PictureRecorder_endRecording'; } /// A single shadow. From 58758c724ee2aa7790ffb2ce069d28dc7bc5e4f4 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 20 Feb 2020 14:19:46 -0800 Subject: [PATCH 3/8] unnecessary moves --- lib/ui/painting/engine_layer.h | 2 +- lib/ui/painting/picture.cc | 2 +- lib/ui/painting/picture_recorder.cc | 7 +++---- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/ui/painting/engine_layer.h b/lib/ui/painting/engine_layer.h index 09ba7d4b09e6d..3982a036ab152 100644 --- a/lib/ui/painting/engine_layer.h +++ b/lib/ui/painting/engine_layer.h @@ -34,7 +34,7 @@ class EngineLayer : public RefCountedDartWrappable { Dart_Handle dart_handle, std::shared_ptr layer) { auto engine_layer = fml::MakeRefCounted(layer); - engine_layer->ClaimDartHandle(std::move(dart_handle)); + engine_layer->ClaimDartHandle(dart_handle); } static void RegisterNatives(tonic::DartLibraryNatives* natives); diff --git a/lib/ui/painting/picture.cc b/lib/ui/painting/picture.cc index 3027017853644..b030f634322f6 100644 --- a/lib/ui/painting/picture.cc +++ b/lib/ui/painting/picture.cc @@ -31,7 +31,7 @@ fml::RefPtr Picture::Create( flutter::SkiaGPUObject picture) { auto canvas_picture = fml::MakeRefCounted(std::move(picture)); - canvas_picture->ClaimDartHandle(std::move(dart_handle)); + canvas_picture->ClaimDartHandle(dart_handle); return canvas_picture; } diff --git a/lib/ui/painting/picture_recorder.cc b/lib/ui/painting/picture_recorder.cc index fa44efc77af08..56d3297a1abe9 100644 --- a/lib/ui/painting/picture_recorder.cc +++ b/lib/ui/painting/picture_recorder.cc @@ -51,10 +51,9 @@ fml::RefPtr PictureRecorder::endRecording(Dart_Handle dart_picture) { if (!isRecording()) return nullptr; - fml::RefPtr picture = - Picture::Create(std::move(dart_picture), - UIDartState::CreateGPUObject( - picture_recorder_.finishRecordingAsPicture())); + fml::RefPtr picture = Picture::Create( + dart_picture, UIDartState::CreateGPUObject( + picture_recorder_.finishRecordingAsPicture())); canvas_->Clear(); canvas_->ClearDartWrapper(); canvas_ = nullptr; From eb08fd535fc4f21f5d0e2e83b8bd07eda5e18c6d Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 20 Feb 2020 14:21:36 -0800 Subject: [PATCH 4/8] more --- lib/ui/compositing/scene.cc | 2 +- lib/ui/compositing/scene_builder.cc | 28 ++++++++++++++-------------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/lib/ui/compositing/scene.cc b/lib/ui/compositing/scene.cc index fe2b12d13738a..b5d3e51b2492e 100644 --- a/lib/ui/compositing/scene.cc +++ b/lib/ui/compositing/scene.cc @@ -34,7 +34,7 @@ void Scene::create(Dart_Handle scene_handle, auto scene = fml::MakeRefCounted( std::move(rootLayer), rasterizerTracingThreshold, checkerboardRasterCacheImages, checkerboardOffscreenLayers); - scene->ClaimDartHandle(std::move(scene_handle)); + scene->ClaimDartHandle(scene_handle); } Scene::Scene(std::shared_ptr rootLayer, diff --git a/lib/ui/compositing/scene_builder.cc b/lib/ui/compositing/scene_builder.cc index 0594c471e4100..7e307e3174c5f 100644 --- a/lib/ui/compositing/scene_builder.cc +++ b/lib/ui/compositing/scene_builder.cc @@ -95,14 +95,14 @@ void SceneBuilder::pushTransform(Dart_Handle layer_handle, PushLayer(layer); // matrix4 has to be released before we can return another Dart object matrix4.Release(); - EngineLayer::MakeRetained(std::move(layer_handle), layer); + EngineLayer::MakeRetained(layer_handle, layer); } void SceneBuilder::pushOffset(Dart_Handle layer_handle, double dx, double dy) { SkMatrix sk_matrix = SkMatrix::MakeTrans(dx, dy); auto layer = std::make_shared(sk_matrix); PushLayer(layer); - EngineLayer::MakeRetained(std::move(layer_handle), layer); + EngineLayer::MakeRetained(layer_handle, layer); } void SceneBuilder::pushClipRect(Dart_Handle layer_handle, @@ -116,7 +116,7 @@ void SceneBuilder::pushClipRect(Dart_Handle layer_handle, auto layer = std::make_shared(clipRect, clip_behavior); PushLayer(layer); - EngineLayer::MakeRetained(std::move(layer_handle), layer); + EngineLayer::MakeRetained(layer_handle, layer); } void SceneBuilder::pushClipRRect(Dart_Handle layer_handle, @@ -126,7 +126,7 @@ void SceneBuilder::pushClipRRect(Dart_Handle layer_handle, auto layer = std::make_shared(rrect.sk_rrect, clip_behavior); PushLayer(layer); - EngineLayer::MakeRetained(std::move(layer_handle), layer); + EngineLayer::MakeRetained(layer_handle, layer); } void SceneBuilder::pushClipPath(Dart_Handle layer_handle, @@ -137,7 +137,7 @@ void SceneBuilder::pushClipPath(Dart_Handle layer_handle, auto layer = std::make_shared(path->path(), clip_behavior); PushLayer(layer); - EngineLayer::MakeRetained(std::move(layer_handle), layer); + EngineLayer::MakeRetained(layer_handle, layer); } void SceneBuilder::pushOpacity(Dart_Handle layer_handle, @@ -147,7 +147,7 @@ void SceneBuilder::pushOpacity(Dart_Handle layer_handle, auto layer = std::make_shared(alpha, SkPoint::Make(dx, dy)); PushLayer(layer); - EngineLayer::MakeRetained(std::move(layer_handle), layer); + EngineLayer::MakeRetained(layer_handle, layer); } void SceneBuilder::pushColorFilter(Dart_Handle layer_handle, @@ -155,7 +155,7 @@ void SceneBuilder::pushColorFilter(Dart_Handle layer_handle, auto layer = std::make_shared(color_filter->filter()); PushLayer(layer); - EngineLayer::MakeRetained(std::move(layer_handle), layer); + EngineLayer::MakeRetained(layer_handle, layer); } void SceneBuilder::pushImageFilter(Dart_Handle layer_handle, @@ -163,14 +163,14 @@ void SceneBuilder::pushImageFilter(Dart_Handle layer_handle, auto layer = std::make_shared(image_filter->filter()); PushLayer(layer); - EngineLayer::MakeRetained(std::move(layer_handle), layer); + EngineLayer::MakeRetained(layer_handle, layer); } void SceneBuilder::pushBackdropFilter(Dart_Handle layer_handle, ImageFilter* filter) { auto layer = std::make_shared(filter->filter()); PushLayer(layer); - EngineLayer::MakeRetained(std::move(layer_handle), layer); + EngineLayer::MakeRetained(layer_handle, layer); } void SceneBuilder::pushShaderMask(Dart_Handle layer_handle, @@ -185,7 +185,7 @@ void SceneBuilder::pushShaderMask(Dart_Handle layer_handle, auto layer = std::make_shared( shader->shader(), rect, static_cast(blendMode)); PushLayer(layer); - EngineLayer::MakeRetained(std::move(layer_handle), layer); + EngineLayer::MakeRetained(layer_handle, layer); } void SceneBuilder::pushPhysicalShape(Dart_Handle layer_handle, @@ -199,7 +199,7 @@ void SceneBuilder::pushPhysicalShape(Dart_Handle layer_handle, static_cast(elevation), path->path(), static_cast(clipBehavior)); PushLayer(layer); - EngineLayer::MakeRetained(std::move(layer_handle), layer); + EngineLayer::MakeRetained(layer_handle, layer); } void SceneBuilder::addRetained(fml::RefPtr retainedLayer) { @@ -285,9 +285,9 @@ void SceneBuilder::setCheckerboardOffscreenLayers(bool checkerboard) { void SceneBuilder::build(Dart_Handle scene_handle) { FML_DCHECK(layer_stack_.size() >= 1); - Scene::create( - std::move(scene_handle), layer_stack_[0], rasterizer_tracing_threshold_, - checkerboard_raster_cache_images_, checkerboard_offscreen_layers_); + Scene::create(scene_handle, layer_stack_[0], rasterizer_tracing_threshold_, + checkerboard_raster_cache_images_, + checkerboard_offscreen_layers_); ClearDartWrapper(); // may delete this object. } From 6d19e8cd3c689bdd68053acf41811f3d8205e6e8 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 20 Feb 2020 14:24:30 -0800 Subject: [PATCH 5/8] issue nit --- third_party/tonic/dart_state.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/third_party/tonic/dart_state.h b/third_party/tonic/dart_state.h index d72514cfc38f1..845914937dcd0 100644 --- a/third_party/tonic/dart_state.h +++ b/third_party/tonic/dart_state.h @@ -49,7 +49,8 @@ class DartState : public std::enable_shared_from_this { Dart_Isolate isolate() { return isolate_; } void SetIsolate(Dart_Isolate isolate); - // TODO(dnfield): delete this https://github.com/flutter/flutter/issues/50997 + // TODO(https://github.com/flutter/flutter/issues/50997): Work around until we + // drop the need for Dart_New in tonic. Dart_PersistentHandle private_constructor_name() { return private_constructor_name_.Get(); } From e4051f9e1f27dd9ee5a2eba08dcd0fdc337ba4da Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 20 Feb 2020 14:59:12 -0800 Subject: [PATCH 6/8] format --- lib/ui/painting/engine_layer.h | 5 ++--- third_party/tonic/dart_state.cc | 3 ++- third_party/tonic/dart_wrappable.cc | 3 ++- third_party/tonic/dart_wrappable.h | 1 - 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/ui/painting/engine_layer.h b/lib/ui/painting/engine_layer.h index 3982a036ab152..29dc42844b636 100644 --- a/lib/ui/painting/engine_layer.h +++ b/lib/ui/painting/engine_layer.h @@ -30,9 +30,8 @@ class EngineLayer : public RefCountedDartWrappable { return fml::MakeRefCounted(layer); } - static void MakeRetained( - Dart_Handle dart_handle, - std::shared_ptr layer) { + static void MakeRetained(Dart_Handle dart_handle, + std::shared_ptr layer) { auto engine_layer = fml::MakeRefCounted(layer); engine_layer->ClaimDartHandle(dart_handle); } diff --git a/third_party/tonic/dart_state.cc b/third_party/tonic/dart_state.cc index 48c9afa969b21..b711a22977788 100644 --- a/third_party/tonic/dart_state.cc +++ b/third_party/tonic/dart_state.cc @@ -39,7 +39,8 @@ void DartState::SetIsolate(Dart_Isolate isolate) { private_constructor_name_.Clear(); Dart_EnterScope(); - private_constructor_name_.Set(this, Dart_NewPersistentHandle(Dart_NewStringFromCString("_"))); + private_constructor_name_.Set( + this, Dart_NewPersistentHandle(Dart_NewStringFromCString("_"))); Dart_ExitScope(); DidSetIsolate(); diff --git a/third_party/tonic/dart_wrappable.cc b/third_party/tonic/dart_wrappable.cc index 60398b9533bdd..72d264a62fb54 100644 --- a/third_party/tonic/dart_wrappable.cc +++ b/third_party/tonic/dart_wrappable.cc @@ -23,7 +23,8 @@ Dart_Handle DartWrappable::CreateDartWrapper(DartState* dart_state) { Dart_PersistentHandle type = dart_state->class_library().GetClass(info); TONIC_DCHECK(!LogIfError(type)); - Dart_Handle wrapper = Dart_New(type, dart_state->private_constructor_name(), 0, nullptr); + Dart_Handle wrapper = + Dart_New(type, dart_state->private_constructor_name(), 0, nullptr); TONIC_DCHECK(!LogIfError(wrapper)); diff --git a/third_party/tonic/dart_wrappable.h b/third_party/tonic/dart_wrappable.h index da93476db26a6..8d9fb569916b7 100644 --- a/third_party/tonic/dart_wrappable.h +++ b/third_party/tonic/dart_wrappable.h @@ -43,7 +43,6 @@ class DartWrappable { virtual void ReleaseDartWrappableReference() const = 0; - // Use this method sparingly. It follows a slower path using Dart_New. // Prefer constructing the object in Dart code and using ClaimDartHandle. Dart_Handle CreateDartWrapper(DartState* dart_state); From 2a83d5fae6e36d63a5fedff35b89ac9271ec98ce Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 20 Feb 2020 16:12:11 -0800 Subject: [PATCH 7/8] refactor --- lib/ui/compositing/scene.cc | 2 +- lib/ui/painting/engine_layer.h | 2 +- lib/ui/painting/picture.cc | 2 +- third_party/tonic/dart_args.h | 13 ++++++++++++- third_party/tonic/dart_wrappable.cc | 25 +------------------------ third_party/tonic/dart_wrappable.h | 5 ++--- 6 files changed, 18 insertions(+), 31 deletions(-) diff --git a/lib/ui/compositing/scene.cc b/lib/ui/compositing/scene.cc index b5d3e51b2492e..f5403ecae9a61 100644 --- a/lib/ui/compositing/scene.cc +++ b/lib/ui/compositing/scene.cc @@ -34,7 +34,7 @@ void Scene::create(Dart_Handle scene_handle, auto scene = fml::MakeRefCounted( std::move(rootLayer), rasterizerTracingThreshold, checkerboardRasterCacheImages, checkerboardOffscreenLayers); - scene->ClaimDartHandle(scene_handle); + scene->AssociateWithDartWrapper(scene_handle); } Scene::Scene(std::shared_ptr rootLayer, diff --git a/lib/ui/painting/engine_layer.h b/lib/ui/painting/engine_layer.h index 29dc42844b636..8e402ba9150f5 100644 --- a/lib/ui/painting/engine_layer.h +++ b/lib/ui/painting/engine_layer.h @@ -33,7 +33,7 @@ class EngineLayer : public RefCountedDartWrappable { static void MakeRetained(Dart_Handle dart_handle, std::shared_ptr layer) { auto engine_layer = fml::MakeRefCounted(layer); - engine_layer->ClaimDartHandle(dart_handle); + engine_layer->AssociateWithDartWrapper(dart_handle); } static void RegisterNatives(tonic::DartLibraryNatives* natives); diff --git a/lib/ui/painting/picture.cc b/lib/ui/painting/picture.cc index b030f634322f6..8527b83ec139b 100644 --- a/lib/ui/painting/picture.cc +++ b/lib/ui/painting/picture.cc @@ -31,7 +31,7 @@ fml::RefPtr Picture::Create( flutter::SkiaGPUObject picture) { auto canvas_picture = fml::MakeRefCounted(std::move(picture)); - canvas_picture->ClaimDartHandle(dart_handle); + canvas_picture->AssociateWithDartWrapper(dart_handle); return canvas_picture; } diff --git a/third_party/tonic/dart_args.h b/third_party/tonic/dart_args.h index 3d6f9ddd5908a..b8dc5a6858d24 100644 --- a/third_party/tonic/dart_args.h +++ b/third_party/tonic/dart_args.h @@ -226,7 +226,18 @@ void DartCallConstructor(Sig func, Dart_NativeArguments args) { return; wrappable = decoder.DispatchCtor(func); } - wrappable->AssociateWithDartWrapper(args); + + + Dart_Handle wrapper = Dart_GetNativeArgument(args, 0); + TONIC_CHECK(!LogIfError(wrapper)); + + intptr_t native_fields[DartWrappable::kNumberOfNativeFields]; + TONIC_CHECK(!LogIfError(Dart_GetNativeFieldsOfArgument( + args, 0, DartWrappable::kNumberOfNativeFields, native_fields))); + TONIC_CHECK(!native_fields[DartWrappable::kPeerIndex]); + TONIC_CHECK(!native_fields[DartWrappable::kWrapperInfoIndex]); + + wrappable->AssociateWithDartWrapper(wrapper); } } // namespace tonic diff --git a/third_party/tonic/dart_wrappable.cc b/third_party/tonic/dart_wrappable.cc index 72d264a62fb54..4ebfda7e1a362 100644 --- a/third_party/tonic/dart_wrappable.cc +++ b/third_party/tonic/dart_wrappable.cc @@ -42,7 +42,7 @@ Dart_Handle DartWrappable::CreateDartWrapper(DartState* dart_state) { return wrapper; } -void DartWrappable::ClaimDartHandle(Dart_Handle wrapper) { +void DartWrappable::AssociateWithDartWrapper(Dart_Handle wrapper) { TONIC_DCHECK(!dart_wrapper_); TONIC_CHECK(!LogIfError(wrapper)); @@ -58,29 +58,6 @@ void DartWrappable::ClaimDartHandle(Dart_Handle wrapper) { wrapper, this, GetAllocationSize(), &FinalizeDartWrapper); } -void DartWrappable::AssociateWithDartWrapper(Dart_NativeArguments args) { - TONIC_DCHECK(!dart_wrapper_); - - Dart_Handle wrapper = Dart_GetNativeArgument(args, 0); - TONIC_CHECK(!LogIfError(wrapper)); - - intptr_t native_fields[kNumberOfNativeFields]; - TONIC_CHECK(!LogIfError(Dart_GetNativeFieldsOfArgument( - args, 0, kNumberOfNativeFields, native_fields))); - TONIC_CHECK(!native_fields[kPeerIndex]); - TONIC_CHECK(!native_fields[kWrapperInfoIndex]); - - const DartWrapperInfo& info = GetDartWrapperInfo(); - TONIC_CHECK(!LogIfError(Dart_SetNativeInstanceField( - wrapper, kPeerIndex, reinterpret_cast(this)))); - TONIC_CHECK(!LogIfError(Dart_SetNativeInstanceField( - wrapper, kWrapperInfoIndex, reinterpret_cast(&info)))); - - this->RetainDartWrappableReference(); // Balanced in FinalizeDartWrapper. - dart_wrapper_ = Dart_NewWeakPersistentHandle( - wrapper, this, GetAllocationSize(), &FinalizeDartWrapper); -} - void DartWrappable::ClearDartWrapper() { TONIC_DCHECK(dart_wrapper_); Dart_Handle wrapper = Dart_HandleFromWeakPersistent(dart_wrapper_); diff --git a/third_party/tonic/dart_wrappable.h b/third_party/tonic/dart_wrappable.h index 8d9fb569916b7..59cd15f100b5c 100644 --- a/third_party/tonic/dart_wrappable.h +++ b/third_party/tonic/dart_wrappable.h @@ -44,10 +44,9 @@ class DartWrappable { virtual void ReleaseDartWrappableReference() const = 0; // Use this method sparingly. It follows a slower path using Dart_New. - // Prefer constructing the object in Dart code and using ClaimDartHandle. + // Prefer constructing the object in Dart code and using AssociateWithDartWrapper. Dart_Handle CreateDartWrapper(DartState* dart_state); - void ClaimDartHandle(Dart_Handle wrappable); - void AssociateWithDartWrapper(Dart_NativeArguments args); + void AssociateWithDartWrapper(Dart_Handle wrappable); void ClearDartWrapper(); // Warning: Might delete this. Dart_WeakPersistentHandle dart_wrapper() const { return dart_wrapper_; } From 983f8b07dadf3e2625f9a0644bb37e16062e589a Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 20 Feb 2020 16:22:03 -0800 Subject: [PATCH 8/8] format --- third_party/tonic/dart_args.h | 1 - third_party/tonic/dart_wrappable.h | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/third_party/tonic/dart_args.h b/third_party/tonic/dart_args.h index b8dc5a6858d24..96a5b2c5b0f17 100644 --- a/third_party/tonic/dart_args.h +++ b/third_party/tonic/dart_args.h @@ -227,7 +227,6 @@ void DartCallConstructor(Sig func, Dart_NativeArguments args) { wrappable = decoder.DispatchCtor(func); } - Dart_Handle wrapper = Dart_GetNativeArgument(args, 0); TONIC_CHECK(!LogIfError(wrapper)); diff --git a/third_party/tonic/dart_wrappable.h b/third_party/tonic/dart_wrappable.h index 59cd15f100b5c..1d2e5e75bacb2 100644 --- a/third_party/tonic/dart_wrappable.h +++ b/third_party/tonic/dart_wrappable.h @@ -44,7 +44,8 @@ class DartWrappable { virtual void ReleaseDartWrappableReference() const = 0; // Use this method sparingly. It follows a slower path using Dart_New. - // Prefer constructing the object in Dart code and using AssociateWithDartWrapper. + // Prefer constructing the object in Dart code and using + // AssociateWithDartWrapper. Dart_Handle CreateDartWrapper(DartState* dart_state); void AssociateWithDartWrapper(Dart_Handle wrappable); void ClearDartWrapper(); // Warning: Might delete this.