From cca233e03b6891a8443d783e4f56a28a1f9cd2eb Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 19 Feb 2020 21:41:38 -0800 Subject: [PATCH 01/22] experiment --- lib/ui/painting.dart | 8 +++++++- 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_wrappable.cc | 20 ++++++++++++++++++++ third_party/tonic/dart_wrappable.h | 1 + 7 files changed, 41 insertions(+), 7 deletions(-) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index e44287a96a573..7d2e4ef5300a2 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -4158,7 +4158,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/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_wrappable.cc b/third_party/tonic/dart_wrappable.cc index 96b5e44ed80fd..f41f35ba74816 100644 --- a/third_party/tonic/dart_wrappable.cc +++ b/third_party/tonic/dart_wrappable.cc @@ -36,6 +36,26 @@ 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(); + + intptr_t native_fields[kNumberOfNativeFields]; + native_fields[kPeerIndex] = reinterpret_cast(this); + native_fields[kWrapperInfoIndex] = reinterpret_cast(&info); + + 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..c254be5e7b7fa 100644 --- a/third_party/tonic/dart_wrappable.h +++ b/third_party/tonic/dart_wrappable.h @@ -44,6 +44,7 @@ class DartWrappable { virtual void ReleaseDartWrappableReference() const = 0; 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 3296a12479e532dfb725fa4d9f60e0705f657d25 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 19 Feb 2020 22:28:48 -0800 Subject: [PATCH 02/22] some layers and scene --- lib/ui/compositing.dart | 47 +++++++++++++++--------- lib/ui/compositing/scene.cc | 11 ++++++ lib/ui/compositing/scene.h | 6 ++++ lib/ui/compositing/scene_builder.cc | 55 +++++++++++++++-------------- lib/ui/compositing/scene_builder.h | 31 +++++++++------- lib/ui/painting/engine_layer.h | 7 ++++ 6 files changed, 103 insertions(+), 54 deletions(-) diff --git a/lib/ui/compositing.dart b/lib/ui/compositing.dart index 01a3550ae9dce..5a0a6bfdd0812 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,15 @@ class SceneBuilder extends NativeFieldWrapperClass2 { OpacityEngineLayer oldLayer, }) { assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushOpacity')); + final EngineLayer engineLayer = EngineLayer._(); + _pushOpacity(engineLayer, alpha, offset.dx, offset.dy); final OpacityEngineLayer layer = - OpacityEngineLayer._(_pushOpacity(alpha, offset.dx, offset.dy)); + 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. /// @@ -769,7 +778,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..64330c2419d3b 100644 --- a/lib/ui/compositing/scene.cc +++ b/lib/ui/compositing/scene.cc @@ -35,6 +35,17 @@ fml::RefPtr Scene::create(std::shared_ptr rootLayer, checkerboardRasterCacheImages, checkerboardOffscreenLayers); } +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, uint32_t rasterizerTracingThreshold, bool checkerboardRasterCacheImages, diff --git a/lib/ui/compositing/scene.h b/lib/ui/compositing/scene.h index 42c972d8d4ee3..6cbefcd2fadaa 100644 --- a/lib/ui/compositing/scene.h +++ b/lib/ui/compositing/scene.h @@ -29,6 +29,12 @@ class Scene : public RefCountedDartWrappable { 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(); Dart_Handle toImage(uint32_t width, diff --git a/lib/ui/compositing/scene_builder.cc b/lib/ui/compositing/scene_builder.cc index 467f187400255..1c855d5f09a93 100644 --- a/lib/ui/compositing/scene_builder.cc +++ b/lib/ui/compositing/scene_builder.cc @@ -88,62 +88,66 @@ 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(layer_handle, layer); } fml::RefPtr SceneBuilder::pushColorFilter( @@ -275,14 +279,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_, - checkerboard_raster_cache_images_, checkerboard_offscreen_layers_); + 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..2cd16352f9382 100644 --- a/lib/ui/compositing/scene_builder.h +++ b/lib/ui/compositing/scene_builder.h @@ -38,17 +38,24 @@ 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); + 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); fml::RefPtr pushColorFilter(const ColorFilter* color_filter); fml::RefPtr pushImageFilter(const ImageFilter* image_filter); fml::RefPtr pushBackdropFilter(ImageFilter* filter); @@ -102,7 +109,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/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_; } From d444f64db071892561c8ca94f2c7e8bd454d9b40 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 19 Feb 2020 23:33:21 -0800 Subject: [PATCH 03/22] more layers, path --- lib/ui/compositing.dart | 50 +++++++++++++++++++-------- lib/ui/compositing/scene_builder.cc | 53 +++++++++++++++-------------- lib/ui/compositing/scene_builder.h | 32 +++++++++-------- lib/ui/painting.dart | 28 ++++++++++----- lib/ui/painting/path.cc | 15 ++++---- lib/ui/painting/path.h | 17 ++++++--- lib/ui/painting/path_measure.cc | 37 ++++++++++---------- lib/ui/painting/path_measure.h | 15 ++++---- third_party/tonic/dart_wrappable.cc | 2 ++ 9 files changed, 149 insertions(+), 100 deletions(-) diff --git a/lib/ui/compositing.dart b/lib/ui/compositing.dart index 5a0a6bfdd0812..bf223d9ef9d51 100644 --- a/lib/ui/compositing.dart +++ b/lib/ui/compositing.dart @@ -416,8 +416,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushOpacity')); final EngineLayer engineLayer = EngineLayer._(); _pushOpacity(engineLayer, alpha, offset.dx, offset.dy); - final OpacityEngineLayer layer = - OpacityEngineLayer._(engineLayer); + final OpacityEngineLayer layer = OpacityEngineLayer._(engineLayer); assert(_debugPushLayer(layer)); return layer; } @@ -442,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. /// @@ -467,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. /// @@ -489,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. /// @@ -514,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, @@ -554,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. diff --git a/lib/ui/compositing/scene_builder.cc b/lib/ui/compositing/scene_builder.cc index 1c855d5f09a93..0594c471e4100 100644 --- a/lib/ui/compositing/scene_builder.cc +++ b/lib/ui/compositing/scene_builder.cc @@ -147,56 +147,59 @@ void SceneBuilder::pushOpacity(Dart_Handle layer_handle, auto layer = std::make_shared(alpha, SkPoint::Make(dx, dy)); PushLayer(layer); - EngineLayer::MakeRetained(layer_handle, 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) { @@ -282,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( + std::move(scene_handle), layer_stack_[0], rasterizer_tracing_threshold_, + checkerboard_raster_cache_images_, checkerboard_offscreen_layers_); ClearDartWrapper(); // may delete this object. } diff --git a/lib/ui/compositing/scene_builder.h b/lib/ui/compositing/scene_builder.h index 2cd16352f9382..11c04951226d0 100644 --- a/lib/ui/compositing/scene_builder.h +++ b/lib/ui/compositing/scene_builder.h @@ -56,20 +56,24 @@ class SceneBuilder : public RefCountedDartWrappable { 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 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); diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 7d2e4ef5300a2..d7314a70f7268 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -1892,14 +1892,20 @@ class Path extends NativeFieldWrapperClass2 { Path() { _constructor(); } void _constructor() native 'Path_constructor'; + /// Avoids creating a new native backing for the path for methods that will + /// create it later, such as [Path.from], [shift] and [transform]. + Path._(); + /// Creates a copy of another [Path]. /// /// This copy is fast and does not require additional memory unless either /// the `source` path or the path returned by this constructor are modified. factory Path.from(Path source) { - return source._clone(); + final Path clonedPath = Path._(); + source._clone(clonedPath); + return clonedPath; } - Path _clone() native 'Path_clone'; + void _clone(Path path) native 'Path_clone'; /// Determines how the interior of this path is calculated. /// @@ -2162,17 +2168,21 @@ class Path extends NativeFieldWrapperClass2 { /// sub-path translated by the given offset. Path shift(Offset offset) { assert(_offsetIsValid(offset)); - return _shift(offset.dx, offset.dy); + final Path path = Path._(); + _shift(path, offset.dx, offset.dy); + return path; } - Path _shift(double dx, double dy) native 'Path_shift'; + void _shift(Path path, double dx, double dy) native 'Path_shift'; /// Returns a copy of the path with all the segments of every /// sub-path transformed by the given matrix. Path transform(Float64List matrix4) { assert(_matrix4IsValid(matrix4)); - return _transform(matrix4); + final Path path = Path._(); + _transform(path, matrix4); + return path; } - Path _transform(Float64List matrix4) native 'Path_transform'; + void _transform(Path path, Float64List matrix4) native 'Path_transform'; /// Computes the bounding rectangle for this path. /// @@ -2448,9 +2458,11 @@ class _PathMeasure extends NativeFieldWrapperClass2 { Path extractPath(int contourIndex, double start, double end, {bool startWithMoveTo = true}) { assert(contourIndex <= currentContourIndex, 'Iterator must be advanced before index $contourIndex can be used.'); - return _extractPath(contourIndex, start, end, startWithMoveTo: startWithMoveTo); + final Path path = Path._(); + _extractPath(path, contourIndex, start, end, startWithMoveTo: startWithMoveTo); + return path; } - Path _extractPath(int contourIndex, double start, double end, {bool startWithMoveTo = true}) native 'PathMeasure_getSegment'; + void _extractPath(Path path, int contourIndex, double start, double end, {bool startWithMoveTo = true}) native 'PathMeasure_getSegment'; bool isClosed(int contourIndex) { assert(contourIndex <= currentContourIndex, 'Iterator must be advanced before index $contourIndex can be used.'); diff --git a/lib/ui/painting/path.cc b/lib/ui/painting/path.cc index e9f74255b5b9a..d54da613477a5 100644 --- a/lib/ui/painting/path.cc +++ b/lib/ui/painting/path.cc @@ -262,17 +262,15 @@ bool CanvasPath::contains(double x, double y) { return path_.contains(x, y); } -fml::RefPtr CanvasPath::shift(double dx, double dy) { - fml::RefPtr path = CanvasPath::Create(); +void CanvasPath::shift(Dart_Handle path_handle, double dx, double dy) { + fml::RefPtr path = CanvasPath::Claim(std::move(path_handle)); path_.offset(dx, dy, &path->path_); - return path; } -fml::RefPtr CanvasPath::transform(tonic::Float64List& matrix4) { - fml::RefPtr path = CanvasPath::Create(); +void CanvasPath::transform(Dart_Handle path_handle, tonic::Float64List& matrix4) { + fml::RefPtr path = CanvasPath::Claim(std::move(path_handle)); path_.transform(ToSkMatrix(matrix4), &path->path_); matrix4.Release(); - return path; } tonic::Float32List CanvasPath::getBounds() { @@ -289,12 +287,11 @@ bool CanvasPath::op(CanvasPath* path1, CanvasPath* path2, int operation) { return Op(path1->path(), path2->path(), (SkPathOp)operation, &path_); } -fml::RefPtr CanvasPath::clone() { - fml::RefPtr path = CanvasPath::Create(); +void CanvasPath::clone(Dart_Handle path_handle) { + fml::RefPtr path = CanvasPath::Claim(std::move(path_handle)); // per Skia docs, this will create a fast copy // data is shared until the source path or dest path are mutated path->path_ = path_; - return path; } } // namespace flutter diff --git a/lib/ui/painting/path.h b/lib/ui/painting/path.h index c7be3c83b9f87..bd21dccf38500 100644 --- a/lib/ui/painting/path.h +++ b/lib/ui/painting/path.h @@ -27,12 +27,19 @@ class CanvasPath : public RefCountedDartWrappable { return fml::MakeRefCounted(); } - static fml::RefPtr CreateFrom(const SkPath& src) { - fml::RefPtr path = CanvasPath::Create(); + static fml::RefPtr ClaimFrom(Dart_Handle path_handle, + const SkPath& src) { + fml::RefPtr path = CanvasPath::Claim(std::move(path_handle)); path->path_ = src; return path; } + static fml::RefPtr Claim(Dart_Handle path_handle) { + auto path = fml::MakeRefCounted(); + path->ClaimDartHandle(std::move(path_handle)); + return path; + } + int getFillType(); void setFillType(int fill_type); @@ -95,11 +102,11 @@ class CanvasPath : public RefCountedDartWrappable { void close(); void reset(); bool contains(double x, double y); - fml::RefPtr shift(double dx, double dy); - fml::RefPtr transform(tonic::Float64List& matrix4); + void shift(Dart_Handle path_handle, double dx, double dy); + void transform(Dart_Handle path_handle, tonic::Float64List& matrix4); tonic::Float32List getBounds(); bool op(CanvasPath* path1, CanvasPath* path2, int operation); - fml::RefPtr clone(); + void clone(Dart_Handle path_handle); const SkPath& path() const { return path_; } diff --git a/lib/ui/painting/path_measure.cc b/lib/ui/painting/path_measure.cc index d195485d83652..37d8fa49d1d87 100644 --- a/lib/ui/painting/path_measure.cc +++ b/lib/ui/painting/path_measure.cc @@ -65,26 +65,26 @@ void CanvasPathMeasure::setPath(const CanvasPath* path, bool isClosed) { path_measure_->reset(skPath, isClosed); } -float CanvasPathMeasure::getLength(int contourIndex) { +float CanvasPathMeasure::getLength(int contour_index) { if (static_cast>::size_type>( - contourIndex) < measures_.size()) { - return measures_[contourIndex]->length(); + contour_index) < measures_.size()) { + return measures_[contour_index]->length(); } return -1; } -tonic::Float32List CanvasPathMeasure::getPosTan(int contourIndex, +tonic::Float32List CanvasPathMeasure::getPosTan(int contour_index, float distance) { tonic::Float32List posTan(Dart_NewTypedData(Dart_TypedData_kFloat32, 5)); posTan[0] = 0; // dart code will check for this for failure if (static_cast>::size_type>( - contourIndex) >= measures_.size()) { + contour_index) >= measures_.size()) { return posTan; } SkPoint pos; SkVector tan; - bool success = measures_[contourIndex]->getPosTan(distance, &pos, &tan); + bool success = measures_[contour_index]->getPosTan(distance, &pos, &tan); if (success) { posTan[0] = 1; // dart code will check for this for success @@ -97,28 +97,29 @@ tonic::Float32List CanvasPathMeasure::getPosTan(int contourIndex, return posTan; } -fml::RefPtr CanvasPathMeasure::getSegment(int contourIndex, - float startD, - float stopD, - bool startWithMoveTo) { +void CanvasPathMeasure::getSegment(Dart_Handle path_handle, + int contour_index, + float start_d, + float stop_d, + bool start_with_move_to) { if (static_cast>::size_type>( - contourIndex) >= measures_.size()) { - return CanvasPath::Create(); + contour_index) >= measures_.size()) { + CanvasPath::Claim(std::move(path_handle)); } SkPath dst; bool success = - measures_[contourIndex]->getSegment(startD, stopD, &dst, startWithMoveTo); + measures_[contour_index]->getSegment(start_d, stop_d, &dst, start_with_move_to); if (!success) { - return CanvasPath::Create(); + CanvasPath::Claim(std::move(path_handle)); } else { - return CanvasPath::CreateFrom(dst); + CanvasPath::ClaimFrom(std::move(path_handle), dst); } } -bool CanvasPathMeasure::isClosed(int contourIndex) { +bool CanvasPathMeasure::isClosed(int contour_index) { if (static_cast>::size_type>( - contourIndex) < measures_.size()) { - return measures_[contourIndex]->isClosed(); + contour_index) < measures_.size()) { + return measures_[contour_index]->isClosed(); } return false; } diff --git a/lib/ui/painting/path_measure.h b/lib/ui/painting/path_measure.h index f73a1b941002d..85a967919ed69 100644 --- a/lib/ui/painting/path_measure.h +++ b/lib/ui/painting/path_measure.h @@ -32,13 +32,14 @@ class CanvasPathMeasure : public RefCountedDartWrappable { bool forceClosed); void setPath(const CanvasPath* path, bool isClosed); - float getLength(int contourIndex); - tonic::Float32List getPosTan(int contourIndex, float distance); - fml::RefPtr getSegment(int contourIndex, - float startD, - float stopD, - bool startWithMoveTo); - bool isClosed(int contourIndex); + float getLength(int contour_index); + tonic::Float32List getPosTan(int contour_index, float distance); + void getSegment(Dart_Handle path_handle, + int contour_index, + float start_d, + float stop_d, + bool start_with_move_to); + bool isClosed(int contour_index); bool nextContour(); static void RegisterNatives(tonic::DartLibraryNatives* natives); diff --git a/third_party/tonic/dart_wrappable.cc b/third_party/tonic/dart_wrappable.cc index f41f35ba74816..a1a7faafe46df 100644 --- a/third_party/tonic/dart_wrappable.cc +++ b/third_party/tonic/dart_wrappable.cc @@ -4,6 +4,7 @@ #include "tonic/dart_wrappable.h" +#include "fml/logging.h" #include "tonic/dart_class_library.h" #include "tonic/dart_state.h" #include "tonic/dart_wrapper_info.h" @@ -16,6 +17,7 @@ DartWrappable::~DartWrappable() { } Dart_Handle DartWrappable::CreateDartWrapper(DartState* dart_state) { + FML_DLOG(ERROR) << "NO TOUCHY"; TONIC_DCHECK(!dart_wrapper_); const DartWrapperInfo& info = GetDartWrapperInfo(); From 09d5a9987da57a6adeb3ffbb7c272fb9f3758981 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 20 Feb 2020 01:49:11 -0800 Subject: [PATCH 04/22] paths, images, more layers --- lib/ui/compositing.dart | 5 ++-- lib/ui/compositing/scene.cc | 15 +++-------- lib/ui/compositing/scene.h | 8 ++---- lib/ui/painting.dart | 37 ++++++++++++++++----------- lib/ui/painting/codec.cc | 22 ++++++++-------- lib/ui/painting/codec.h | 4 ++- lib/ui/painting/frame_info.h | 8 ++++++ lib/ui/painting/image.h | 6 +++-- lib/ui/painting/multi_frame_codec.cc | 20 ++++++++++----- lib/ui/painting/multi_frame_codec.h | 16 +++++++++--- lib/ui/painting/path.cc | 8 +++--- lib/ui/painting/path.h | 16 ++++++------ lib/ui/painting/path_measure.cc | 6 ++--- lib/ui/painting/picture.cc | 14 ++++++---- lib/ui/painting/picture.h | 6 +++-- lib/ui/painting/single_frame_codec.cc | 19 +++++++------- lib/ui/painting/single_frame_codec.h | 14 +++++++--- 17 files changed, 133 insertions(+), 91 deletions(-) diff --git a/lib/ui/compositing.dart b/lib/ui/compositing.dart index bf223d9ef9d51..53413a9730e2c 100644 --- a/lib/ui/compositing.dart +++ b/lib/ui/compositing.dart @@ -25,10 +25,11 @@ class Scene extends NativeFieldWrapperClass2 { if (width <= 0 || height <= 0) { throw Exception('Invalid image dimensions.'); } - return _futurize((_Callback callback) => _toImage(width, height, callback)); + final Image image = Image._(); + return _futurize((_Callback callback) => _toImage(image, width, height, callback)); } - String _toImage(int width, int height, _Callback callback) native 'Scene_toImage'; + String _toImage(Image image, int width, int height, _Callback callback) native 'Scene_toImage'; /// Releases the resources used by this scene. /// diff --git a/lib/ui/compositing/scene.cc b/lib/ui/compositing/scene.cc index 64330c2419d3b..29d7cacc55f7e 100644 --- a/lib/ui/compositing/scene.cc +++ b/lib/ui/compositing/scene.cc @@ -26,15 +26,6 @@ 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( - std::move(rootLayer), rasterizerTracingThreshold, - checkerboardRasterCacheImages, checkerboardOffscreenLayers); -} - void Scene::create(Dart_Handle scene_handle, std::shared_ptr rootLayer, uint32_t rasterizerTracingThreshold, @@ -70,7 +61,8 @@ void Scene::dispose() { ClearDartWrapper(); } -Dart_Handle Scene::toImage(uint32_t width, +Dart_Handle Scene::toImage(Dart_Handle image_handle, + uint32_t width, uint32_t height, Dart_Handle raw_image_callback) { TRACE_EVENT0("flutter", "Scene::toImage"); @@ -84,7 +76,8 @@ Dart_Handle Scene::toImage(uint32_t width, return tonic::ToDart("Could not flatten scene into a layer tree."); } - return Picture::RasterizeToImage(picture, width, height, raw_image_callback); + return Picture::RasterizeToImage(image_handle, picture, width, height, + raw_image_callback); } std::unique_ptr Scene::takeLayerTree() { diff --git a/lib/ui/compositing/scene.h b/lib/ui/compositing/scene.h index 6cbefcd2fadaa..7039dc410874e 100644 --- a/lib/ui/compositing/scene.h +++ b/lib/ui/compositing/scene.h @@ -24,11 +24,6 @@ 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, @@ -37,7 +32,8 @@ class Scene : public RefCountedDartWrappable { std::unique_ptr takeLayerTree(); - Dart_Handle toImage(uint32_t width, + Dart_Handle toImage(Dart_Handle image_handle, + uint32_t width, uint32_t height, Dart_Handle image_callback); diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index d7314a70f7268..ed9d67eae6507 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -1685,12 +1685,15 @@ class Codec extends NativeFieldWrapperClass2 { /// Wraps back to the first frame after returning the last frame. /// /// The returned future can complete with an error if the decoding has failed. - Future getNextFrame() { - return _futurize(_getNextFrame); + Future getNextFrame() async { + final Image image = Image._(); + final FrameInfo frameInfo = FrameInfo._(); + await _futurize((_Callback callback) => _getNextFrame(image, frameInfo, callback)); + return frameInfo; } /// Returns an error message on failure, null on success. - String _getNextFrame(_Callback callback) native 'Codec_getNextFrame'; + String _getNextFrame(Image image, FrameInfo frameInfo, _Callback callback) native 'Codec_getNextFrame'; /// Release the resources used by this object. The object is no longer usable /// after this method is called. @@ -1714,10 +1717,12 @@ class Codec extends NativeFieldWrapperClass2 { Future instantiateImageCodec(Uint8List list, { int targetWidth, int targetHeight, -}) { - return _futurize( - (_Callback callback) => _instantiateImageCodec(list, callback, null, targetWidth ?? _kDoNotResizeDimension, targetHeight ?? _kDoNotResizeDimension) - ); +}) async { + final Codec codec = Codec._(); + await _futurize((_Callback callback) { + return _instantiateImageCodec(codec, list, callback, null, targetWidth ?? _kDoNotResizeDimension, targetHeight ?? _kDoNotResizeDimension); + }); + return codec; } /// Instantiates a [Codec] object for an image binary data. @@ -1731,7 +1736,7 @@ Future instantiateImageCodec(Uint8List list, { /// If both are equal to [_kDoNotResizeDimension], then the image maintains its real size. /// /// Returns an error message if the instantiation has failed, null otherwise. -String _instantiateImageCodec(Uint8List list, _Callback callback, _ImageInfo imageInfo, int targetWidth, int targetHeight) +String _instantiateImageCodec(Codec codec, Uint8List list, _Callback callback, _ImageInfo imageInfo, int targetWidth, int targetHeight) native 'instantiateImageCodec'; /// Loads a single image frame from a byte array into an [Image] object. @@ -1772,11 +1777,12 @@ void decodeImageFromPixels( {int rowBytes, int targetWidth, int targetHeight} ) { final _ImageInfo imageInfo = _ImageInfo(width, height, format.index, rowBytes); - final Future codecFuture = _futurize( - (_Callback callback) => _instantiateImageCodec(pixels, callback, imageInfo, targetWidth ?? _kDoNotResizeDimension, targetHeight ?? _kDoNotResizeDimension) - ); - codecFuture.then((Codec codec) => codec.getNextFrame()) - .then((FrameInfo frameInfo) => callback(frameInfo.image)); + final Codec codec = Codec._(); + _futurize( + (_Callback callback) => _instantiateImageCodec(codec, pixels, callback, imageInfo, targetWidth ?? _kDoNotResizeDimension, targetHeight ?? _kDoNotResizeDimension) + ).then((bool _) { + codec.getNextFrame().then((FrameInfo frameInfo) => callback(frameInfo.image)); + }); } /// Determines the winding rule that decides how the interior of a [Path] is @@ -4124,12 +4130,13 @@ class Picture extends NativeFieldWrapperClass2 { Future toImage(int width, int height) { if (width <= 0 || height <= 0) throw Exception('Invalid image dimensions.'); + final Image image = Image._(); return _futurize( - (_Callback callback) => _toImage(width, height, callback) + (_Callback callback) => _toImage(image, width, height, callback) ); } - String _toImage(int width, int height, _Callback callback) native 'Picture_toImage'; + String _toImage(Image image, int width, int height, _Callback callback) native 'Picture_toImage'; /// Release the resources used by this object. The object is no longer usable /// after this method is called. diff --git a/lib/ui/painting/codec.cc b/lib/ui/painting/codec.cc index 829b9db65ed9d..f6ef8c4110763 100644 --- a/lib/ui/painting/codec.cc +++ b/lib/ui/painting/codec.cc @@ -145,13 +145,14 @@ static std::variant ConvertImageInfo( } static void InstantiateImageCodec(Dart_NativeArguments args) { - Dart_Handle callback_handle = Dart_GetNativeArgument(args, 1); + Dart_Handle codec_handle = Dart_GetNativeArgument(args, 0); + Dart_Handle callback_handle = Dart_GetNativeArgument(args, 2); if (!Dart_IsClosure(callback_handle)) { Dart_SetReturnValue(args, tonic::ToDart("Callback must be a function")); return; } - Dart_Handle image_info_handle = Dart_GetNativeArgument(args, 2); + Dart_Handle image_info_handle = Dart_GetNativeArgument(args, 3); std::optional image_info; @@ -171,7 +172,7 @@ static void InstantiateImageCodec(Dart_NativeArguments args) { { Dart_Handle exception = nullptr; tonic::Uint8List list = - tonic::DartConverter::FromArguments(args, 0, + tonic::DartConverter::FromArguments(args, 1, exception); if (exception) { Dart_SetReturnValue(args, exception); @@ -191,9 +192,9 @@ static void InstantiateImageCodec(Dart_NativeArguments args) { } const int targetWidth = - tonic::DartConverter::FromDart(Dart_GetNativeArgument(args, 3)); - const int targetHeight = tonic::DartConverter::FromDart(Dart_GetNativeArgument(args, 4)); + const int targetHeight = + tonic::DartConverter::FromDart(Dart_GetNativeArgument(args, 5)); std::unique_ptr codec; bool single_frame; @@ -208,8 +209,6 @@ static void InstantiateImageCodec(Dart_NativeArguments args) { single_frame = codec->getFrameCount() == 1; } - fml::RefPtr ui_codec; - if (single_frame) { ImageDecoder::ImageDescriptor descriptor; descriptor.decompressed_image_info = image_info; @@ -222,12 +221,13 @@ static void InstantiateImageCodec(Dart_NativeArguments args) { } descriptor.data = std::move(buffer); - ui_codec = fml::MakeRefCounted(std::move(descriptor)); + SingleFrameCodec::Create(std::move(codec_handle), std::move(descriptor)); } else { - ui_codec = fml::MakeRefCounted(std::move(codec)); + MultiFrameCodec::Create(std::move(codec_handle), std::move(codec)); } - tonic::DartInvoke(callback_handle, {ToDart(ui_codec)}); + tonic::DartInvoke(callback_handle, {Dart_True()}); + Dart_SetReturnValue(args, Dart_Null()); } IMPLEMENT_WRAPPERTYPEINFO(ui, Codec); @@ -246,7 +246,7 @@ void Codec::dispose() { void Codec::RegisterNatives(tonic::DartLibraryNatives* natives) { natives->Register({ - {"instantiateImageCodec", InstantiateImageCodec, 5, true}, + {"instantiateImageCodec", InstantiateImageCodec, 6, true}, }); natives->Register({FOR_EACH_BINDING(DART_REGISTER_NATIVE)}); } diff --git a/lib/ui/painting/codec.h b/lib/ui/painting/codec.h index 2e0c746eac2d6..900e4cc612bae 100644 --- a/lib/ui/painting/codec.h +++ b/lib/ui/painting/codec.h @@ -30,7 +30,9 @@ class Codec : public RefCountedDartWrappable { virtual int repetitionCount() const = 0; - virtual Dart_Handle getNextFrame(Dart_Handle callback_handle) = 0; + virtual Dart_Handle getNextFrame(Dart_Handle image_handle, + Dart_Handle frame_handle, + Dart_Handle callback_handle) = 0; void dispose(); diff --git a/lib/ui/painting/frame_info.h b/lib/ui/painting/frame_info.h index 184b132d17791..515e8e68f605e 100644 --- a/lib/ui/painting/frame_info.h +++ b/lib/ui/painting/frame_info.h @@ -15,6 +15,14 @@ class FrameInfo final : public RefCountedDartWrappable { DEFINE_WRAPPERTYPEINFO(); public: + static fml::RefPtr Create(Dart_Handle frame_handle, + fml::RefPtr image, + int durationMillis) { + auto frame_info = + fml::MakeRefCounted(std::move(image), durationMillis); + frame_info->ClaimDartHandle(std::move(frame_handle)); + return frame_info; + } int durationMillis() { return durationMillis_; } fml::RefPtr image() { return image_; } diff --git a/lib/ui/painting/image.h b/lib/ui/painting/image.h index 516a2e5d59c49..cff2deb5e1f5c 100644 --- a/lib/ui/painting/image.h +++ b/lib/ui/painting/image.h @@ -22,8 +22,10 @@ class CanvasImage final : public RefCountedDartWrappable { public: ~CanvasImage() override; - static fml::RefPtr Create() { - return fml::MakeRefCounted(); + static fml::RefPtr Create(Dart_Handle dart_handle) { + auto image = fml::MakeRefCounted(); + image->ClaimDartHandle(dart_handle); + return image; } int width() { return image_.get()->width(); } diff --git a/lib/ui/painting/multi_frame_codec.cc b/lib/ui/painting/multi_frame_codec.cc index f650e25a97e61..1f38bb403308c 100644 --- a/lib/ui/painting/multi_frame_codec.cc +++ b/lib/ui/painting/multi_frame_codec.cc @@ -33,7 +33,7 @@ static void InvokeNextFrameCallback( if (!frameInfo) { tonic::DartInvoke(callback->value(), {Dart_Null()}); } else { - tonic::DartInvoke(callback->value(), {ToDart(frameInfo)}); + tonic::DartInvoke(callback->value(), {Dart_True()}); } } @@ -129,6 +129,8 @@ sk_sp MultiFrameCodec::GetNextFrameImage( } void MultiFrameCodec::GetNextFrameAndInvokeCallback( + Dart_Handle image_handle, + Dart_Handle frame_handle, std::unique_ptr callback, fml::RefPtr ui_task_runner, fml::WeakPtr resourceContext, @@ -137,12 +139,13 @@ void MultiFrameCodec::GetNextFrameAndInvokeCallback( fml::RefPtr frameInfo = NULL; sk_sp skImage = GetNextFrameImage(resourceContext); if (skImage) { - fml::RefPtr image = CanvasImage::Create(); + fml::RefPtr image = + CanvasImage::Create(std::move(image_handle)); image->set_image({skImage, std::move(unref_queue)}); SkCodec::FrameInfo skFrameInfo; codec_->getFrameInfo(nextFrameIndex_, &skFrameInfo); - frameInfo = - fml::MakeRefCounted(std::move(image), skFrameInfo.fDuration); + frameInfo = FrameInfo::Create(std::move(frame_handle), std::move(image), + skFrameInfo.fDuration); } nextFrameIndex_ = (nextFrameIndex_ + 1) % frameCount_; @@ -152,7 +155,9 @@ void MultiFrameCodec::GetNextFrameAndInvokeCallback( })); } -Dart_Handle MultiFrameCodec::getNextFrame(Dart_Handle callback_handle) { +Dart_Handle MultiFrameCodec::getNextFrame(Dart_Handle image_handle, + Dart_Handle frame_handle, + Dart_Handle callback_handle) { static size_t trace_counter = 1; const size_t trace_id = trace_counter++; @@ -165,11 +170,14 @@ Dart_Handle MultiFrameCodec::getNextFrame(Dart_Handle callback_handle) { const auto& task_runners = dart_state->GetTaskRunners(); task_runners.GetIOTaskRunner()->PostTask(fml::MakeCopyable( - [callback = std::make_unique( + [image_handle = std::move(image_handle), + frame_handle = std::move(frame_handle), + callback = std::make_unique( tonic::DartState::Current(), callback_handle), this, trace_id, ui_task_runner = task_runners.GetUITaskRunner(), io_manager = dart_state->GetIOManager()]() mutable { GetNextFrameAndInvokeCallback( + std::move(image_handle), std::move(frame_handle), std::move(callback), std::move(ui_task_runner), io_manager->GetResourceContext(), io_manager->GetSkiaUnrefQueue(), trace_id); diff --git a/lib/ui/painting/multi_frame_codec.h b/lib/ui/painting/multi_frame_codec.h index 0be63c2a57582..ccd205c0b6800 100644 --- a/lib/ui/painting/multi_frame_codec.h +++ b/lib/ui/painting/multi_frame_codec.h @@ -12,8 +12,13 @@ namespace flutter { class MultiFrameCodec : public Codec { public: - MultiFrameCodec(std::unique_ptr codec); - + static fml::RefPtr Create(Dart_Handle codec_handle, + std::unique_ptr codec) { + auto multi_frame_codec = + fml::MakeRefCounted(std::move(codec)); + multi_frame_codec->ClaimDartHandle(std::move(codec_handle)); + return multi_frame_codec; + } ~MultiFrameCodec() override; // |Codec| @@ -23,9 +28,12 @@ class MultiFrameCodec : public Codec { int repetitionCount() const override; // |Codec| - Dart_Handle getNextFrame(Dart_Handle args) override; + Dart_Handle getNextFrame(Dart_Handle image_handle, + Dart_Handle frame_handle, + Dart_Handle args) override; private: + MultiFrameCodec(std::unique_ptr codec); const std::unique_ptr codec_; const int frameCount_; const int repetitionCount_; @@ -39,6 +47,8 @@ class MultiFrameCodec : public Codec { sk_sp GetNextFrameImage(fml::WeakPtr resourceContext); void GetNextFrameAndInvokeCallback( + Dart_Handle image_handle, + Dart_Handle frame_handle, std::unique_ptr callback, fml::RefPtr ui_task_runner, fml::WeakPtr resourceContext, diff --git a/lib/ui/painting/path.cc b/lib/ui/painting/path.cc index d54da613477a5..0227c07a8f105 100644 --- a/lib/ui/painting/path.cc +++ b/lib/ui/painting/path.cc @@ -20,7 +20,7 @@ namespace flutter { typedef CanvasPath Path; static void Path_constructor(Dart_NativeArguments args) { - DartCallConstructor(&CanvasPath::Create, args); + DartCallConstructor(&CanvasPath::CreateNew, args); } IMPLEMENT_WRAPPERTYPEINFO(ui, Path); @@ -263,12 +263,12 @@ bool CanvasPath::contains(double x, double y) { } void CanvasPath::shift(Dart_Handle path_handle, double dx, double dy) { - fml::RefPtr path = CanvasPath::Claim(std::move(path_handle)); + fml::RefPtr path = CanvasPath::Create(std::move(path_handle)); path_.offset(dx, dy, &path->path_); } void CanvasPath::transform(Dart_Handle path_handle, tonic::Float64List& matrix4) { - fml::RefPtr path = CanvasPath::Claim(std::move(path_handle)); + fml::RefPtr path = CanvasPath::Create(std::move(path_handle)); path_.transform(ToSkMatrix(matrix4), &path->path_); matrix4.Release(); } @@ -288,7 +288,7 @@ bool CanvasPath::op(CanvasPath* path1, CanvasPath* path2, int operation) { } void CanvasPath::clone(Dart_Handle path_handle) { - fml::RefPtr path = CanvasPath::Claim(std::move(path_handle)); + fml::RefPtr path = CanvasPath::Create(std::move(path_handle)); // per Skia docs, this will create a fast copy // data is shared until the source path or dest path are mutated path->path_ = path_; diff --git a/lib/ui/painting/path.h b/lib/ui/painting/path.h index bd21dccf38500..73ed8304ae722 100644 --- a/lib/ui/painting/path.h +++ b/lib/ui/painting/path.h @@ -23,20 +23,20 @@ class CanvasPath : public RefCountedDartWrappable { public: ~CanvasPath() override; - static fml::RefPtr Create() { + static fml::RefPtr CreateNew(Dart_Handle path_handle) { return fml::MakeRefCounted(); } - static fml::RefPtr ClaimFrom(Dart_Handle path_handle, - const SkPath& src) { - fml::RefPtr path = CanvasPath::Claim(std::move(path_handle)); - path->path_ = src; + static fml::RefPtr Create(Dart_Handle path_handle) { + auto path = fml::MakeRefCounted(); + path->ClaimDartHandle(std::move(path_handle)); return path; } - static fml::RefPtr Claim(Dart_Handle path_handle) { - auto path = fml::MakeRefCounted(); - path->ClaimDartHandle(std::move(path_handle)); + static fml::RefPtr CreateFrom(Dart_Handle path_handle, + const SkPath& src) { + fml::RefPtr path = CanvasPath::Create(std::move(path_handle)); + path->path_ = src; return path; } diff --git a/lib/ui/painting/path_measure.cc b/lib/ui/painting/path_measure.cc index 37d8fa49d1d87..8ef30ddbe1c3a 100644 --- a/lib/ui/painting/path_measure.cc +++ b/lib/ui/painting/path_measure.cc @@ -104,15 +104,15 @@ void CanvasPathMeasure::getSegment(Dart_Handle path_handle, bool start_with_move_to) { if (static_cast>::size_type>( contour_index) >= measures_.size()) { - CanvasPath::Claim(std::move(path_handle)); + CanvasPath::Create(std::move(path_handle)); } SkPath dst; bool success = measures_[contour_index]->getSegment(start_d, stop_d, &dst, start_with_move_to); if (!success) { - CanvasPath::Claim(std::move(path_handle)); + CanvasPath::Create(std::move(path_handle)); } else { - CanvasPath::ClaimFrom(std::move(path_handle), dst); + CanvasPath::CreateFrom(std::move(path_handle), dst); } } diff --git a/lib/ui/painting/picture.cc b/lib/ui/painting/picture.cc index 3027017853644..69806436b9a61 100644 --- a/lib/ui/painting/picture.cc +++ b/lib/ui/painting/picture.cc @@ -40,14 +40,16 @@ Picture::Picture(flutter::SkiaGPUObject picture) Picture::~Picture() = default; -Dart_Handle Picture::toImage(uint32_t width, +Dart_Handle Picture::toImage(Dart_Handle image_handle, + uint32_t width, uint32_t height, Dart_Handle raw_image_callback) { if (!picture_.get()) { return tonic::ToDart("Picture is null"); } - return RasterizeToImage(picture_.get(), width, height, raw_image_callback); + return RasterizeToImage(std::move(image_handle), picture_.get(), width, + height, raw_image_callback); } void Picture::dispose() { @@ -62,7 +64,8 @@ size_t Picture::GetAllocationSize() { } } -Dart_Handle Picture::RasterizeToImage(sk_sp picture, +Dart_Handle Picture::RasterizeToImage(Dart_Handle image_handle, + sk_sp picture, uint32_t width, uint32_t height, Dart_Handle raw_image_callback) { @@ -89,7 +92,8 @@ Dart_Handle Picture::RasterizeToImage(sk_sp picture, auto picture_bounds = SkISize::Make(width, height); - auto ui_task = fml::MakeCopyable([image_callback, unref_queue]( + auto ui_task = fml::MakeCopyable([image_handle = std::move(image_handle), + image_callback, unref_queue]( sk_sp raster_image) mutable { auto dart_state = image_callback->dart_state().lock(); if (!dart_state) { @@ -103,7 +107,7 @@ Dart_Handle Picture::RasterizeToImage(sk_sp picture, return; } - auto dart_image = CanvasImage::Create(); + auto dart_image = CanvasImage::Create(std::move(image_handle)); dart_image->set_image({std::move(raster_image), std::move(unref_queue)}); auto* raw_dart_image = tonic::ToDart(std::move(dart_image)); diff --git a/lib/ui/painting/picture.h b/lib/ui/painting/picture.h index f6dd98887d264..7ed628550c4f7 100644 --- a/lib/ui/painting/picture.h +++ b/lib/ui/painting/picture.h @@ -28,7 +28,8 @@ class Picture : public RefCountedDartWrappable { sk_sp picture() const { return picture_.get(); } - Dart_Handle toImage(uint32_t width, + Dart_Handle toImage(Dart_Handle image_handle, + uint32_t width, uint32_t height, Dart_Handle raw_image_callback); @@ -38,7 +39,8 @@ class Picture : public RefCountedDartWrappable { static void RegisterNatives(tonic::DartLibraryNatives* natives); - static Dart_Handle RasterizeToImage(sk_sp picture, + static Dart_Handle RasterizeToImage(Dart_Handle image_handle, + sk_sp picture, uint32_t width, uint32_t height, Dart_Handle raw_image_callback); diff --git a/lib/ui/painting/single_frame_codec.cc b/lib/ui/painting/single_frame_codec.cc index 44361f583a583..ccc2fbcca8ead 100644 --- a/lib/ui/painting/single_frame_codec.cc +++ b/lib/ui/painting/single_frame_codec.cc @@ -23,7 +23,9 @@ int SingleFrameCodec::repetitionCount() const { return 0; } -Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle callback_handle) { +Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle image_handle, + Dart_Handle frame_handle, + Dart_Handle callback_handle) { if (!Dart_IsClosure(callback_handle)) { return tonic::ToDart("Callback must be a function"); } @@ -57,7 +59,9 @@ Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle callback_handle) { fml::RefPtr* raw_codec_ref = new fml::RefPtr(this); - decoder->Decode(descriptor_, [raw_codec_ref](auto image) { + decoder->Decode(descriptor_, [image_handle = std::move(image_handle), + frame_handle = std::move(frame_handle), + raw_codec_ref](auto image) { std::unique_ptr> codec_ref(raw_codec_ref); fml::RefPtr codec(std::move(*codec_ref)); @@ -66,18 +70,16 @@ Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle callback_handle) { if (!state) { // This is probably because the isolate has been terminated before the // image could be decoded. - return; } tonic::DartState::Scope scope(state.get()); if (image.get()) { - auto canvas_image = fml::MakeRefCounted(); + auto canvas_image = CanvasImage::Create(std::move(image_handle)); canvas_image->set_image(std::move(image)); - - codec->cached_frame_ = fml::MakeRefCounted( - std::move(canvas_image), 0 /* duration */); + codec->cached_frame_ = FrameInfo::Create( + std::move(frame_handle), std::move(canvas_image), 0 /* duration */); } // The cached frame is now available and should be returned to any future @@ -85,9 +87,8 @@ Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle callback_handle) { codec->status_ = Status::kComplete; // Invoke any callbacks that were provided before the frame was decoded. - Dart_Handle frame = tonic::ToDart(codec->cached_frame_); for (const DartPersistentValue& callback : codec->pending_callbacks_) { - tonic::DartInvoke(callback.value(), {frame}); + tonic::DartInvoke(callback.value(), {Dart_True()}); } codec->pending_callbacks_.clear(); }); diff --git a/lib/ui/painting/single_frame_codec.h b/lib/ui/painting/single_frame_codec.h index b01638c71ce63..b2ec49951aab3 100644 --- a/lib/ui/painting/single_frame_codec.h +++ b/lib/ui/painting/single_frame_codec.h @@ -14,8 +14,13 @@ namespace flutter { class SingleFrameCodec : public Codec { public: - SingleFrameCodec(ImageDecoder::ImageDescriptor descriptor); - + static fml::RefPtr Create( + Dart_Handle codec_handle, + ImageDecoder::ImageDescriptor descriptor) { + auto codec = fml::MakeRefCounted(std::move(descriptor)); + codec->ClaimDartHandle(std::move(codec_handle)); + return codec; + } ~SingleFrameCodec() override; // |Codec| @@ -25,12 +30,15 @@ class SingleFrameCodec : public Codec { int repetitionCount() const override; // |Codec| - Dart_Handle getNextFrame(Dart_Handle args) override; + Dart_Handle getNextFrame(Dart_Handle image_handle, + Dart_Handle codec_handle, + Dart_Handle args) override; // |DartWrappable| size_t GetAllocationSize() override; private: + SingleFrameCodec(ImageDecoder::ImageDescriptor descriptor); enum class Status { kNew, kInProgress, kComplete }; Status status_; ImageDecoder::ImageDescriptor descriptor_; From ad4a6544dd80af244d7f49d78570e801d1497df0 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 20 Feb 2020 10:47:26 -0800 Subject: [PATCH 05/22] start paragraph, fix images,drop frame_info --- ci/licenses_golden/licenses_flutter | 2 -- lib/ui/BUILD.gn | 2 -- lib/ui/dart_ui.cc | 2 -- lib/ui/painting.dart | 30 +++++++++++------- lib/ui/painting/codec.cc | 1 - lib/ui/painting/codec.h | 2 -- lib/ui/painting/frame_info.cc | 30 ------------------ lib/ui/painting/frame_info.h | 45 --------------------------- lib/ui/painting/image.h | 2 +- lib/ui/painting/multi_frame_codec.cc | 43 +++++++++++++------------ lib/ui/painting/multi_frame_codec.h | 8 ++--- lib/ui/painting/single_frame_codec.cc | 16 +++------- lib/ui/painting/single_frame_codec.h | 5 ++- lib/ui/text.dart | 8 ++++- lib/ui/text/paragraph.h | 8 +++-- lib/ui/text/paragraph_builder.cc | 4 +-- lib/ui/text/paragraph_builder.h | 2 +- third_party/tonic/BUILD.gn | 1 + third_party/tonic/dart_wrappable.cc | 4 --- 19 files changed, 68 insertions(+), 147 deletions(-) delete mode 100644 lib/ui/painting/frame_info.cc delete mode 100644 lib/ui/painting/frame_info.h diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index f8cba6e7c66a6..e16578e89aef1 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -296,8 +296,6 @@ FILE: ../../../flutter/lib/ui/painting/color_filter.cc FILE: ../../../flutter/lib/ui/painting/color_filter.h FILE: ../../../flutter/lib/ui/painting/engine_layer.cc FILE: ../../../flutter/lib/ui/painting/engine_layer.h -FILE: ../../../flutter/lib/ui/painting/frame_info.cc -FILE: ../../../flutter/lib/ui/painting/frame_info.h FILE: ../../../flutter/lib/ui/painting/gradient.cc FILE: ../../../flutter/lib/ui/painting/gradient.h FILE: ../../../flutter/lib/ui/painting/image.cc diff --git a/lib/ui/BUILD.gn b/lib/ui/BUILD.gn index 4cc81dff2154d..bd75bd975afa3 100644 --- a/lib/ui/BUILD.gn +++ b/lib/ui/BUILD.gn @@ -30,8 +30,6 @@ source_set("ui") { "painting/color_filter.h", "painting/engine_layer.cc", "painting/engine_layer.h", - "painting/frame_info.cc", - "painting/frame_info.h", "painting/gradient.cc", "painting/gradient.h", "painting/image.cc", diff --git a/lib/ui/dart_ui.cc b/lib/ui/dart_ui.cc index 5b3189dfa80f8..c3b758564a5d0 100644 --- a/lib/ui/dart_ui.cc +++ b/lib/ui/dart_ui.cc @@ -13,7 +13,6 @@ #include "flutter/lib/ui/painting/codec.h" #include "flutter/lib/ui/painting/color_filter.h" #include "flutter/lib/ui/painting/engine_layer.h" -#include "flutter/lib/ui/painting/frame_info.h" #include "flutter/lib/ui/painting/gradient.h" #include "flutter/lib/ui/painting/image.h" #include "flutter/lib/ui/painting/image_filter.h" @@ -80,7 +79,6 @@ void DartUI::InitForGlobal() { DartRuntimeHooks::RegisterNatives(g_natives); EngineLayer::RegisterNatives(g_natives); FontCollection::RegisterNatives(g_natives); - FrameInfo::RegisterNatives(g_natives); ImageFilter::RegisterNatives(g_natives); ImageShader::RegisterNatives(g_natives); IsolateNameServerNatives::RegisterNatives(g_natives); diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index ed9d67eae6507..ba08b08726132 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -1635,22 +1635,19 @@ typedef ImageDecoderCallback = void Function(Image result); /// /// To obtain an instance of the [FrameInfo] interface, see /// [Codec.getNextFrame]. -@pragma('vm:entry-point') -class FrameInfo extends NativeFieldWrapperClass2 { +class FrameInfo { /// This class is created by the engine, and should not be instantiated /// or extended directly. /// /// To obtain an instance of the [FrameInfo] interface, see /// [Codec.getNextFrame]. - @pragma('vm:entry-point') - FrameInfo._(); + FrameInfo._(int durationMilliseconds, this.image) : duration = Duration(milliseconds: durationMilliseconds); /// The duration this frame should be shown. - Duration get duration => Duration(milliseconds: _durationMillis); - int get _durationMillis native 'FrameInfo_durationMillis'; + final Duration duration; /// The [Image] object for this frame. - Image get image native 'FrameInfo_image'; + final Image image; } /// A handle to an image codec. @@ -1680,24 +1677,33 @@ class Codec extends NativeFieldWrapperClass2 { /// * -1 for infinity repetitions. int get repetitionCount native 'Codec_repetitionCount'; + FrameInfo _cachedFrame; + /// Fetches the next animation frame. /// /// Wraps back to the first frame after returning the last frame. /// /// The returned future can complete with an error if the decoding has failed. Future getNextFrame() async { + if (_cachedFrame != null && frameCount == 1) { + return _cachedFrame; + } final Image image = Image._(); - final FrameInfo frameInfo = FrameInfo._(); - await _futurize((_Callback callback) => _getNextFrame(image, frameInfo, callback)); - return frameInfo; + final int durationMilliseconds = await _futurize((_Callback callback) => _getNextFrame(image, callback)); + + return _cachedFrame = FrameInfo._(durationMilliseconds, image); } /// Returns an error message on failure, null on success. - String _getNextFrame(Image image, FrameInfo frameInfo, _Callback callback) native 'Codec_getNextFrame'; + String _getNextFrame(Image image, _Callback callback) native 'Codec_getNextFrame'; /// Release the resources used by this object. The object is no longer usable /// after this method is called. - void dispose() native 'Codec_dispose'; + void dispose() { + _cachedFrame = null; + _dispose(); + } + void _dispose() native 'Codec_dispose'; } /// Instantiates an image codec [Codec] object. diff --git a/lib/ui/painting/codec.cc b/lib/ui/painting/codec.cc index f6ef8c4110763..5d290fc4304a9 100644 --- a/lib/ui/painting/codec.cc +++ b/lib/ui/painting/codec.cc @@ -10,7 +10,6 @@ #include "flutter/fml/logging.h" #include "flutter/fml/make_copyable.h" #include "flutter/fml/trace_event.h" -#include "flutter/lib/ui/painting/frame_info.h" #include "flutter/lib/ui/painting/multi_frame_codec.h" #include "flutter/lib/ui/painting/single_frame_codec.h" #include "third_party/skia/include/codec/SkCodec.h" diff --git a/lib/ui/painting/codec.h b/lib/ui/painting/codec.h index 900e4cc612bae..53ac9e426448d 100644 --- a/lib/ui/painting/codec.h +++ b/lib/ui/painting/codec.h @@ -6,7 +6,6 @@ #define FLUTTER_LIB_UI_PAINTING_CODEC_H_ #include "flutter/lib/ui/dart_wrapper.h" -#include "flutter/lib/ui/painting/frame_info.h" #include "third_party/skia/include/codec/SkCodec.h" #include "third_party/skia/include/core/SkBitmap.h" #include "third_party/skia/include/core/SkImage.h" @@ -31,7 +30,6 @@ class Codec : public RefCountedDartWrappable { virtual int repetitionCount() const = 0; virtual Dart_Handle getNextFrame(Dart_Handle image_handle, - Dart_Handle frame_handle, Dart_Handle callback_handle) = 0; void dispose(); diff --git a/lib/ui/painting/frame_info.cc b/lib/ui/painting/frame_info.cc deleted file mode 100644 index ad77f7b50d8a4..0000000000000 --- a/lib/ui/painting/frame_info.cc +++ /dev/null @@ -1,30 +0,0 @@ - -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "flutter/lib/ui/painting/frame_info.h" - -#include "third_party/tonic/dart_binding_macros.h" -#include "third_party/tonic/dart_library_natives.h" - -namespace flutter { - -IMPLEMENT_WRAPPERTYPEINFO(ui, FrameInfo); - -#define FOR_EACH_BINDING(V) \ - V(FrameInfo, durationMillis) \ - V(FrameInfo, image) - -FOR_EACH_BINDING(DART_NATIVE_CALLBACK) - -FrameInfo::FrameInfo(fml::RefPtr image, int durationMillis) - : image_(std::move(image)), durationMillis_(durationMillis) {} - -FrameInfo::~FrameInfo(){}; - -void FrameInfo::RegisterNatives(tonic::DartLibraryNatives* natives) { - natives->Register({FOR_EACH_BINDING(DART_REGISTER_NATIVE)}); -} - -} // namespace flutter diff --git a/lib/ui/painting/frame_info.h b/lib/ui/painting/frame_info.h deleted file mode 100644 index 515e8e68f605e..0000000000000 --- a/lib/ui/painting/frame_info.h +++ /dev/null @@ -1,45 +0,0 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef FLUTTER_LIB_UI_PAINTING_FRAME_INFO_H_ -#define FLUTTER_LIB_UI_PAINTING_FRAME_INFO_H_ - -#include "flutter/lib/ui/dart_wrapper.h" -#include "flutter/lib/ui/painting/image.h" - -namespace flutter { - -// A single animation frame. -class FrameInfo final : public RefCountedDartWrappable { - DEFINE_WRAPPERTYPEINFO(); - - public: - static fml::RefPtr Create(Dart_Handle frame_handle, - fml::RefPtr image, - int durationMillis) { - auto frame_info = - fml::MakeRefCounted(std::move(image), durationMillis); - frame_info->ClaimDartHandle(std::move(frame_handle)); - return frame_info; - } - int durationMillis() { return durationMillis_; } - fml::RefPtr image() { return image_; } - - static void RegisterNatives(tonic::DartLibraryNatives* natives); - - private: - FrameInfo(fml::RefPtr image, int durationMillis); - - ~FrameInfo() override; - - const fml::RefPtr image_; - const int durationMillis_; - - FML_FRIEND_MAKE_REF_COUNTED(FrameInfo); - FML_FRIEND_REF_COUNTED_THREAD_SAFE(FrameInfo); -}; - -} // namespace flutter - -#endif // FLUTTER_LIB_UI_PAINTING_FRAME_INFO_H_ diff --git a/lib/ui/painting/image.h b/lib/ui/painting/image.h index cff2deb5e1f5c..470bc9bd5e33b 100644 --- a/lib/ui/painting/image.h +++ b/lib/ui/painting/image.h @@ -24,7 +24,7 @@ class CanvasImage final : public RefCountedDartWrappable { ~CanvasImage() override; static fml::RefPtr Create(Dart_Handle dart_handle) { auto image = fml::MakeRefCounted(); - image->ClaimDartHandle(dart_handle); + image->ClaimDartHandle(std::move(dart_handle)); return image; } diff --git a/lib/ui/painting/multi_frame_codec.cc b/lib/ui/painting/multi_frame_codec.cc index 1f38bb403308c..f2fcb9aa54339 100644 --- a/lib/ui/painting/multi_frame_codec.cc +++ b/lib/ui/painting/multi_frame_codec.cc @@ -20,8 +20,11 @@ MultiFrameCodec::MultiFrameCodec(std::unique_ptr codec) MultiFrameCodec::~MultiFrameCodec() = default; static void InvokeNextFrameCallback( - fml::RefPtr frameInfo, + sk_sp skImage, + Dart_Handle image_handle, std::unique_ptr callback, + fml::RefPtr unref_queue, + SkCodec::FrameInfo skFrameInfo, size_t trace_id) { std::shared_ptr dart_state = callback->dart_state().lock(); if (!dart_state) { @@ -30,10 +33,15 @@ static void InvokeNextFrameCallback( return; } tonic::DartState::Scope scope(dart_state); - if (!frameInfo) { - tonic::DartInvoke(callback->value(), {Dart_Null()}); + + if (skImage) { + fml::RefPtr image = + CanvasImage::Create(std::move(image_handle)); + image->set_image({skImage, std::move(unref_queue)}); + tonic::DartInvoke(callback->value(), + {tonic::ToDart(skFrameInfo.fDuration)}); } else { - tonic::DartInvoke(callback->value(), {Dart_True()}); + tonic::DartInvoke(callback->value(), {Dart_Null()}); } } @@ -130,33 +138,30 @@ sk_sp MultiFrameCodec::GetNextFrameImage( void MultiFrameCodec::GetNextFrameAndInvokeCallback( Dart_Handle image_handle, - Dart_Handle frame_handle, std::unique_ptr callback, fml::RefPtr ui_task_runner, fml::WeakPtr resourceContext, fml::RefPtr unref_queue, size_t trace_id) { - fml::RefPtr frameInfo = NULL; sk_sp skImage = GetNextFrameImage(resourceContext); + SkCodec::FrameInfo skFrameInfo; if (skImage) { - fml::RefPtr image = - CanvasImage::Create(std::move(image_handle)); - image->set_image({skImage, std::move(unref_queue)}); - SkCodec::FrameInfo skFrameInfo; codec_->getFrameInfo(nextFrameIndex_, &skFrameInfo); - frameInfo = FrameInfo::Create(std::move(frame_handle), std::move(image), - skFrameInfo.fDuration); } nextFrameIndex_ = (nextFrameIndex_ + 1) % frameCount_; ui_task_runner->PostTask(fml::MakeCopyable( - [callback = std::move(callback), frameInfo, trace_id]() mutable { - InvokeNextFrameCallback(frameInfo, std::move(callback), trace_id); + [skImage = std::move(skImage), callback = std::move(callback), + image_handle = std::move(image_handle), + unref_queue = std::move(unref_queue), + skFrameInfo = std::move(skFrameInfo), trace_id]() mutable { + InvokeNextFrameCallback(std::move(skImage), std::move(image_handle), + std::move(callback), std::move(unref_queue), + std::move(skFrameInfo), trace_id); })); } Dart_Handle MultiFrameCodec::getNextFrame(Dart_Handle image_handle, - Dart_Handle frame_handle, Dart_Handle callback_handle) { static size_t trace_counter = 1; const size_t trace_id = trace_counter++; @@ -171,16 +176,14 @@ Dart_Handle MultiFrameCodec::getNextFrame(Dart_Handle image_handle, task_runners.GetIOTaskRunner()->PostTask(fml::MakeCopyable( [image_handle = std::move(image_handle), - frame_handle = std::move(frame_handle), callback = std::make_unique( tonic::DartState::Current(), callback_handle), this, trace_id, ui_task_runner = task_runners.GetUITaskRunner(), io_manager = dart_state->GetIOManager()]() mutable { GetNextFrameAndInvokeCallback( - std::move(image_handle), std::move(frame_handle), - std::move(callback), std::move(ui_task_runner), - io_manager->GetResourceContext(), io_manager->GetSkiaUnrefQueue(), - trace_id); + std::move(image_handle), std::move(callback), + std::move(ui_task_runner), io_manager->GetResourceContext(), + io_manager->GetSkiaUnrefQueue(), trace_id); })); return Dart_Null(); diff --git a/lib/ui/painting/multi_frame_codec.h b/lib/ui/painting/multi_frame_codec.h index ccd205c0b6800..b2ee278def7fd 100644 --- a/lib/ui/painting/multi_frame_codec.h +++ b/lib/ui/painting/multi_frame_codec.h @@ -5,8 +5,11 @@ #ifndef FLUTTER_LIB_UI_PAINTING_MUTLI_FRAME_CODEC_H_ #define FLUTTER_LIB_UI_PAINTING_MUTLI_FRAME_CODEC_H_ +#include "flutter/flow/skia_gpu_object.h" #include "flutter/fml/macros.h" +#include "flutter/fml/memory/weak_ptr.h" #include "flutter/lib/ui/painting/codec.h" +#include "flutter/lib/ui/painting/image.h" namespace flutter { @@ -28,9 +31,7 @@ class MultiFrameCodec : public Codec { int repetitionCount() const override; // |Codec| - Dart_Handle getNextFrame(Dart_Handle image_handle, - Dart_Handle frame_handle, - Dart_Handle args) override; + Dart_Handle getNextFrame(Dart_Handle image_handle, Dart_Handle args) override; private: MultiFrameCodec(std::unique_ptr codec); @@ -48,7 +49,6 @@ class MultiFrameCodec : public Codec { void GetNextFrameAndInvokeCallback( Dart_Handle image_handle, - Dart_Handle frame_handle, std::unique_ptr callback, fml::RefPtr ui_task_runner, fml::WeakPtr resourceContext, diff --git a/lib/ui/painting/single_frame_codec.cc b/lib/ui/painting/single_frame_codec.cc index ccc2fbcca8ead..e2c07126553d1 100644 --- a/lib/ui/painting/single_frame_codec.cc +++ b/lib/ui/painting/single_frame_codec.cc @@ -4,7 +4,6 @@ #include "flutter/lib/ui/painting/single_frame_codec.h" -#include "flutter/lib/ui/painting/frame_info.h" #include "flutter/lib/ui/ui_dart_state.h" #include "third_party/tonic/logging/dart_invoke.h" @@ -24,15 +23,13 @@ int SingleFrameCodec::repetitionCount() const { } Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle image_handle, - Dart_Handle frame_handle, Dart_Handle callback_handle) { if (!Dart_IsClosure(callback_handle)) { return tonic::ToDart("Callback must be a function"); } if (status_ == Status::kComplete) { - tonic::DartInvoke(callback_handle, {tonic::ToDart(cached_frame_)}); - return Dart_Null(); + return tonic::ToDart("Dart callers are responsible for caching the frame callback information"); } // This has to be valid because this method is called from Dart. @@ -60,7 +57,6 @@ Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle image_handle, new fml::RefPtr(this); decoder->Decode(descriptor_, [image_handle = std::move(image_handle), - frame_handle = std::move(frame_handle), raw_codec_ref](auto image) { std::unique_ptr> codec_ref(raw_codec_ref); fml::RefPtr codec(std::move(*codec_ref)); @@ -78,8 +74,7 @@ Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle image_handle, if (image.get()) { auto canvas_image = CanvasImage::Create(std::move(image_handle)); canvas_image->set_image(std::move(image)); - codec->cached_frame_ = FrameInfo::Create( - std::move(frame_handle), std::move(canvas_image), 0 /* duration */); + codec->cached_frame_image_size_ = canvas_image->GetAllocationSize(); } // The cached frame is now available and should be returned to any future @@ -88,7 +83,7 @@ Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle image_handle, // Invoke any callbacks that were provided before the frame was decoded. for (const DartPersistentValue& callback : codec->pending_callbacks_) { - tonic::DartInvoke(callback.value(), {Dart_True()}); + tonic::DartInvoke(callback.value(), {tonic::ToDart(0)}); } codec->pending_callbacks_.clear(); }); @@ -105,10 +100,7 @@ Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle image_handle, size_t SingleFrameCodec::GetAllocationSize() { const auto& data = descriptor_.data; const auto data_byte_size = data ? data->size() : 0; - const auto frame_byte_size = (cached_frame_ && cached_frame_->image()) - ? cached_frame_->image()->GetAllocationSize() - : 0; - return data_byte_size + frame_byte_size + sizeof(this); + return data_byte_size + cached_frame_image_size_ + sizeof(this); } } // namespace flutter diff --git a/lib/ui/painting/single_frame_codec.h b/lib/ui/painting/single_frame_codec.h index b2ec49951aab3..c53b54510c60a 100644 --- a/lib/ui/painting/single_frame_codec.h +++ b/lib/ui/painting/single_frame_codec.h @@ -6,8 +6,8 @@ #define FLUTTER_LIB_UI_PAINTING_SINGLE_FRAME_CODEC_H_ #include "flutter/fml/macros.h" +#include "flutter/lib/ui/painting/image.h" #include "flutter/lib/ui/painting/codec.h" -#include "flutter/lib/ui/painting/frame_info.h" #include "flutter/lib/ui/painting/image_decoder.h" namespace flutter { @@ -31,7 +31,6 @@ class SingleFrameCodec : public Codec { // |Codec| Dart_Handle getNextFrame(Dart_Handle image_handle, - Dart_Handle codec_handle, Dart_Handle args) override; // |DartWrappable| @@ -42,7 +41,7 @@ class SingleFrameCodec : public Codec { enum class Status { kNew, kInProgress, kComplete }; Status status_; ImageDecoder::ImageDescriptor descriptor_; - fml::RefPtr cached_frame_; + size_t cached_frame_image_size_; std::vector pending_callbacks_; FML_FRIEND_MAKE_REF_COUNTED(SingleFrameCodec); diff --git a/lib/ui/text.dart b/lib/ui/text.dart index beb54eafb068c..90fe60cda3195 100644 --- a/lib/ui/text.dart +++ b/lib/ui/text.dart @@ -2194,7 +2194,13 @@ class ParagraphBuilder extends NativeFieldWrapperClass2 { /// /// After calling this function, the paragraph builder object is invalid and /// cannot be used further. - Paragraph build() native 'ParagraphBuilder_build'; + Paragraph build() { + final Paragraph paragraph = Paragraph._(); + _build(paragraph); + return paragraph; + } + + void _build(Paragraph paragraph) native 'ParagraphBuilder_build'; } /// Loads a font from a buffer and makes it available for rendering text. diff --git a/lib/ui/text/paragraph.h b/lib/ui/text/paragraph.h index 7aea6079378e1..67323384ac849 100644 --- a/lib/ui/text/paragraph.h +++ b/lib/ui/text/paragraph.h @@ -23,9 +23,11 @@ class Paragraph : public RefCountedDartWrappable { FML_FRIEND_MAKE_REF_COUNTED(Paragraph); public: - static fml::RefPtr Create( - std::unique_ptr paragraph) { - return fml::MakeRefCounted(std::move(paragraph)); + static void Create( + Dart_Handle paragraph_handle, + std::unique_ptr txt_paragraph) { + auto paragraph = fml::MakeRefCounted(std::move(txt_paragraph)); + paragraph->ClaimDartHandle(std::move(paragraph_handle)); } ~Paragraph() override; diff --git a/lib/ui/text/paragraph_builder.cc b/lib/ui/text/paragraph_builder.cc index 51739fc2cf3fd..208b4e3a56770 100644 --- a/lib/ui/text/paragraph_builder.cc +++ b/lib/ui/text/paragraph_builder.cc @@ -493,8 +493,8 @@ Dart_Handle ParagraphBuilder::addPlaceholder(double width, return Dart_Null(); } -fml::RefPtr ParagraphBuilder::build() { - return Paragraph::Create(m_paragraphBuilder->Build()); +void ParagraphBuilder::build(Dart_Handle paragraph_handle) { + Paragraph::Create(std::move(paragraph_handle), m_paragraphBuilder->Build()); } } // namespace flutter diff --git a/lib/ui/text/paragraph_builder.h b/lib/ui/text/paragraph_builder.h index eebc5e7f26868..4f4a00b5b94ab 100644 --- a/lib/ui/text/paragraph_builder.h +++ b/lib/ui/text/paragraph_builder.h @@ -69,7 +69,7 @@ class ParagraphBuilder : public RefCountedDartWrappable { double baseline_offset, unsigned baseline); - fml::RefPtr build(); + void build(Dart_Handle paragraph_handle); static void RegisterNatives(tonic::DartLibraryNatives* natives); diff --git a/third_party/tonic/BUILD.gn b/third_party/tonic/BUILD.gn index eb0136d54bd59..06a54a3811d5a 100644 --- a/third_party/tonic/BUILD.gn +++ b/third_party/tonic/BUILD.gn @@ -42,6 +42,7 @@ source_set("tonic") { "scopes", "typed_data", "//third_party/dart/runtime:dart_api", + "//flutter/fml" ] public_configs = [ ":config" ] diff --git a/third_party/tonic/dart_wrappable.cc b/third_party/tonic/dart_wrappable.cc index a1a7faafe46df..5e09f900d5d8f 100644 --- a/third_party/tonic/dart_wrappable.cc +++ b/third_party/tonic/dart_wrappable.cc @@ -44,10 +44,6 @@ void DartWrappable::ClaimDartHandle(Dart_Handle wrapper) { const DartWrapperInfo& info = GetDartWrapperInfo(); - intptr_t native_fields[kNumberOfNativeFields]; - native_fields[kPeerIndex] = reinterpret_cast(this); - native_fields[kWrapperInfoIndex] = reinterpret_cast(&info); - TONIC_CHECK(!LogIfError(Dart_SetNativeInstanceField( wrapper, kPeerIndex, reinterpret_cast(this)))); TONIC_CHECK(!LogIfError(Dart_SetNativeInstanceField( From 027ce427778316498b4f7ecc6c9b56cb425d26ad Mon Sep 17 00:00:00 2001 From: Dan Field Date: Sat, 22 Feb 2020 00:04:48 -0800 Subject: [PATCH 06/22] merge more --- lib/ui/painting/codec.cc | 4 ++-- lib/ui/painting/image.h | 2 +- lib/ui/painting/multi_frame_codec.cc | 12 +++++------- lib/ui/painting/multi_frame_codec.h | 2 +- lib/ui/painting/path.cc | 6 +++--- lib/ui/painting/path.h | 4 ++-- lib/ui/painting/path_measure.cc | 6 +++--- lib/ui/painting/picture.cc | 6 +++--- lib/ui/painting/single_frame_codec.cc | 4 ++-- lib/ui/painting/single_frame_codec.h | 2 +- lib/ui/text/paragraph.h | 2 +- lib/ui/text/paragraph_builder.cc | 2 +- 12 files changed, 25 insertions(+), 27 deletions(-) diff --git a/lib/ui/painting/codec.cc b/lib/ui/painting/codec.cc index 5d290fc4304a9..76ee2b9fe71e1 100644 --- a/lib/ui/painting/codec.cc +++ b/lib/ui/painting/codec.cc @@ -220,9 +220,9 @@ static void InstantiateImageCodec(Dart_NativeArguments args) { } descriptor.data = std::move(buffer); - SingleFrameCodec::Create(std::move(codec_handle), std::move(descriptor)); + SingleFrameCodec::Create(codec_handle, std::move(descriptor)); } else { - MultiFrameCodec::Create(std::move(codec_handle), std::move(codec)); + MultiFrameCodec::Create(codec_handle, std::move(codec)); } tonic::DartInvoke(callback_handle, {Dart_True()}); diff --git a/lib/ui/painting/image.h b/lib/ui/painting/image.h index 470bc9bd5e33b..43a00517421b8 100644 --- a/lib/ui/painting/image.h +++ b/lib/ui/painting/image.h @@ -24,7 +24,7 @@ class CanvasImage final : public RefCountedDartWrappable { ~CanvasImage() override; static fml::RefPtr Create(Dart_Handle dart_handle) { auto image = fml::MakeRefCounted(); - image->ClaimDartHandle(std::move(dart_handle)); + image->AssociateWithDartWrapper(dart_handle); return image; } diff --git a/lib/ui/painting/multi_frame_codec.cc b/lib/ui/painting/multi_frame_codec.cc index f2fcb9aa54339..d638c32706518 100644 --- a/lib/ui/painting/multi_frame_codec.cc +++ b/lib/ui/painting/multi_frame_codec.cc @@ -35,8 +35,7 @@ static void InvokeNextFrameCallback( tonic::DartState::Scope scope(dart_state); if (skImage) { - fml::RefPtr image = - CanvasImage::Create(std::move(image_handle)); + fml::RefPtr image = CanvasImage::Create(image_handle); image->set_image({skImage, std::move(unref_queue)}); tonic::DartInvoke(callback->value(), {tonic::ToDart(skFrameInfo.fDuration)}); @@ -152,10 +151,9 @@ void MultiFrameCodec::GetNextFrameAndInvokeCallback( ui_task_runner->PostTask(fml::MakeCopyable( [skImage = std::move(skImage), callback = std::move(callback), - image_handle = std::move(image_handle), - unref_queue = std::move(unref_queue), + image_handle, unref_queue = std::move(unref_queue), skFrameInfo = std::move(skFrameInfo), trace_id]() mutable { - InvokeNextFrameCallback(std::move(skImage), std::move(image_handle), + InvokeNextFrameCallback(std::move(skImage), image_handle, std::move(callback), std::move(unref_queue), std::move(skFrameInfo), trace_id); })); @@ -175,13 +173,13 @@ Dart_Handle MultiFrameCodec::getNextFrame(Dart_Handle image_handle, const auto& task_runners = dart_state->GetTaskRunners(); task_runners.GetIOTaskRunner()->PostTask(fml::MakeCopyable( - [image_handle = std::move(image_handle), + [image_handle, callback = std::make_unique( tonic::DartState::Current(), callback_handle), this, trace_id, ui_task_runner = task_runners.GetUITaskRunner(), io_manager = dart_state->GetIOManager()]() mutable { GetNextFrameAndInvokeCallback( - std::move(image_handle), std::move(callback), + image_handle, std::move(callback), std::move(ui_task_runner), io_manager->GetResourceContext(), io_manager->GetSkiaUnrefQueue(), trace_id); })); diff --git a/lib/ui/painting/multi_frame_codec.h b/lib/ui/painting/multi_frame_codec.h index b2ee278def7fd..bee27844aabc5 100644 --- a/lib/ui/painting/multi_frame_codec.h +++ b/lib/ui/painting/multi_frame_codec.h @@ -19,7 +19,7 @@ class MultiFrameCodec : public Codec { std::unique_ptr codec) { auto multi_frame_codec = fml::MakeRefCounted(std::move(codec)); - multi_frame_codec->ClaimDartHandle(std::move(codec_handle)); + multi_frame_codec->AssociateWithDartWrapper(codec_handle); return multi_frame_codec; } ~MultiFrameCodec() override; diff --git a/lib/ui/painting/path.cc b/lib/ui/painting/path.cc index 0227c07a8f105..b3633227787be 100644 --- a/lib/ui/painting/path.cc +++ b/lib/ui/painting/path.cc @@ -263,12 +263,12 @@ bool CanvasPath::contains(double x, double y) { } void CanvasPath::shift(Dart_Handle path_handle, double dx, double dy) { - fml::RefPtr path = CanvasPath::Create(std::move(path_handle)); + fml::RefPtr path = CanvasPath::Create(path_handle); path_.offset(dx, dy, &path->path_); } void CanvasPath::transform(Dart_Handle path_handle, tonic::Float64List& matrix4) { - fml::RefPtr path = CanvasPath::Create(std::move(path_handle)); + fml::RefPtr path = CanvasPath::Create(path_handle); path_.transform(ToSkMatrix(matrix4), &path->path_); matrix4.Release(); } @@ -288,7 +288,7 @@ bool CanvasPath::op(CanvasPath* path1, CanvasPath* path2, int operation) { } void CanvasPath::clone(Dart_Handle path_handle) { - fml::RefPtr path = CanvasPath::Create(std::move(path_handle)); + fml::RefPtr path = CanvasPath::Create(path_handle); // per Skia docs, this will create a fast copy // data is shared until the source path or dest path are mutated path->path_ = path_; diff --git a/lib/ui/painting/path.h b/lib/ui/painting/path.h index 73ed8304ae722..7deb50e4e7d7c 100644 --- a/lib/ui/painting/path.h +++ b/lib/ui/painting/path.h @@ -29,13 +29,13 @@ class CanvasPath : public RefCountedDartWrappable { static fml::RefPtr Create(Dart_Handle path_handle) { auto path = fml::MakeRefCounted(); - path->ClaimDartHandle(std::move(path_handle)); + path->AssociateWithDartWrapper(path_handle); return path; } static fml::RefPtr CreateFrom(Dart_Handle path_handle, const SkPath& src) { - fml::RefPtr path = CanvasPath::Create(std::move(path_handle)); + fml::RefPtr path = CanvasPath::Create(path_handle); path->path_ = src; return path; } diff --git a/lib/ui/painting/path_measure.cc b/lib/ui/painting/path_measure.cc index 8ef30ddbe1c3a..e3e6f39d0d426 100644 --- a/lib/ui/painting/path_measure.cc +++ b/lib/ui/painting/path_measure.cc @@ -104,15 +104,15 @@ void CanvasPathMeasure::getSegment(Dart_Handle path_handle, bool start_with_move_to) { if (static_cast>::size_type>( contour_index) >= measures_.size()) { - CanvasPath::Create(std::move(path_handle)); + CanvasPath::Create(path_handle); } SkPath dst; bool success = measures_[contour_index]->getSegment(start_d, stop_d, &dst, start_with_move_to); if (!success) { - CanvasPath::Create(std::move(path_handle)); + CanvasPath::Create(path_handle); } else { - CanvasPath::CreateFrom(std::move(path_handle), dst); + CanvasPath::CreateFrom(path_handle, dst); } } diff --git a/lib/ui/painting/picture.cc b/lib/ui/painting/picture.cc index d451518ad7272..b0b808edd2f8a 100644 --- a/lib/ui/painting/picture.cc +++ b/lib/ui/painting/picture.cc @@ -48,7 +48,7 @@ Dart_Handle Picture::toImage(Dart_Handle image_handle, return tonic::ToDart("Picture is null"); } - return RasterizeToImage(std::move(image_handle), picture_.get(), width, + return RasterizeToImage(image_handle, picture_.get(), width, height, raw_image_callback); } @@ -92,7 +92,7 @@ Dart_Handle Picture::RasterizeToImage(Dart_Handle image_handle, auto picture_bounds = SkISize::Make(width, height); - auto ui_task = fml::MakeCopyable([image_handle = std::move(image_handle), + auto ui_task = fml::MakeCopyable([image_handle, image_callback, unref_queue]( sk_sp raster_image) mutable { auto dart_state = image_callback->dart_state().lock(); @@ -107,7 +107,7 @@ Dart_Handle Picture::RasterizeToImage(Dart_Handle image_handle, return; } - auto dart_image = CanvasImage::Create(std::move(image_handle)); + auto dart_image = CanvasImage::Create(image_handle); dart_image->set_image({std::move(raster_image), std::move(unref_queue)}); auto* raw_dart_image = tonic::ToDart(std::move(dart_image)); diff --git a/lib/ui/painting/single_frame_codec.cc b/lib/ui/painting/single_frame_codec.cc index e2c07126553d1..00ad7bec055f5 100644 --- a/lib/ui/painting/single_frame_codec.cc +++ b/lib/ui/painting/single_frame_codec.cc @@ -56,7 +56,7 @@ Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle image_handle, fml::RefPtr* raw_codec_ref = new fml::RefPtr(this); - decoder->Decode(descriptor_, [image_handle = std::move(image_handle), + decoder->Decode(descriptor_, [image_handle, raw_codec_ref](auto image) { std::unique_ptr> codec_ref(raw_codec_ref); fml::RefPtr codec(std::move(*codec_ref)); @@ -72,7 +72,7 @@ Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle image_handle, tonic::DartState::Scope scope(state.get()); if (image.get()) { - auto canvas_image = CanvasImage::Create(std::move(image_handle)); + auto canvas_image = CanvasImage::Create(image_handle); canvas_image->set_image(std::move(image)); codec->cached_frame_image_size_ = canvas_image->GetAllocationSize(); } diff --git a/lib/ui/painting/single_frame_codec.h b/lib/ui/painting/single_frame_codec.h index c53b54510c60a..dd4d9563a3206 100644 --- a/lib/ui/painting/single_frame_codec.h +++ b/lib/ui/painting/single_frame_codec.h @@ -18,7 +18,7 @@ class SingleFrameCodec : public Codec { Dart_Handle codec_handle, ImageDecoder::ImageDescriptor descriptor) { auto codec = fml::MakeRefCounted(std::move(descriptor)); - codec->ClaimDartHandle(std::move(codec_handle)); + codec->AssociateWithDartWrapper(codec_handle); return codec; } ~SingleFrameCodec() override; diff --git a/lib/ui/text/paragraph.h b/lib/ui/text/paragraph.h index 67323384ac849..e8a6ae4e94e57 100644 --- a/lib/ui/text/paragraph.h +++ b/lib/ui/text/paragraph.h @@ -27,7 +27,7 @@ class Paragraph : public RefCountedDartWrappable { Dart_Handle paragraph_handle, std::unique_ptr txt_paragraph) { auto paragraph = fml::MakeRefCounted(std::move(txt_paragraph)); - paragraph->ClaimDartHandle(std::move(paragraph_handle)); + paragraph->AssociateWithDartWrapper(paragraph_handle); } ~Paragraph() override; diff --git a/lib/ui/text/paragraph_builder.cc b/lib/ui/text/paragraph_builder.cc index 208b4e3a56770..0b6eea6e0abfd 100644 --- a/lib/ui/text/paragraph_builder.cc +++ b/lib/ui/text/paragraph_builder.cc @@ -494,7 +494,7 @@ Dart_Handle ParagraphBuilder::addPlaceholder(double width, } void ParagraphBuilder::build(Dart_Handle paragraph_handle) { - Paragraph::Create(std::move(paragraph_handle), m_paragraphBuilder->Build()); + Paragraph::Create(paragraph_handle, m_paragraphBuilder->Build()); } } // namespace flutter From 25106df293e8a793725bb73471fc0bf38658c181 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 27 Feb 2020 15:15:23 -0800 Subject: [PATCH 07/22] remove usage of Dart_New for paragraph/libtxt --- ci/licenses_golden/licenses_flutter | 2 - lib/ui/BUILD.gn | 2 - lib/ui/text.dart | 76 ++++++++++++++++++----------- lib/ui/text/line_metrics.cc | 48 ------------------ lib/ui/text/line_metrics.h | 13 ----- lib/ui/text/paragraph.cc | 73 +++++++++++++++++++-------- lib/ui/text/paragraph.h | 19 ++++---- lib/ui/text/paragraph_builder.cc | 4 +- lib/ui/text/paragraph_builder.h | 2 +- lib/ui/text/text_box.cc | 45 ----------------- 10 files changed, 112 insertions(+), 172 deletions(-) delete mode 100644 lib/ui/text/line_metrics.cc delete mode 100644 lib/ui/text/text_box.cc diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 21d438c4a7b41..d0182f4580ab0 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -354,13 +354,11 @@ FILE: ../../../flutter/lib/ui/text/asset_manager_font_provider.cc FILE: ../../../flutter/lib/ui/text/asset_manager_font_provider.h FILE: ../../../flutter/lib/ui/text/font_collection.cc FILE: ../../../flutter/lib/ui/text/font_collection.h -FILE: ../../../flutter/lib/ui/text/line_metrics.cc FILE: ../../../flutter/lib/ui/text/line_metrics.h FILE: ../../../flutter/lib/ui/text/paragraph.cc FILE: ../../../flutter/lib/ui/text/paragraph.h FILE: ../../../flutter/lib/ui/text/paragraph_builder.cc FILE: ../../../flutter/lib/ui/text/paragraph_builder.h -FILE: ../../../flutter/lib/ui/text/text_box.cc FILE: ../../../flutter/lib/ui/text/text_box.h FILE: ../../../flutter/lib/ui/ui.dart FILE: ../../../flutter/lib/ui/ui_dart_state.cc diff --git a/lib/ui/BUILD.gn b/lib/ui/BUILD.gn index 4cc81dff2154d..c904141b338e8 100644 --- a/lib/ui/BUILD.gn +++ b/lib/ui/BUILD.gn @@ -81,13 +81,11 @@ source_set("ui") { "text/asset_manager_font_provider.h", "text/font_collection.cc", "text/font_collection.h", - "text/line_metrics.cc", "text/line_metrics.h", "text/paragraph.cc", "text/paragraph.h", "text/paragraph_builder.cc", "text/paragraph_builder.h", - "text/text_box.cc", "text/text_box.h", "ui_dart_state.cc", "ui_dart_state.h", diff --git a/lib/ui/text.dart b/lib/ui/text.dart index bba18ded716be..f33ffbcc3762c 100644 --- a/lib/ui/text.dart +++ b/lib/ui/text.dart @@ -1306,16 +1306,6 @@ class TextBox { this.direction, ); - @pragma('vm:entry-point') - // ignore: unused_element - TextBox._( - this.left, - this.top, - this.right, - this.bottom, - int directionIndex, - ) : direction = TextDirection.values[directionIndex]; - /// The left edge of the text box, irrespective of direction. /// /// To get the leading edge (which may depend on the [direction]), consider [start]. @@ -1764,20 +1754,6 @@ class LineMetrics { this.lineNumber, }); - @pragma('vm:entry-point') - // ignore: unused_element - LineMetrics._( - this.hardBreak, - this.ascent, - this.descent, - this.unscaledAscent, - this.height, - this.width, - this.left, - this.baseline, - this.lineNumber, - ); - /// True if this line ends with an explicit line break (e.g. '\n') or is the end /// of the paragraph. False otherwise. final bool hardBreak; @@ -1914,6 +1890,20 @@ class Paragraph extends NativeFieldWrapperClass2 { void layout(ParagraphConstraints constraints) => _layout(constraints.width); void _layout(double width) native 'Paragraph_layout'; + List _decodeTextBoxes(Float32List encoded) { + final List boxes = List(encoded[0].toInt()); + for (int index = 0; index < boxes.length; index += 1) { + final int position = (index * 5) + 1; + boxes[index] = TextBox.fromLTRBD( + encoded[position + 0], + encoded[position + 1], + encoded[position + 2], + encoded[position + 3], + TextDirection.values[encoded[position + 4].toInt()], + ); + } + return boxes; + } /// Returns a list of text boxes that enclose the given text range. /// /// The [boxHeightStyle] and [boxWidthStyle] parameters allow customization @@ -1930,9 +1920,10 @@ class Paragraph extends NativeFieldWrapperClass2 { List getBoxesForRange(int start, int end, {BoxHeightStyle boxHeightStyle = BoxHeightStyle.tight, BoxWidthStyle boxWidthStyle = BoxWidthStyle.tight}) { assert(boxHeightStyle != null); assert(boxWidthStyle != null); - return _getBoxesForRange(start, end, boxHeightStyle.index, boxWidthStyle.index); + return _decodeTextBoxes(_getBoxesForRange(start, end, boxHeightStyle.index, boxWidthStyle.index)); } - List _getBoxesForRange(int start, int end, int boxHeightStyle, int boxWidthStyle) native 'Paragraph_getRectsForRange'; + // See paragraph.cc for the layout of this return value. + Float32List _getBoxesForRange(int start, int end, int boxHeightStyle, int boxWidthStyle) native 'Paragraph_getRectsForRange'; /// Returns a list of text boxes that enclose all placeholders in the paragraph. /// @@ -1940,7 +1931,10 @@ class Paragraph extends NativeFieldWrapperClass2 { /// /// Coordinates of the [TextBox] are relative to the upper-left corner of the paragraph, /// where positive y values indicate down. - List getBoxesForPlaceholders() native 'Paragraph_getRectsForPlaceholders'; + List getBoxesForPlaceholders() { + return _decodeTextBoxes(_getBoxesForPlaceholders()); + } + Float32List _getBoxesForPlaceholders() native 'Paragraph_getRectsForPlaceholders'; /// Returns the text position closest to the given offset. TextPosition getPositionForOffset(Offset offset) { @@ -1987,7 +1981,26 @@ class Paragraph extends NativeFieldWrapperClass2 { /// /// This can potentially return a large amount of data, so it is not recommended /// to repeatedly call this. Instead, cache the results. - List computeLineMetrics() native 'Paragraph_computeLineMetrics'; + List computeLineMetrics() { + final Float64List encoded = _computeLineMetrics(); + final List metrics = List(encoded[0].toInt()); + for (int index = 0; index < metrics.length; index += 1) { + final int position = (index * 5) + 1; + metrics[index] = LineMetrics( + hardBreak: encoded[position + 0] != 0, + ascent: encoded[position + 1], + descent: encoded[position + 2], + unscaledAscent: encoded[position + 3], + height: encoded[position + 4], + width: encoded[position + 5], + left: encoded[position + 6], + baseline: encoded[position + 7], + lineNumber: encoded[position + 8].toInt(), + ); + } + return metrics; + } + Float64List _computeLineMetrics() native 'Paragraph_computeLineMetrics'; } /// Builds a [Paragraph] containing text with the given styling information. @@ -2195,7 +2208,12 @@ class ParagraphBuilder extends NativeFieldWrapperClass2 { /// /// After calling this function, the paragraph builder object is invalid and /// cannot be used further. - Paragraph build() native 'ParagraphBuilder_build'; + Paragraph build() { + final Paragraph paragraph = Paragraph._(); + _build(paragraph); + return paragraph; + } + void _build(Paragraph outParagraph) native 'ParagraphBuilder_build'; } /// Loads a font from a buffer and makes it available for rendering text. diff --git a/lib/ui/text/line_metrics.cc b/lib/ui/text/line_metrics.cc deleted file mode 100644 index a224e9b29b66c..0000000000000 --- a/lib/ui/text/line_metrics.cc +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "flutter/lib/ui/text/line_metrics.h" - -#include "flutter/fml/logging.h" -#include "third_party/tonic/dart_class_library.h" -#include "third_party/tonic/dart_state.h" -#include "third_party/tonic/logging/dart_error.h" - -using namespace flutter; - -namespace tonic { - -namespace { - -Dart_Handle GetLineMetricsType() { - DartClassLibrary& class_library = DartState::Current()->class_library(); - Dart_Handle type = - Dart_HandleFromPersistent(class_library.GetClass("ui", "LineMetrics")); - FML_DCHECK(!LogIfError(type)); - return type; -} - -} // anonymous namespace - -Dart_Handle DartConverter::ToDart( - const flutter::LineMetrics& val) { - constexpr int argc = 9; - - Dart_Handle argv[argc] = { - tonic::ToDart(*val.hard_break), tonic::ToDart(*val.ascent), - tonic::ToDart(*val.descent), tonic::ToDart(*val.unscaled_ascent), - // We add then round to get the height. The - // definition of height here is different - // than the one in LibTxt. - tonic::ToDart(round(*val.ascent + *val.descent)), - tonic::ToDart(*val.width), tonic::ToDart(*val.left), - tonic::ToDart(*val.baseline), tonic::ToDart(*val.line_number)}; - return Dart_New(GetLineMetricsType(), tonic::ToDart("_"), argc, argv); -} - -Dart_Handle DartListFactory::NewList(intptr_t length) { - return Dart_NewListOfType(GetLineMetricsType(), length); -} - -} // namespace tonic diff --git a/lib/ui/text/line_metrics.h b/lib/ui/text/line_metrics.h index bcd53c3d276a9..f5f656cd816f9 100644 --- a/lib/ui/text/line_metrics.h +++ b/lib/ui/text/line_metrics.h @@ -59,17 +59,4 @@ struct LineMetrics { } // namespace flutter -namespace tonic { -template <> -struct DartConverter { - static Dart_Handle ToDart(const flutter::LineMetrics& val); -}; - -template <> -struct DartListFactory { - static Dart_Handle NewList(intptr_t length); -}; - -} // namespace tonic - #endif // FLUTTER_LIB_UI_TEXT_LINE_METRICS_H_ diff --git a/lib/ui/text/paragraph.cc b/lib/ui/text/paragraph.cc index 2618c3f08406a..2776dfd7629af 100644 --- a/lib/ui/text/paragraph.cc +++ b/lib/ui/text/paragraph.cc @@ -94,28 +94,40 @@ void Paragraph::paint(Canvas* canvas, double x, double y) { m_paragraph->Paint(sk_canvas, x, y); } -std::vector Paragraph::getRectsForRange(unsigned start, - unsigned end, - unsigned boxHeightStyle, - unsigned boxWidthStyle) { - std::vector result; +static tonic::Float32List EncodeTextBoxes(const std::vector& boxes) { + // Layout: + // First value is the number of values. + // Then there are boxes.size() groups of 5 which are LTRBD, where D is the + // text direction index. + auto count = boxes.size() * 5; + tonic::Float32List result(Dart_NewTypedData(Dart_TypedData_kFloat32, 1 + count)); + result[0] = count; + for (unsigned long i = 0; i < boxes.size(); i++) { + auto position = (i * 5) + 1; + const txt::Paragraph::TextBox& box = boxes[i]; + result[position + 0] = box.rect.fLeft; + result[position + 1] = box.rect.fTop; + result[position + 2] = box.rect.fRight; + result[position + 3] = box.rect.fBottom; + result[position + 4] = static_cast(box.direction); + } + return result; +} + +tonic::Float32List Paragraph::getRectsForRange(unsigned start, + unsigned end, + unsigned boxHeightStyle, + unsigned boxWidthStyle) { std::vector boxes = m_paragraph->GetRectsForRange( start, end, static_cast(boxHeightStyle), static_cast(boxWidthStyle)); - for (const txt::Paragraph::TextBox& box : boxes) { - result.emplace_back(box.rect, static_cast(box.direction)); - } - return result; + return EncodeTextBoxes(boxes); } -std::vector Paragraph::getRectsForPlaceholders() { - std::vector result; +tonic::Float32List Paragraph::getRectsForPlaceholders() { std::vector boxes = m_paragraph->GetRectsForPlaceholders(); - for (const txt::Paragraph::TextBox& box : boxes) { - result.emplace_back(box.rect, static_cast(box.direction)); - } - return result; + return EncodeTextBoxes(boxes); } Dart_Handle Paragraph::getPositionForOffset(double dx, double dy) { @@ -152,14 +164,33 @@ Dart_Handle Paragraph::getLineBoundary(unsigned offset) { return result; } -std::vector Paragraph::computeLineMetrics() { - std::vector result; +tonic::Float64List Paragraph::computeLineMetrics() { std::vector metrics = m_paragraph->GetLineMetrics(); - for (txt::LineMetrics& line : metrics) { - result.emplace_back(&line.hard_break, &line.ascent, &line.descent, - &line.unscaled_ascent, &line.height, &line.width, - &line.left, &line.baseline, &line.line_number); + + // Layout: + // First value is the number of values. + // Then there are boxes.size() groups of 9 which are the line metrics + // properties + auto count = metrics.size() * 9; + tonic::Float64List result(Dart_NewTypedData(Dart_TypedData_kFloat64, 1 + count)); + result[0] = count; + for (unsigned long i = 0; i < metrics.size(); i++) { + auto position = (i * 9) + 1; + const txt::LineMetrics& line = metrics[i]; + result[position + 0] = static_cast(line.hard_break); + result[position + 1] = line.ascent; + result[position + 2] = line.descent; + result[position + 3] = line.unscaled_ascent; + // We add then round to get the height. The + // definition of height here is different + // than the one in LibTxt. + result[position + 4] = round(line.ascent + line.descent); + result[position + 5] = line.width; + result[position + 6] = line.left; + result[position + 7] = line.baseline; + result[position + 8] = static_cast(line.line_number); } + return result; } diff --git a/lib/ui/text/paragraph.h b/lib/ui/text/paragraph.h index 7aea6079378e1..73c79d3c67de3 100644 --- a/lib/ui/text/paragraph.h +++ b/lib/ui/text/paragraph.h @@ -23,9 +23,10 @@ class Paragraph : public RefCountedDartWrappable { FML_FRIEND_MAKE_REF_COUNTED(Paragraph); public: - static fml::RefPtr Create( - std::unique_ptr paragraph) { - return fml::MakeRefCounted(std::move(paragraph)); + static void Create(Dart_Handle paragraph_handle, + std::unique_ptr txt_paragraph) { + auto paragraph = fml::MakeRefCounted(std::move(txt_paragraph)); + paragraph->AssociateWithDartWrapper(paragraph_handle); } ~Paragraph() override; @@ -42,15 +43,15 @@ class Paragraph : public RefCountedDartWrappable { void layout(double width); void paint(Canvas* canvas, double x, double y); - std::vector getRectsForRange(unsigned start, - unsigned end, - unsigned boxHeightStyle, - unsigned boxWidthStyle); - std::vector getRectsForPlaceholders(); + tonic::Float32List getRectsForRange(unsigned start, + unsigned end, + unsigned boxHeightStyle, + unsigned boxWidthStyle); + tonic::Float32List getRectsForPlaceholders(); Dart_Handle getPositionForOffset(double dx, double dy); Dart_Handle getWordBoundary(unsigned offset); Dart_Handle getLineBoundary(unsigned offset); - std::vector computeLineMetrics(); + tonic::Float64List computeLineMetrics(); size_t GetAllocationSize() override; diff --git a/lib/ui/text/paragraph_builder.cc b/lib/ui/text/paragraph_builder.cc index 51739fc2cf3fd..0b6eea6e0abfd 100644 --- a/lib/ui/text/paragraph_builder.cc +++ b/lib/ui/text/paragraph_builder.cc @@ -493,8 +493,8 @@ Dart_Handle ParagraphBuilder::addPlaceholder(double width, return Dart_Null(); } -fml::RefPtr ParagraphBuilder::build() { - return Paragraph::Create(m_paragraphBuilder->Build()); +void ParagraphBuilder::build(Dart_Handle paragraph_handle) { + Paragraph::Create(paragraph_handle, m_paragraphBuilder->Build()); } } // namespace flutter diff --git a/lib/ui/text/paragraph_builder.h b/lib/ui/text/paragraph_builder.h index eebc5e7f26868..4f4a00b5b94ab 100644 --- a/lib/ui/text/paragraph_builder.h +++ b/lib/ui/text/paragraph_builder.h @@ -69,7 +69,7 @@ class ParagraphBuilder : public RefCountedDartWrappable { double baseline_offset, unsigned baseline); - fml::RefPtr build(); + void build(Dart_Handle paragraph_handle); static void RegisterNatives(tonic::DartLibraryNatives* natives); diff --git a/lib/ui/text/text_box.cc b/lib/ui/text/text_box.cc deleted file mode 100644 index 99438cf4b87cb..0000000000000 --- a/lib/ui/text/text_box.cc +++ /dev/null @@ -1,45 +0,0 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "flutter/lib/ui/text/text_box.h" - -#include "flutter/fml/logging.h" -#include "third_party/tonic/dart_class_library.h" -#include "third_party/tonic/dart_state.h" -#include "third_party/tonic/logging/dart_error.h" - -using namespace flutter; - -namespace tonic { - -namespace { - -Dart_Handle GetTextBoxType() { - DartClassLibrary& class_library = DartState::Current()->class_library(); - Dart_Handle type = - Dart_HandleFromPersistent(class_library.GetClass("ui", "TextBox")); - FML_DCHECK(!LogIfError(type)); - return type; -} - -} // anonymous namespace - -Dart_Handle DartConverter::ToDart( - const flutter::TextBox& val) { - constexpr int argc = 5; - Dart_Handle argv[argc] = { - tonic::ToDart(val.rect.fLeft), - tonic::ToDart(val.rect.fTop), - tonic::ToDart(val.rect.fRight), - tonic::ToDart(val.rect.fBottom), - tonic::ToDart(static_cast(val.direction)), - }; - return Dart_New(GetTextBoxType(), tonic::ToDart("_"), argc, argv); -} - -Dart_Handle DartListFactory::NewList(intptr_t length) { - return Dart_NewListOfType(GetTextBoxType(), length); -} - -} // namespace tonic From 600ff75e3f75ef325b1c82bc11beb0788893dd56 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 27 Feb 2020 15:21:35 -0800 Subject: [PATCH 08/22] format --- lib/ui/text/paragraph.cc | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/ui/text/paragraph.cc b/lib/ui/text/paragraph.cc index 2776dfd7629af..c40c810263e7e 100644 --- a/lib/ui/text/paragraph.cc +++ b/lib/ui/text/paragraph.cc @@ -94,13 +94,15 @@ void Paragraph::paint(Canvas* canvas, double x, double y) { m_paragraph->Paint(sk_canvas, x, y); } -static tonic::Float32List EncodeTextBoxes(const std::vector& boxes) { +static tonic::Float32List EncodeTextBoxes( + const std::vector& boxes) { // Layout: // First value is the number of values. // Then there are boxes.size() groups of 5 which are LTRBD, where D is the // text direction index. auto count = boxes.size() * 5; - tonic::Float32List result(Dart_NewTypedData(Dart_TypedData_kFloat32, 1 + count)); + tonic::Float32List result( + Dart_NewTypedData(Dart_TypedData_kFloat32, 1 + count)); result[0] = count; for (unsigned long i = 0; i < boxes.size(); i++) { auto position = (i * 5) + 1; @@ -172,7 +174,8 @@ tonic::Float64List Paragraph::computeLineMetrics() { // Then there are boxes.size() groups of 9 which are the line metrics // properties auto count = metrics.size() * 9; - tonic::Float64List result(Dart_NewTypedData(Dart_TypedData_kFloat64, 1 + count)); + tonic::Float64List result( + Dart_NewTypedData(Dart_TypedData_kFloat64, 1 + count)); result[0] = count; for (unsigned long i = 0; i < metrics.size(); i++) { auto position = (i * 9) + 1; From 9af252e9e2c2203f90283ab24ac0899acb6c3c06 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 27 Feb 2020 15:21:54 -0800 Subject: [PATCH 09/22] format --- lib/ui/painting/multi_frame_codec.cc | 6 +++--- lib/ui/painting/picture.cc | 7 +++---- lib/ui/painting/single_frame_codec.cc | 7 ++++--- lib/ui/painting/single_frame_codec.h | 5 ++--- lib/ui/text/paragraph.cc | 9 ++++++--- 5 files changed, 18 insertions(+), 16 deletions(-) diff --git a/lib/ui/painting/multi_frame_codec.cc b/lib/ui/painting/multi_frame_codec.cc index d638c32706518..14e6ad6cb6473 100644 --- a/lib/ui/painting/multi_frame_codec.cc +++ b/lib/ui/painting/multi_frame_codec.cc @@ -179,9 +179,9 @@ Dart_Handle MultiFrameCodec::getNextFrame(Dart_Handle image_handle, this, trace_id, ui_task_runner = task_runners.GetUITaskRunner(), io_manager = dart_state->GetIOManager()]() mutable { GetNextFrameAndInvokeCallback( - image_handle, std::move(callback), - std::move(ui_task_runner), io_manager->GetResourceContext(), - io_manager->GetSkiaUnrefQueue(), trace_id); + image_handle, std::move(callback), std::move(ui_task_runner), + io_manager->GetResourceContext(), io_manager->GetSkiaUnrefQueue(), + trace_id); })); return Dart_Null(); diff --git a/lib/ui/painting/picture.cc b/lib/ui/painting/picture.cc index b0b808edd2f8a..76a8d70cd5fc9 100644 --- a/lib/ui/painting/picture.cc +++ b/lib/ui/painting/picture.cc @@ -48,8 +48,8 @@ Dart_Handle Picture::toImage(Dart_Handle image_handle, return tonic::ToDart("Picture is null"); } - return RasterizeToImage(image_handle, picture_.get(), width, - height, raw_image_callback); + return RasterizeToImage(image_handle, picture_.get(), width, height, + raw_image_callback); } void Picture::dispose() { @@ -92,8 +92,7 @@ Dart_Handle Picture::RasterizeToImage(Dart_Handle image_handle, auto picture_bounds = SkISize::Make(width, height); - auto ui_task = fml::MakeCopyable([image_handle, - image_callback, unref_queue]( + auto ui_task = fml::MakeCopyable([image_handle, image_callback, unref_queue]( sk_sp raster_image) mutable { auto dart_state = image_callback->dart_state().lock(); if (!dart_state) { diff --git a/lib/ui/painting/single_frame_codec.cc b/lib/ui/painting/single_frame_codec.cc index 00ad7bec055f5..4e964e6cb1c9c 100644 --- a/lib/ui/painting/single_frame_codec.cc +++ b/lib/ui/painting/single_frame_codec.cc @@ -29,7 +29,9 @@ Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle image_handle, } if (status_ == Status::kComplete) { - return tonic::ToDart("Dart callers are responsible for caching the frame callback information"); + return tonic::ToDart( + "Dart callers are responsible for caching the frame callback " + "information"); } // This has to be valid because this method is called from Dart. @@ -56,8 +58,7 @@ Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle image_handle, fml::RefPtr* raw_codec_ref = new fml::RefPtr(this); - decoder->Decode(descriptor_, [image_handle, - raw_codec_ref](auto image) { + decoder->Decode(descriptor_, [image_handle, raw_codec_ref](auto image) { std::unique_ptr> codec_ref(raw_codec_ref); fml::RefPtr codec(std::move(*codec_ref)); diff --git a/lib/ui/painting/single_frame_codec.h b/lib/ui/painting/single_frame_codec.h index dd4d9563a3206..58b8fc35014b3 100644 --- a/lib/ui/painting/single_frame_codec.h +++ b/lib/ui/painting/single_frame_codec.h @@ -6,8 +6,8 @@ #define FLUTTER_LIB_UI_PAINTING_SINGLE_FRAME_CODEC_H_ #include "flutter/fml/macros.h" -#include "flutter/lib/ui/painting/image.h" #include "flutter/lib/ui/painting/codec.h" +#include "flutter/lib/ui/painting/image.h" #include "flutter/lib/ui/painting/image_decoder.h" namespace flutter { @@ -30,8 +30,7 @@ class SingleFrameCodec : public Codec { int repetitionCount() const override; // |Codec| - Dart_Handle getNextFrame(Dart_Handle image_handle, - Dart_Handle args) override; + Dart_Handle getNextFrame(Dart_Handle image_handle, Dart_Handle args) override; // |DartWrappable| size_t GetAllocationSize() override; diff --git a/lib/ui/text/paragraph.cc b/lib/ui/text/paragraph.cc index 2776dfd7629af..c40c810263e7e 100644 --- a/lib/ui/text/paragraph.cc +++ b/lib/ui/text/paragraph.cc @@ -94,13 +94,15 @@ void Paragraph::paint(Canvas* canvas, double x, double y) { m_paragraph->Paint(sk_canvas, x, y); } -static tonic::Float32List EncodeTextBoxes(const std::vector& boxes) { +static tonic::Float32List EncodeTextBoxes( + const std::vector& boxes) { // Layout: // First value is the number of values. // Then there are boxes.size() groups of 5 which are LTRBD, where D is the // text direction index. auto count = boxes.size() * 5; - tonic::Float32List result(Dart_NewTypedData(Dart_TypedData_kFloat32, 1 + count)); + tonic::Float32List result( + Dart_NewTypedData(Dart_TypedData_kFloat32, 1 + count)); result[0] = count; for (unsigned long i = 0; i < boxes.size(); i++) { auto position = (i * 5) + 1; @@ -172,7 +174,8 @@ tonic::Float64List Paragraph::computeLineMetrics() { // Then there are boxes.size() groups of 9 which are the line metrics // properties auto count = metrics.size() * 9; - tonic::Float64List result(Dart_NewTypedData(Dart_TypedData_kFloat64, 1 + count)); + tonic::Float64List result( + Dart_NewTypedData(Dart_TypedData_kFloat64, 1 + count)); result[0] = count; for (unsigned long i = 0; i < metrics.size(); i++) { auto position = (i * 9) + 1; From 3f72ff29a3da970673022028d5c7e1af66896984 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 27 Feb 2020 15:42:31 -0800 Subject: [PATCH 10/22] The last bits --- third_party/tonic/dart_wrappable.cc | 30 +---------------------------- third_party/tonic/dart_wrappable.h | 19 +++++++++--------- 2 files changed, 11 insertions(+), 38 deletions(-) diff --git a/third_party/tonic/dart_wrappable.cc b/third_party/tonic/dart_wrappable.cc index f1b00a1fdef3e..089663dff2d14 100644 --- a/third_party/tonic/dart_wrappable.cc +++ b/third_party/tonic/dart_wrappable.cc @@ -16,34 +16,6 @@ DartWrappable::~DartWrappable() { TONIC_CHECK(!dart_wrapper_); } -// TODO(dnfield): Delete this. https://github.com/flutter/flutter/issues/50997 -Dart_Handle DartWrappable::CreateDartWrapper(DartState* dart_state) { - FML_DLOG(ERROR) << "NO TOUCHY"; - TONIC_DCHECK(!dart_wrapper_); - const DartWrapperInfo& info = GetDartWrapperInfo(); - - 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); - - 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); - - return wrapper; -} - void DartWrappable::AssociateWithDartWrapper(Dart_Handle wrapper) { TONIC_DCHECK(!dart_wrapper_); TONIC_CHECK(!LogIfError(wrapper)); @@ -76,7 +48,7 @@ void DartWrappable::FinalizeDartWrapper(void* isolate_callback_data, void* peer) { DartWrappable* wrappable = reinterpret_cast(peer); wrappable->dart_wrapper_ = nullptr; - wrappable->ReleaseDartWrappableReference(); // Balanced in CreateDartWrapper. + wrappable->ReleaseDartWrappableReference(); // Balanced in AssociateWithDartWrapper. } size_t DartWrappable::GetAllocationSize() { diff --git a/third_party/tonic/dart_wrappable.h b/third_party/tonic/dart_wrappable.h index 1d2e5e75bacb2..d4a6e1711aa15 100644 --- a/third_party/tonic/dart_wrappable.h +++ b/third_party/tonic/dart_wrappable.h @@ -43,10 +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 - // AssociateWithDartWrapper. - Dart_Handle CreateDartWrapper(DartState* dart_state); void AssociateWithDartWrapper(Dart_Handle wrappable); void ClearDartWrapper(); // Warning: Might delete this. Dart_WeakPersistentHandle dart_wrapper() const { return dart_wrapper_; } @@ -107,18 +103,23 @@ struct DartConverter< return Dart_Null(); if (Dart_WeakPersistentHandle wrapper = val->dart_wrapper()) return Dart_HandleFromWeakPersistent(wrapper); - return val->CreateDartWrapper(DartState::Current()); + + Log("Do not create non-primitive Dart objects from C++ code."); + TONIC_DCHECK(false); + return Dart_NewApiError("Invalid object conversion"); } static void SetReturnValue(Dart_NativeArguments args, DartWrappable* val, bool auto_scope = true) { - if (!val) + if (!val) { Dart_SetReturnValue(args, Dart_Null()); - else if (Dart_WeakPersistentHandle wrapper = val->dart_wrapper()) + } else if (Dart_WeakPersistentHandle wrapper = val->dart_wrapper()) { Dart_SetWeakHandleReturnValue(args, wrapper); - else - Dart_SetReturnValue(args, val->CreateDartWrapper(DartState::Current())); + } else { + Log("Do not create non-primitive Dart objects from C++ code."); + TONIC_DCHECK(false); + } } static T* FromDart(Dart_Handle handle) { From 58778cc32ed95afe60209703ba1d137c483d1b5d Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 27 Feb 2020 15:45:42 -0800 Subject: [PATCH 11/22] more dead code --- third_party/tonic/dart_state.cc | 7 ------- third_party/tonic/dart_state.h | 7 ------- 2 files changed, 14 deletions(-) diff --git a/third_party/tonic/dart_state.cc b/third_party/tonic/dart_state.cc index b711a22977788..870f058b9f2fc 100644 --- a/third_party/tonic/dart_state.cc +++ b/third_party/tonic/dart_state.cc @@ -22,7 +22,6 @@ 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)), @@ -37,12 +36,6 @@ void DartState::SetIsolate(Dart_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 845914937dcd0..d2c6e03cb7cc9 100644 --- a/third_party/tonic/dart_state.h +++ b/third_party/tonic/dart_state.h @@ -49,12 +49,6 @@ class DartState : public std::enable_shared_from_this { Dart_Isolate isolate() { return isolate_; } void SetIsolate(Dart_Isolate isolate); - // 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(); - } - DartClassLibrary& class_library() { return *class_library_; } DartMessageHandler& message_handler() { return *message_handler_; } FileLoader& file_loader() { return *file_loader_; } @@ -76,7 +70,6 @@ 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_; From 33cc2e53b7376aa5e9eb8b1baa2ae63fb1030422 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 27 Feb 2020 17:45:50 -0800 Subject: [PATCH 12/22] review, fixes, tests --- lib/ui/text.dart | 73 ++++++++++++++++++------ lib/ui/text/paragraph.cc | 43 ++++++-------- testing/dart/paragraph_builder_test.dart | 40 +++++++++++++ 3 files changed, 113 insertions(+), 43 deletions(-) diff --git a/lib/ui/text.dart b/lib/ui/text.dart index f33ffbcc3762c..01b4b8c754beb 100644 --- a/lib/ui/text.dart +++ b/lib/ui/text.dart @@ -1819,6 +1819,39 @@ class LineMetrics { /// /// For example, the first line is line 0, second line is line 1. final int lineNumber; + + @override + bool operator ==(Object other) { + if (other.runtimeType != runtimeType) { + return false; + } + return other is LineMetrics + && other.hardBreak == hardBreak + && other.ascent == ascent + && other.descent == descent + && other.unscaledAscent == unscaledAscent + && other.height == height + && other.width == width + && other.left == left + && other.baseline == baseline + && other.lineNumber == lineNumber; + } + + @override + int get hashCode => hashValues(hardBreak, ascent, descent, unscaledAscent, height, width, left, baseline, lineNumber); + + @override + String toString() { + return 'LineMetrics(hardBreak: $hardBreak, ' + 'ascent: $ascent, ' + 'descent: $descent, ' + 'unscaledAscent: $unscaledAscent, ' + 'height: $height, ' + 'width: $width, ' + 'left: $left, ' + 'baseline: $baseline, ' + 'lineNumber: $lineNumber)'; + } } /// A paragraph of text. @@ -1891,15 +1924,16 @@ class Paragraph extends NativeFieldWrapperClass2 { void _layout(double width) native 'Paragraph_layout'; List _decodeTextBoxes(Float32List encoded) { - final List boxes = List(encoded[0].toInt()); - for (int index = 0; index < boxes.length; index += 1) { - final int position = (index * 5) + 1; + final int count = encoded.length ~/ 5; + final List boxes = List(count); + int position = 0; + for (int index = 0; index < count; index += 1) { boxes[index] = TextBox.fromLTRBD( - encoded[position + 0], - encoded[position + 1], - encoded[position + 2], - encoded[position + 3], - TextDirection.values[encoded[position + 4].toInt()], + encoded[position++], + encoded[position++], + encoded[position++], + encoded[position++], + TextDirection.values[encoded[position++].toInt()], ); } return boxes; @@ -1983,19 +2017,20 @@ class Paragraph extends NativeFieldWrapperClass2 { /// to repeatedly call this. Instead, cache the results. List computeLineMetrics() { final Float64List encoded = _computeLineMetrics(); - final List metrics = List(encoded[0].toInt()); + final int count = encoded.length ~/ 9; + int position = 0; + final List metrics = List(count); for (int index = 0; index < metrics.length; index += 1) { - final int position = (index * 5) + 1; metrics[index] = LineMetrics( - hardBreak: encoded[position + 0] != 0, - ascent: encoded[position + 1], - descent: encoded[position + 2], - unscaledAscent: encoded[position + 3], - height: encoded[position + 4], - width: encoded[position + 5], - left: encoded[position + 6], - baseline: encoded[position + 7], - lineNumber: encoded[position + 8].toInt(), + hardBreak: encoded[position++] != 0, + ascent: encoded[position++], + descent: encoded[position++], + unscaledAscent: encoded[position++], + height: encoded[position++], + width: encoded[position++], + left: encoded[position++], + baseline: encoded[position++], + lineNumber: encoded[position++].toInt(), ); } return metrics; diff --git a/lib/ui/text/paragraph.cc b/lib/ui/text/paragraph.cc index c40c810263e7e..92800db2ebd67 100644 --- a/lib/ui/text/paragraph.cc +++ b/lib/ui/text/paragraph.cc @@ -100,18 +100,16 @@ static tonic::Float32List EncodeTextBoxes( // First value is the number of values. // Then there are boxes.size() groups of 5 which are LTRBD, where D is the // text direction index. - auto count = boxes.size() * 5; tonic::Float32List result( - Dart_NewTypedData(Dart_TypedData_kFloat32, 1 + count)); - result[0] = count; + Dart_NewTypedData(Dart_TypedData_kFloat32, boxes.size() * 5)); + unsigned long position = 0; for (unsigned long i = 0; i < boxes.size(); i++) { - auto position = (i * 5) + 1; const txt::Paragraph::TextBox& box = boxes[i]; - result[position + 0] = box.rect.fLeft; - result[position + 1] = box.rect.fTop; - result[position + 2] = box.rect.fRight; - result[position + 3] = box.rect.fBottom; - result[position + 4] = static_cast(box.direction); + result[position++] = box.rect.fLeft; + result[position++] = box.rect.fTop; + result[position++] = box.rect.fRight; + result[position++] = box.rect.fBottom; + result[position++] = static_cast(box.direction); } return result; } @@ -170,28 +168,25 @@ tonic::Float64List Paragraph::computeLineMetrics() { std::vector metrics = m_paragraph->GetLineMetrics(); // Layout: - // First value is the number of values. - // Then there are boxes.size() groups of 9 which are the line metrics + // boxes.size() groups of 9 which are the line metrics // properties - auto count = metrics.size() * 9; tonic::Float64List result( - Dart_NewTypedData(Dart_TypedData_kFloat64, 1 + count)); - result[0] = count; + Dart_NewTypedData(Dart_TypedData_kFloat64, metrics.size() * 9)); + unsigned long position = 0; for (unsigned long i = 0; i < metrics.size(); i++) { - auto position = (i * 9) + 1; const txt::LineMetrics& line = metrics[i]; - result[position + 0] = static_cast(line.hard_break); - result[position + 1] = line.ascent; - result[position + 2] = line.descent; - result[position + 3] = line.unscaled_ascent; + result[position++] = static_cast(line.hard_break); + result[position++] = line.ascent; + result[position++] = line.descent; + result[position++] = line.unscaled_ascent; // We add then round to get the height. The // definition of height here is different // than the one in LibTxt. - result[position + 4] = round(line.ascent + line.descent); - result[position + 5] = line.width; - result[position + 6] = line.left; - result[position + 7] = line.baseline; - result[position + 8] = static_cast(line.line_number); + result[position++] = round(line.ascent + line.descent); + result[position++] = line.width; + result[position++] = line.left; + result[position++] = line.baseline; + result[position++] = static_cast(line.line_number); } return result; diff --git a/testing/dart/paragraph_builder_test.dart b/testing/dart/paragraph_builder_test.dart index 81862dd9772bd..8cabfce9bf566 100644 --- a/testing/dart/paragraph_builder_test.dart +++ b/testing/dart/paragraph_builder_test.dart @@ -25,4 +25,44 @@ void main() { paragraphBuilder.build(); paragraphBuilder.pushStyle(TextStyle()); }); + + test('GetRectsForRange smoke test', () { + final ParagraphBuilder builder = ParagraphBuilder(ParagraphStyle()); + builder.addText('Hello'); + final Paragraph paragraph = builder.build(); + expect(paragraph, isNotNull); + + paragraph.layout(const ParagraphConstraints(width: 800.0)); + expect(paragraph.width, isNonZero); + expect(paragraph.height, isNonZero); + + final List boxes = paragraph.getBoxesForRange(0, 3); + expect(boxes.length, 1); + expect(boxes.first, const TextBox.fromLTRBD(0, 0, 42, 14, TextDirection.ltr)); + }); + + test('LineMetrics smoke test', () { + final ParagraphBuilder builder = ParagraphBuilder(ParagraphStyle()); + builder.addText('Hello'); + final Paragraph paragraph = builder.build(); + expect(paragraph, isNotNull); + + paragraph.layout(const ParagraphConstraints(width: 800.0)); + expect(paragraph.width, isNonZero); + expect(paragraph.height, isNonZero); + + final List metrics = paragraph.computeLineMetrics(); + expect(metrics.length, 1); + // The actual values here will vary by platform slightly. + const double epsillon = 0.00001; + expect(metrics.first.hardBreak, true); + expect(metrics.first.ascent, closeTo(11.200042724609375, epsillon)); + expect(metrics.first.descent, closeTo(2.799957275390625, epsillon)); + expect(metrics.first.unscaledAscent, closeTo(11.200042724609375, epsillon)); + expect(metrics.first.height, 14.0); + expect(metrics.first.width, 70.0); + expect(metrics.first.left, 0.0); + expect(metrics.first.baseline, closeTo(11.200042724609375, epsillon)); + expect(metrics.first.lineNumber, 0); + }); } From 8f8eb802d420ee52d865d2f9d40e864ee480016e Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 27 Feb 2020 21:17:17 -0800 Subject: [PATCH 13/22] more permissive test --- testing/dart/paragraph_builder_test.dart | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/testing/dart/paragraph_builder_test.dart b/testing/dart/paragraph_builder_test.dart index 8cabfce9bf566..2ec44109fe878 100644 --- a/testing/dart/paragraph_builder_test.dart +++ b/testing/dart/paragraph_builder_test.dart @@ -8,6 +8,9 @@ import 'dart:ui'; import 'package:test/test.dart'; void main() { + // The actual values for font measurements will vary by platform slightly. + const double epsillon = 0.0001; + test('Should be able to build and layout a paragraph', () { final ParagraphBuilder builder = ParagraphBuilder(ParagraphStyle()); builder.addText('Hello'); @@ -38,7 +41,11 @@ void main() { final List boxes = paragraph.getBoxesForRange(0, 3); expect(boxes.length, 1); - expect(boxes.first, const TextBox.fromLTRBD(0, 0, 42, 14, TextDirection.ltr)); + expect(boxes.first.left, 0); + expect(boxes.first.top, 0); + expect(boxes.first.right, closeTo(42, epsillon)); + expect(boxes.first.bottom, closeTo(14, epsillon)); + expect(boxes.first.direction, TextDirection.ltr); }); test('LineMetrics smoke test', () { @@ -53,8 +60,6 @@ void main() { final List metrics = paragraph.computeLineMetrics(); expect(metrics.length, 1); - // The actual values here will vary by platform slightly. - const double epsillon = 0.00001; expect(metrics.first.hardBreak, true); expect(metrics.first.ascent, closeTo(11.200042724609375, epsillon)); expect(metrics.first.descent, closeTo(2.799957275390625, epsillon)); From cbd75686c6acff6682f9b856e33a882ca1db5dcc Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 27 Feb 2020 21:19:16 -0800 Subject: [PATCH 14/22] more permissive test --- testing/dart/paragraph_builder_test.dart | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/testing/dart/paragraph_builder_test.dart b/testing/dart/paragraph_builder_test.dart index 8cabfce9bf566..2ec44109fe878 100644 --- a/testing/dart/paragraph_builder_test.dart +++ b/testing/dart/paragraph_builder_test.dart @@ -8,6 +8,9 @@ import 'dart:ui'; import 'package:test/test.dart'; void main() { + // The actual values for font measurements will vary by platform slightly. + const double epsillon = 0.0001; + test('Should be able to build and layout a paragraph', () { final ParagraphBuilder builder = ParagraphBuilder(ParagraphStyle()); builder.addText('Hello'); @@ -38,7 +41,11 @@ void main() { final List boxes = paragraph.getBoxesForRange(0, 3); expect(boxes.length, 1); - expect(boxes.first, const TextBox.fromLTRBD(0, 0, 42, 14, TextDirection.ltr)); + expect(boxes.first.left, 0); + expect(boxes.first.top, 0); + expect(boxes.first.right, closeTo(42, epsillon)); + expect(boxes.first.bottom, closeTo(14, epsillon)); + expect(boxes.first.direction, TextDirection.ltr); }); test('LineMetrics smoke test', () { @@ -53,8 +60,6 @@ void main() { final List metrics = paragraph.computeLineMetrics(); expect(metrics.length, 1); - // The actual values here will vary by platform slightly. - const double epsillon = 0.00001; expect(metrics.first.hardBreak, true); expect(metrics.first.ascent, closeTo(11.200042724609375, epsillon)); expect(metrics.first.descent, closeTo(2.799957275390625, epsillon)); From b648364b619bccb2bdf328473afa32517f4032ef Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 27 Feb 2020 23:20:45 -0800 Subject: [PATCH 15/22] epsillon for top --- testing/dart/paragraph_builder_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/dart/paragraph_builder_test.dart b/testing/dart/paragraph_builder_test.dart index 2ec44109fe878..cf8ccce0bf8b1 100644 --- a/testing/dart/paragraph_builder_test.dart +++ b/testing/dart/paragraph_builder_test.dart @@ -42,7 +42,7 @@ void main() { final List boxes = paragraph.getBoxesForRange(0, 3); expect(boxes.length, 1); expect(boxes.first.left, 0); - expect(boxes.first.top, 0); + expect(boxes.first.top, closeTo(0, epsillon)); expect(boxes.first.right, closeTo(42, epsillon)); expect(boxes.first.bottom, closeTo(14, epsillon)); expect(boxes.first.direction, TextDirection.ltr); From 016351fe3f582e714f4a3b5fb8791a55ceecae65 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Fri, 28 Feb 2020 00:09:10 -0800 Subject: [PATCH 16/22] semantics --- lib/ui/semantics.dart | 7 ++++++- lib/ui/semantics/semantics_update.cc | 8 +++++--- lib/ui/semantics/semantics_update.h | 3 ++- lib/ui/semantics/semantics_update_builder.cc | 4 ++-- lib/ui/semantics/semantics_update_builder.h | 2 +- 5 files changed, 16 insertions(+), 8 deletions(-) diff --git a/lib/ui/semantics.dart b/lib/ui/semantics.dart index 2a0cb13413367..508e001925866 100644 --- a/lib/ui/semantics.dart +++ b/lib/ui/semantics.dart @@ -805,7 +805,12 @@ class SemanticsUpdateBuilder extends NativeFieldWrapperClass2 { /// /// The returned object can be passed to [Window.updateSemantics] to actually /// update the semantics retained by the system. - SemanticsUpdate build() native 'SemanticsUpdateBuilder_build'; + SemanticsUpdate build() { + final SemanticsUpdate semanticsUpdate = SemanticsUpdate._(); + _build(semanticsUpdate); + return semanticsUpdate; + } + void _build(SemanticsUpdate outSemanticsUpdate) native 'SemanticsUpdateBuilder_build'; } /// An opaque object representing a batch of semantics updates. diff --git a/lib/ui/semantics/semantics_update.cc b/lib/ui/semantics/semantics_update.cc index e559af58aec61..26e531a0f3fd5 100644 --- a/lib/ui/semantics/semantics_update.cc +++ b/lib/ui/semantics/semantics_update.cc @@ -20,11 +20,13 @@ IMPLEMENT_WRAPPERTYPEINFO(ui, SemanticsUpdate); DART_BIND_ALL(SemanticsUpdate, FOR_EACH_BINDING) -fml::RefPtr SemanticsUpdate::create( +void SemanticsUpdate::create( + Dart_Handle semantics_update_handle, SemanticsNodeUpdates nodes, CustomAccessibilityActionUpdates actions) { - return fml::MakeRefCounted(std::move(nodes), - std::move(actions)); + auto semantics_update = fml::MakeRefCounted( + std::move(nodes), std::move(actions)); + semantics_update->AssociateWithDartWrapper(semantics_update_handle); } SemanticsUpdate::SemanticsUpdate(SemanticsNodeUpdates nodes, diff --git a/lib/ui/semantics/semantics_update.h b/lib/ui/semantics/semantics_update.h index 35ecbfbb9d510..1ec8b734c446e 100644 --- a/lib/ui/semantics/semantics_update.h +++ b/lib/ui/semantics/semantics_update.h @@ -21,7 +21,8 @@ class SemanticsUpdate : public RefCountedDartWrappable { public: ~SemanticsUpdate() override; - static fml::RefPtr create( + static void create( + Dart_Handle semantics_update_handle, SemanticsNodeUpdates nodes, CustomAccessibilityActionUpdates actions); diff --git a/lib/ui/semantics/semantics_update_builder.cc b/lib/ui/semantics/semantics_update_builder.cc index 7048f6a45f44c..e4a9be37b81e5 100644 --- a/lib/ui/semantics/semantics_update_builder.cc +++ b/lib/ui/semantics/semantics_update_builder.cc @@ -121,8 +121,8 @@ void SemanticsUpdateBuilder::updateCustomAction(int id, actions_[id] = action; } -fml::RefPtr SemanticsUpdateBuilder::build() { - return SemanticsUpdate::create(std::move(nodes_), std::move(actions_)); +void SemanticsUpdateBuilder::build(Dart_Handle semantics_update_handle) { + SemanticsUpdate::create(semantics_update_handle, std::move(nodes_), std::move(actions_)); } } // namespace flutter diff --git a/lib/ui/semantics/semantics_update_builder.h b/lib/ui/semantics/semantics_update_builder.h index bbf595e631858..a183e1384b637 100644 --- a/lib/ui/semantics/semantics_update_builder.h +++ b/lib/ui/semantics/semantics_update_builder.h @@ -58,7 +58,7 @@ class SemanticsUpdateBuilder std::string hint, int overrideId); - fml::RefPtr build(); + void build(Dart_Handle semantics_update_handle); static void RegisterNatives(tonic::DartLibraryNatives* natives); From 7709586fc177b8e8764d083090ff994f22da9b54 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Fri, 28 Feb 2020 00:12:58 -0800 Subject: [PATCH 17/22] format again --- lib/ui/semantics/semantics_update.cc | 7 +++---- lib/ui/semantics/semantics_update.h | 7 +++---- lib/ui/semantics/semantics_update_builder.cc | 3 ++- third_party/tonic/dart_wrappable.cc | 3 ++- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/ui/semantics/semantics_update.cc b/lib/ui/semantics/semantics_update.cc index 26e531a0f3fd5..c4ef22e34f7be 100644 --- a/lib/ui/semantics/semantics_update.cc +++ b/lib/ui/semantics/semantics_update.cc @@ -20,10 +20,9 @@ IMPLEMENT_WRAPPERTYPEINFO(ui, SemanticsUpdate); DART_BIND_ALL(SemanticsUpdate, FOR_EACH_BINDING) -void SemanticsUpdate::create( - Dart_Handle semantics_update_handle, - SemanticsNodeUpdates nodes, - CustomAccessibilityActionUpdates actions) { +void SemanticsUpdate::create(Dart_Handle semantics_update_handle, + SemanticsNodeUpdates nodes, + CustomAccessibilityActionUpdates actions) { auto semantics_update = fml::MakeRefCounted( std::move(nodes), std::move(actions)); semantics_update->AssociateWithDartWrapper(semantics_update_handle); diff --git a/lib/ui/semantics/semantics_update.h b/lib/ui/semantics/semantics_update.h index 1ec8b734c446e..c3411bb43bfaa 100644 --- a/lib/ui/semantics/semantics_update.h +++ b/lib/ui/semantics/semantics_update.h @@ -21,10 +21,9 @@ class SemanticsUpdate : public RefCountedDartWrappable { public: ~SemanticsUpdate() override; - static void create( - Dart_Handle semantics_update_handle, - SemanticsNodeUpdates nodes, - CustomAccessibilityActionUpdates actions); + static void create(Dart_Handle semantics_update_handle, + SemanticsNodeUpdates nodes, + CustomAccessibilityActionUpdates actions); SemanticsNodeUpdates takeNodes(); diff --git a/lib/ui/semantics/semantics_update_builder.cc b/lib/ui/semantics/semantics_update_builder.cc index e4a9be37b81e5..d3fc40deab19c 100644 --- a/lib/ui/semantics/semantics_update_builder.cc +++ b/lib/ui/semantics/semantics_update_builder.cc @@ -122,7 +122,8 @@ void SemanticsUpdateBuilder::updateCustomAction(int id, } void SemanticsUpdateBuilder::build(Dart_Handle semantics_update_handle) { - SemanticsUpdate::create(semantics_update_handle, std::move(nodes_), std::move(actions_)); + SemanticsUpdate::create(semantics_update_handle, std::move(nodes_), + std::move(actions_)); } } // namespace flutter diff --git a/third_party/tonic/dart_wrappable.cc b/third_party/tonic/dart_wrappable.cc index 089663dff2d14..b964c8e8f412a 100644 --- a/third_party/tonic/dart_wrappable.cc +++ b/third_party/tonic/dart_wrappable.cc @@ -48,7 +48,8 @@ void DartWrappable::FinalizeDartWrapper(void* isolate_callback_data, void* peer) { DartWrappable* wrappable = reinterpret_cast(peer); wrappable->dart_wrapper_ = nullptr; - wrappable->ReleaseDartWrappableReference(); // Balanced in AssociateWithDartWrapper. + wrappable->ReleaseDartWrappableReference(); // Balanced in + // AssociateWithDartWrapper. } size_t DartWrappable::GetAllocationSize() { From e001c3582dae917a407f0bc1d8181c999397d673 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Fri, 28 Feb 2020 07:41:12 -0800 Subject: [PATCH 18/22] more foramt --- third_party/tonic/BUILD.gn | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/third_party/tonic/BUILD.gn b/third_party/tonic/BUILD.gn index 06a54a3811d5a..c7c4266359c56 100644 --- a/third_party/tonic/BUILD.gn +++ b/third_party/tonic/BUILD.gn @@ -41,8 +41,8 @@ source_set("tonic") { "logging", "scopes", "typed_data", + "//flutter/fml", "//third_party/dart/runtime:dart_api", - "//flutter/fml" ] public_configs = [ ":config" ] From 0252872a06e1d005790986398a3726308921e2af Mon Sep 17 00:00:00 2001 From: Dan Field Date: Fri, 28 Feb 2020 11:38:20 -0800 Subject: [PATCH 19/22] retain canvas image --- lib/ui/painting.dart | 5 +++-- lib/ui/painting/image.cc | 4 +++- lib/ui/painting/image.h | 1 + lib/ui/painting/multi_frame_codec.cc | 17 +++++++++-------- lib/ui/painting/multi_frame_codec.h | 3 ++- lib/ui/painting/single_frame_codec.cc | 7 ++++--- lib/ui/painting/single_frame_codec.h | 4 +++- 7 files changed, 25 insertions(+), 16 deletions(-) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 94aefcb5f8932..37b7dc22b9ae5 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -1690,13 +1690,14 @@ class Codec extends NativeFieldWrapperClass2 { return _cachedFrame; } final Image image = Image._(); + print('About to call _futureize with image'); final int durationMilliseconds = await _futurize((_Callback callback) => _getNextFrame(image, callback)); - + print(image); return _cachedFrame = FrameInfo._(durationMilliseconds, image); } /// Returns an error message on failure, null on success. - String _getNextFrame(Image image, _Callback callback) native 'Codec_getNextFrame'; + String _getNextFrame(Image outImage, _Callback callback) native 'Codec_getNextFrame'; /// Release the resources used by this object. The object is no longer usable /// after this method is called. diff --git a/lib/ui/painting/image.cc b/lib/ui/painting/image.cc index 8ee65790924bf..69f430e885f32 100644 --- a/lib/ui/painting/image.cc +++ b/lib/ui/painting/image.cc @@ -30,7 +30,9 @@ void CanvasImage::RegisterNatives(tonic::DartLibraryNatives* natives) { CanvasImage::CanvasImage() = default; -CanvasImage::~CanvasImage() = default; +CanvasImage::~CanvasImage() { + FML_DLOG(ERROR) << "Image dtor"; +} Dart_Handle CanvasImage::toByteData(int format, Dart_Handle callback) { return EncodeImage(this, format, callback); diff --git a/lib/ui/painting/image.h b/lib/ui/painting/image.h index 43a00517421b8..98df0133e8471 100644 --- a/lib/ui/painting/image.h +++ b/lib/ui/painting/image.h @@ -23,6 +23,7 @@ class CanvasImage final : public RefCountedDartWrappable { public: ~CanvasImage() override; static fml::RefPtr Create(Dart_Handle dart_handle) { + FML_DLOG(ERROR) << "Image Create"; auto image = fml::MakeRefCounted(); image->AssociateWithDartWrapper(dart_handle); return image; diff --git a/lib/ui/painting/multi_frame_codec.cc b/lib/ui/painting/multi_frame_codec.cc index 14e6ad6cb6473..c38ecb48930e3 100644 --- a/lib/ui/painting/multi_frame_codec.cc +++ b/lib/ui/painting/multi_frame_codec.cc @@ -21,7 +21,7 @@ MultiFrameCodec::~MultiFrameCodec() = default; static void InvokeNextFrameCallback( sk_sp skImage, - Dart_Handle image_handle, + fml::RefPtr canvas_image, std::unique_ptr callback, fml::RefPtr unref_queue, SkCodec::FrameInfo skFrameInfo, @@ -35,8 +35,7 @@ static void InvokeNextFrameCallback( tonic::DartState::Scope scope(dart_state); if (skImage) { - fml::RefPtr image = CanvasImage::Create(image_handle); - image->set_image({skImage, std::move(unref_queue)}); + canvas_image->set_image({skImage, std::move(unref_queue)}); tonic::DartInvoke(callback->value(), {tonic::ToDart(skFrameInfo.fDuration)}); } else { @@ -136,7 +135,7 @@ sk_sp MultiFrameCodec::GetNextFrameImage( } void MultiFrameCodec::GetNextFrameAndInvokeCallback( - Dart_Handle image_handle, + fml::RefPtr canvas_image, std::unique_ptr callback, fml::RefPtr ui_task_runner, fml::WeakPtr resourceContext, @@ -151,9 +150,10 @@ void MultiFrameCodec::GetNextFrameAndInvokeCallback( ui_task_runner->PostTask(fml::MakeCopyable( [skImage = std::move(skImage), callback = std::move(callback), - image_handle, unref_queue = std::move(unref_queue), + canvas_image = std::move(canvas_image), + unref_queue = std::move(unref_queue), skFrameInfo = std::move(skFrameInfo), trace_id]() mutable { - InvokeNextFrameCallback(std::move(skImage), image_handle, + InvokeNextFrameCallback(std::move(skImage), std::move(canvas_image), std::move(callback), std::move(unref_queue), std::move(skFrameInfo), trace_id); })); @@ -168,18 +168,19 @@ Dart_Handle MultiFrameCodec::getNextFrame(Dart_Handle image_handle, return tonic::ToDart("Callback must be a function"); } + auto canvas_image = CanvasImage::Create(image_handle); auto* dart_state = UIDartState::Current(); const auto& task_runners = dart_state->GetTaskRunners(); task_runners.GetIOTaskRunner()->PostTask(fml::MakeCopyable( - [image_handle, + [canvas_image = std::move(canvas_image), callback = std::make_unique( tonic::DartState::Current(), callback_handle), this, trace_id, ui_task_runner = task_runners.GetUITaskRunner(), io_manager = dart_state->GetIOManager()]() mutable { GetNextFrameAndInvokeCallback( - image_handle, std::move(callback), std::move(ui_task_runner), + canvas_image, std::move(callback), std::move(ui_task_runner), io_manager->GetResourceContext(), io_manager->GetSkiaUnrefQueue(), trace_id); })); diff --git a/lib/ui/painting/multi_frame_codec.h b/lib/ui/painting/multi_frame_codec.h index bee27844aabc5..a75ce597d3a9e 100644 --- a/lib/ui/painting/multi_frame_codec.h +++ b/lib/ui/painting/multi_frame_codec.h @@ -17,6 +17,7 @@ class MultiFrameCodec : public Codec { public: static fml::RefPtr Create(Dart_Handle codec_handle, std::unique_ptr codec) { + FML_DLOG(ERROR) << "MFC Create"; auto multi_frame_codec = fml::MakeRefCounted(std::move(codec)); multi_frame_codec->AssociateWithDartWrapper(codec_handle); @@ -48,7 +49,7 @@ class MultiFrameCodec : public Codec { sk_sp GetNextFrameImage(fml::WeakPtr resourceContext); void GetNextFrameAndInvokeCallback( - Dart_Handle image_handle, + fml::RefPtr canvas_image, std::unique_ptr callback, fml::RefPtr ui_task_runner, fml::WeakPtr resourceContext, diff --git a/lib/ui/painting/single_frame_codec.cc b/lib/ui/painting/single_frame_codec.cc index 4e964e6cb1c9c..da669466bc92a 100644 --- a/lib/ui/painting/single_frame_codec.cc +++ b/lib/ui/painting/single_frame_codec.cc @@ -34,6 +34,8 @@ Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle image_handle, "information"); } + auto canvas_image = CanvasImage::Create(image_handle); + // This has to be valid because this method is called from Dart. auto dart_state = UIDartState::Current(); @@ -58,7 +60,7 @@ Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle image_handle, fml::RefPtr* raw_codec_ref = new fml::RefPtr(this); - decoder->Decode(descriptor_, [image_handle, raw_codec_ref](auto image) { + decoder->Decode(descriptor_, [canvas_image, raw_codec_ref](auto image) { std::unique_ptr> codec_ref(raw_codec_ref); fml::RefPtr codec(std::move(*codec_ref)); @@ -73,7 +75,7 @@ Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle image_handle, tonic::DartState::Scope scope(state.get()); if (image.get()) { - auto canvas_image = CanvasImage::Create(image_handle); + FML_DLOG(ERROR) << "Here?"; canvas_image->set_image(std::move(image)); codec->cached_frame_image_size_ = canvas_image->GetAllocationSize(); } @@ -94,7 +96,6 @@ Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle image_handle, descriptor_.data.reset(); status_ = Status::kInProgress; - return Dart_Null(); } diff --git a/lib/ui/painting/single_frame_codec.h b/lib/ui/painting/single_frame_codec.h index 58b8fc35014b3..53a63f54cb53c 100644 --- a/lib/ui/painting/single_frame_codec.h +++ b/lib/ui/painting/single_frame_codec.h @@ -17,6 +17,7 @@ class SingleFrameCodec : public Codec { static fml::RefPtr Create( Dart_Handle codec_handle, ImageDecoder::ImageDescriptor descriptor) { + FML_DLOG(ERROR) << "SFC Create"; auto codec = fml::MakeRefCounted(std::move(descriptor)); codec->AssociateWithDartWrapper(codec_handle); return codec; @@ -30,7 +31,8 @@ class SingleFrameCodec : public Codec { int repetitionCount() const override; // |Codec| - Dart_Handle getNextFrame(Dart_Handle image_handle, Dart_Handle args) override; + Dart_Handle getNextFrame(Dart_Handle image_handle, + Dart_Handle callback_handle) override; // |DartWrappable| size_t GetAllocationSize() override; From e519e56810c58f7dc162752651498be73dd3c036 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Fri, 28 Feb 2020 15:36:34 -0800 Subject: [PATCH 20/22] cleanup, retain handles, fix toImage --- lib/ui/compositing.dart | 7 ++++--- lib/ui/painting.dart | 11 ++++++----- lib/ui/painting/image.cc | 4 +--- lib/ui/painting/image.h | 1 - lib/ui/painting/picture.cc | 11 ++++++----- lib/ui/painting/single_frame_codec.cc | 1 - 6 files changed, 17 insertions(+), 18 deletions(-) diff --git a/lib/ui/compositing.dart b/lib/ui/compositing.dart index 1f238a1d006c7..e86347d9b5389 100644 --- a/lib/ui/compositing.dart +++ b/lib/ui/compositing.dart @@ -22,15 +22,16 @@ class Scene extends NativeFieldWrapperClass2 { /// Creates a raster image representation of the current state of the scene. /// This is a slow operation that is performed on a background thread. - Future toImage(int width, int height) { + Future toImage(int width, int height) async { if (width <= 0 || height <= 0) { throw Exception('Invalid image dimensions.'); } final Image image = Image._(); - return _futurize((_Callback callback) => _toImage(image, width, height, callback)); + await _futurize((_Callback callback) => _toImage(image, width, height, callback)); + return image; } - String _toImage(Image image, int width, int height, _Callback callback) native 'Scene_toImage'; + String _toImage(Image outImage, int width, int height, _Callback callback) native 'Scene_toImage'; /// Releases the resources used by this scene. /// diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 8331b748fef65..648e5d36ef778 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -1747,7 +1747,7 @@ Future instantiateImageCodec(Uint8List list, { /// If both are equal to [_kDoNotResizeDimension], then the image maintains its real size. /// /// Returns an error message if the instantiation has failed, null otherwise. -String _instantiateImageCodec(Codec codec, Uint8List list, _Callback callback, _ImageInfo imageInfo, int targetWidth, int targetHeight) +String _instantiateImageCodec(Codec outCodec, Uint8List list, _Callback callback, _ImageInfo imageInfo, int targetWidth, int targetHeight) native 'instantiateImageCodec'; /// Loads a single image frame from a byte array into an [Image] object. @@ -4138,16 +4138,17 @@ class Picture extends NativeFieldWrapperClass2 { /// /// Although the image is returned synchronously, the picture is actually /// rasterized the first time the image is drawn and then cached. - Future toImage(int width, int height) { + Future toImage(int width, int height) async { if (width <= 0 || height <= 0) throw Exception('Invalid image dimensions.'); final Image image = Image._(); - return _futurize( - (_Callback callback) => _toImage(image, width, height, callback) + await _futurize( + (_Callback callback) => _toImage(image, width, height, callback) ); + return image; } - String _toImage(Image image, int width, int height, _Callback callback) native 'Picture_toImage'; + String _toImage(Image outImage, int width, int height, _Callback callback) native 'Picture_toImage'; /// Release the resources used by this object. The object is no longer usable /// after this method is called. diff --git a/lib/ui/painting/image.cc b/lib/ui/painting/image.cc index 69f430e885f32..8ee65790924bf 100644 --- a/lib/ui/painting/image.cc +++ b/lib/ui/painting/image.cc @@ -30,9 +30,7 @@ void CanvasImage::RegisterNatives(tonic::DartLibraryNatives* natives) { CanvasImage::CanvasImage() = default; -CanvasImage::~CanvasImage() { - FML_DLOG(ERROR) << "Image dtor"; -} +CanvasImage::~CanvasImage() = default; Dart_Handle CanvasImage::toByteData(int format, Dart_Handle callback) { return EncodeImage(this, format, callback); diff --git a/lib/ui/painting/image.h b/lib/ui/painting/image.h index 98df0133e8471..43a00517421b8 100644 --- a/lib/ui/painting/image.h +++ b/lib/ui/painting/image.h @@ -23,7 +23,6 @@ class CanvasImage final : public RefCountedDartWrappable { public: ~CanvasImage() override; static fml::RefPtr Create(Dart_Handle dart_handle) { - FML_DLOG(ERROR) << "Image Create"; auto image = fml::MakeRefCounted(); image->AssociateWithDartWrapper(dart_handle); return image; diff --git a/lib/ui/painting/picture.cc b/lib/ui/painting/picture.cc index 76a8d70cd5fc9..34d765221e0a7 100644 --- a/lib/ui/painting/picture.cc +++ b/lib/ui/painting/picture.cc @@ -77,6 +77,8 @@ Dart_Handle Picture::RasterizeToImage(Dart_Handle image_handle, return tonic::ToDart("Image dimensions for scene were invalid."); } + auto canvas_image = CanvasImage::Create(image_handle); + auto* dart_state = UIDartState::Current(); tonic::DartPersistentValue* image_callback = new tonic::DartPersistentValue(dart_state, raw_image_callback); @@ -92,7 +94,7 @@ Dart_Handle Picture::RasterizeToImage(Dart_Handle image_handle, auto picture_bounds = SkISize::Make(width, height); - auto ui_task = fml::MakeCopyable([image_handle, image_callback, unref_queue]( + auto ui_task = fml::MakeCopyable([canvas_image, image_callback, unref_queue]( sk_sp raster_image) mutable { auto dart_state = image_callback->dart_state().lock(); if (!dart_state) { @@ -103,15 +105,14 @@ Dart_Handle Picture::RasterizeToImage(Dart_Handle image_handle, if (!raster_image) { tonic::DartInvoke(image_callback->Get(), {Dart_Null()}); + delete image_callback; return; } - auto dart_image = CanvasImage::Create(image_handle); - dart_image->set_image({std::move(raster_image), std::move(unref_queue)}); - auto* raw_dart_image = tonic::ToDart(std::move(dart_image)); + canvas_image->set_image({std::move(raster_image), std::move(unref_queue)}); // All done! - tonic::DartInvoke(image_callback->Get(), {raw_dart_image}); + tonic::DartInvoke(image_callback->Get(), {Dart_True()}); // image_callback is associated with the Dart isolate and must be deleted // on the UI thread diff --git a/lib/ui/painting/single_frame_codec.cc b/lib/ui/painting/single_frame_codec.cc index da669466bc92a..51eb870001a3e 100644 --- a/lib/ui/painting/single_frame_codec.cc +++ b/lib/ui/painting/single_frame_codec.cc @@ -75,7 +75,6 @@ Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle image_handle, tonic::DartState::Scope scope(state.get()); if (image.get()) { - FML_DLOG(ERROR) << "Here?"; canvas_image->set_image(std::move(image)); codec->cached_frame_image_size_ = canvas_image->GetAllocationSize(); } From 9619a7822ea7279c1e6636f00580e2185f7e51a3 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Mon, 2 Mar 2020 14:35:26 -0800 Subject: [PATCH 21/22] review --- lib/ui/painting.dart | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 648e5d36ef778..24d179365da60 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -1689,14 +1689,12 @@ class Codec extends NativeFieldWrapperClass2 { /// /// The returned future can complete with an error if the decoding has failed. Future getNextFrame() async { - if (_cachedFrame != null && frameCount == 1) { - return _cachedFrame; + if (_cachedFrame == null || frameCount != 1) { + final Image image = Image._(); + final int durationMilliseconds = await _futurize((_Callback callback) => _getNextFrame(image, callback)); + _cachedFrame = FrameInfo._(durationMilliseconds, image); } - final Image image = Image._(); - print('About to call _futureize with image'); - final int durationMilliseconds = await _futurize((_Callback callback) => _getNextFrame(image, callback)); - print(image); - return _cachedFrame = FrameInfo._(durationMilliseconds, image); + return _cachedFrame; } /// Returns an error message on failure, null on success. From cd55995d886623cf45da751b79c66e5df24d6676 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Mon, 2 Mar 2020 14:38:00 -0800 Subject: [PATCH 22/22] remove debug logging, unneeded includes --- lib/ui/painting/multi_frame_codec.h | 1 - lib/ui/painting/single_frame_codec.h | 1 - third_party/tonic/BUILD.gn | 1 - third_party/tonic/dart_wrappable.cc | 1 - 4 files changed, 4 deletions(-) diff --git a/lib/ui/painting/multi_frame_codec.h b/lib/ui/painting/multi_frame_codec.h index a75ce597d3a9e..1abcded241469 100644 --- a/lib/ui/painting/multi_frame_codec.h +++ b/lib/ui/painting/multi_frame_codec.h @@ -17,7 +17,6 @@ class MultiFrameCodec : public Codec { public: static fml::RefPtr Create(Dart_Handle codec_handle, std::unique_ptr codec) { - FML_DLOG(ERROR) << "MFC Create"; auto multi_frame_codec = fml::MakeRefCounted(std::move(codec)); multi_frame_codec->AssociateWithDartWrapper(codec_handle); diff --git a/lib/ui/painting/single_frame_codec.h b/lib/ui/painting/single_frame_codec.h index 53a63f54cb53c..48639deb10a90 100644 --- a/lib/ui/painting/single_frame_codec.h +++ b/lib/ui/painting/single_frame_codec.h @@ -17,7 +17,6 @@ class SingleFrameCodec : public Codec { static fml::RefPtr Create( Dart_Handle codec_handle, ImageDecoder::ImageDescriptor descriptor) { - FML_DLOG(ERROR) << "SFC Create"; auto codec = fml::MakeRefCounted(std::move(descriptor)); codec->AssociateWithDartWrapper(codec_handle); return codec; diff --git a/third_party/tonic/BUILD.gn b/third_party/tonic/BUILD.gn index c7c4266359c56..eb0136d54bd59 100644 --- a/third_party/tonic/BUILD.gn +++ b/third_party/tonic/BUILD.gn @@ -41,7 +41,6 @@ source_set("tonic") { "logging", "scopes", "typed_data", - "//flutter/fml", "//third_party/dart/runtime:dart_api", ] diff --git a/third_party/tonic/dart_wrappable.cc b/third_party/tonic/dart_wrappable.cc index b964c8e8f412a..42c8fff2805b2 100644 --- a/third_party/tonic/dart_wrappable.cc +++ b/third_party/tonic/dart_wrappable.cc @@ -4,7 +4,6 @@ #include "tonic/dart_wrappable.h" -#include "fml/logging.h" #include "tonic/dart_class_library.h" #include "tonic/dart_state.h" #include "tonic/dart_wrapper_info.h"