diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 87c5731531bc8..9e05118e041a0 100755 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -328,8 +328,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 841a6afa9c766..6ce9238b2738d 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/compositing.dart b/lib/ui/compositing.dart index 39b0d63fc2b36..a6bd03b894906 100644 --- a/lib/ui/compositing.dart +++ b/lib/ui/compositing.dart @@ -23,14 +23,21 @@ 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. + /// + /// Callers must dispose the [Image] when they are done with it. If the result + /// will be shared with other methods or classes, [Image.clone] should be used + /// and each handle created must be disposed. Future toImage(int width, int height) { if (width <= 0 || height <= 0) { throw Exception('Invalid image dimensions.'); } - return _futurize((_Callback callback) => _toImage(width, height, callback)); + return _futurize((_Callback callback) => _toImage(width, height, (_Image image) { + callback(Image._(image)); + }), + ); } - String _toImage(int width, int height, _Callback callback) native 'Scene_toImage'; + String _toImage(int width, int height, _Callback<_Image> callback) native 'Scene_toImage'; /// Releases the resources used by this scene. /// diff --git a/lib/ui/dart_ui.cc b/lib/ui/dart_ui.cc index f9f89d6f22d67..c22b26b17d3f6 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_descriptor.h" @@ -70,7 +69,6 @@ void DartUI::InitForGlobal() { DartRuntimeHooks::RegisterNatives(g_natives); EngineLayer::RegisterNatives(g_natives); FontCollection::RegisterNatives(g_natives); - FrameInfo::RegisterNatives(g_natives); ImageDescriptor::RegisterNatives(g_natives); ImageFilter::RegisterNatives(g_natives); ImageShader::RegisterNatives(g_natives); diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index ed0ec14ccb8d1..993029a1e6998 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -1566,27 +1566,71 @@ enum PixelFormat { /// To draw an [Image], use one of the methods on the [Canvas] class, such as /// [Canvas.drawImage]. /// +/// A class or method that receives an image object must call [dispose] on the +/// handle when it is no longer needed. To create a shareable reference to the +/// underlying image, call [clone]. The method or object that recieves +/// the new instance will then be responsible for disposing it, and the +/// underlying image itself will be disposed when all outstanding handles are +/// disposed. +/// +/// If `dart:ui` passes an `Image` object and the recipient wishes to share +/// that handle with other callers, [clone] must be called _before_ [dispose]. +/// A handle that has been disposed cannot create new handles anymore. +/// /// See also: /// /// * [Image](https://api.flutter.dev/flutter/widgets/Image-class.html), the class in the [widgets] library. /// * [ImageDescriptor], which allows reading information about the image and /// creating a codec to decode it. /// * [instantiateImageCodec], a utility method that wraps [ImageDescriptor]. -/// -@pragma('vm:entry-point') -class Image extends NativeFieldWrapperClass2 { - // This class is created by the engine, and should not be instantiated - // or extended directly. - // - // To obtain an [Image] object, use [instantiateImageCodec]. +class Image { + Image._(this._image) { + assert(() { + _debugStack = StackTrace.current; + return true; + }()); + _image._handles.add(this); + } + + // C++ unit tests access this. @pragma('vm:entry-point') - Image._(); + final _Image _image; + + StackTrace? _debugStack; /// The number of image pixels along the image's horizontal axis. - int get width native 'Image_width'; + int get width { + assert(!_disposed && !_image._disposed); + return _image.width; + } /// The number of image pixels along the image's vertical axis. - int get height native 'Image_height'; + int get height { + assert(!_disposed && !_image._disposed); + return _image.height; + } + + bool _disposed = false; + /// Release this handle's claim on the underlying Image. This handle is no + /// longer usable after this method is called. + /// + /// Once all outstanding handles have been disposed, the underlying image will + /// be disposed as well. + /// + /// In debug mode, [debugGetOpenHandleStackTraces] will return a list of + /// [StackTrace] objects from all open handles' creation points. This is + /// useful when trying to determine what parts of the program are keeping an + /// image resident in memory. + void dispose() { + assert(!_disposed && !_image._disposed); + assert(_image._handles.contains(this)); + _disposed = true; + final bool removed = _image._handles.remove(this); + assert(removed); + if (_image._handles.isEmpty) { + _image.dispose(); + } + } /// Converts the [Image] object into a byte array. /// @@ -1595,6 +1639,125 @@ class Image extends NativeFieldWrapperClass2 { /// /// Returns a future that completes with the binary image data or an error /// if encoding fails. + Future toByteData({ImageByteFormat format = ImageByteFormat.rawRgba}) { + assert(!_disposed && !_image._disposed); + return _image.toByteData(format: format); + } + + /// If asserts are enabled, returns the [StackTrace]s of each open handle from + /// [clone], in creation order. + /// + /// If asserts are disabled, this method always returns null. + List? debugGetOpenHandleStackTraces() { + List? stacks; + assert(() { + stacks = _image._handles.map((Image handle) => handle._debugStack!).toList(); + return true; + }()); + return stacks; + } + + /// Creates a disposable handle to this image. + /// + /// Holders of an [Image] must dispose of the image when they no longer need + /// to access it or draw it. However, once the underlying image is disposed, + /// it is no longer possible to use it. If a holder of an image needs to share + /// access to that image with another object or method, [clone] creates a + /// duplicate handle. The underlying image will only be disposed once all + /// outstanding handles are disposed. This allows for safe sharing of image + /// references while still disposing of the underlying resources when all + /// consumers are finished. + /// + /// It is safe to pass an [Image] handle to another object or method if the + /// current holder no longer needs it. + /// + /// The following example demonstrates valid usage. + /// + /// ```dart + /// import 'dart:async'; + /// + /// Future _loadImage(int width, int height) { + /// final Completer completer = Completer(); + /// decodeImageFromPixels( + /// Uint8List.fromList(List.filled(width * height * 4, 0xFF)), + /// width, + /// height, + /// PixelFormat.rgba8888, + /// // Don't worry about disposing or cloning this image - responsibility + /// // is transferred to the caller, and that is safe since this method + /// // will not touch it again. + /// (Image image) => completer.complete(image), + /// ); + /// return completer.future; + /// } + /// + /// Future main() async { + /// final Image image = await _loadImage(5, 5); + /// // Make sure to clone the image, because MyHolder might dispose it + /// // and we need to access it again. + /// final MyImageHolder holder = MyImageHolder(image.clone()); + /// final MyImageHolder holder2 = MyImageHolder(image.clone()); + /// // Now we dispose it because we won't need it again. + /// image.dispose(); + /// + /// final PictureRecorder recorder = PictureRecorder(); + /// final Canvas canvas = Canvas(recorder); + /// + /// holder.draw(canvas); + /// holder.dispose(); + /// + /// canvas.translate(50, 50); + /// holder2.draw(canvas); + /// holder2.dispose(); + /// } + /// + /// class MyImageHolder { + /// MyImageLoader(this.image); + /// + /// final Image image; + /// + /// void draw(Canvas canvas) { + /// canvas.drawImage(image, Offset.zero, Paint()); + /// } + /// + /// void dispose() => image.dispose(); + /// } + /// ``` + /// + /// The returned object behaves identically to this image. Calling + /// [dispose] on it will only dispose the underlying native resources if it + /// is the last remaining handle. + Image clone() { + if (_disposed) { + throw StateError( + 'Cannot clone a disposed image.\n' + 'The clone() method of a previously-disposed Image was called. Once an ' + 'Image object has been disposed, it can no longer be used to create ' + 'handles, as the underlying data may have been released.' + ); + } + assert(!_image._disposed); + return Image._(_image); + } + + @override + String toString() => _image.toString(); +} + +@pragma('vm:entry-point') +class _Image extends NativeFieldWrapperClass2 { + // This class is created by the engine, and should not be instantiated + // or extended directly. + // + // _Images are always handed out wrapped in [Image]s. To create an [Image], + // use the ImageDescriptor API. + @pragma('vm:entry-point') + _Image._(); + + int get width native 'Image_width'; + + int get height native 'Image_height'; + Future toByteData({ImageByteFormat format = ImageByteFormat.rawRgba}) { return _futurize((_Callback callback) { return _toByteData(format.index, (Uint8List? encoded) { @@ -1606,9 +1769,23 @@ class Image extends NativeFieldWrapperClass2 { /// Returns an error message on failure, null on success. String? _toByteData(int format, _Callback callback) native 'Image_toByteData'; - /// Release the resources used by this object. The object is no longer usable - /// after this method is called. - void dispose() native 'Image_dispose'; + bool _disposed = false; + void dispose() { + assert(!_disposed); + assert( + _handles.isEmpty, + 'Attempted to dispose of an Image object that has ${_handles.length} ' + 'open handles.\n' + 'If you see this, it is a bug in dart:ui. Please file an issue at ' + 'https://github.com/flutter/flutter/issues/new.', + ); + _disposed = true; + _dispose(); + } + + void _dispose() native 'Image_dispose'; + + Set _handles = {}; @override String toString() => '[$width\u00D7$height]'; @@ -1621,22 +1798,65 @@ 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 { +/// +/// The recipient of an instance of this class is responsible for calling +/// [Image.dispose] on [image]. To share the image with other interested +/// parties, use [Image.clone]. If the [FrameInfo] object itself is passed to +/// another method or object, that method or object must assume it is +/// responsible for disposing the image when done, and the passer must not +/// access the [image] after that point. +/// +/// For example, the following code sample is incorrect: +/// +/// ```dart +/// /// BAD +/// Future nextFrameRoutine(Codec codec) async { +/// final FrameInfo frameInfo = await codec.getNextFrame(); +/// _cacheImage(frameInfo); +/// // ERROR - _cacheImage is now responsible for disposing the image, and +/// // the image may not be available any more for this drawing routine. +/// _drawImage(frameInfo); +/// // ERROR again - the previous methods might or might not have created +/// // handles to the image. +/// frameInfo.image.dispose(); +/// } +/// ``` +/// +/// Correct usage is: +/// +/// ```dart +/// /// GOOD +/// Future nextFrameRoutine(Codec codec) async { +/// final FrameInfo frameInfo = await codec.getNextFrame(); +/// _cacheImage(frameInfo.image.clone(), frameInfo.duration); +/// _drawImage(frameInfo.image.clone(), frameInfo.duration); +/// // This method is done with its handle, and has passed handles to its +/// // clients already. +/// // The image will live until those clients dispose of their handles, and +/// // this one must not be disposed since it will not be used again. +/// frameInfo.image.dispose(); +/// } +/// ``` +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._({required this.duration, required this.image}); /// The duration this frame should be shown. - Duration get duration => Duration(milliseconds: _durationMillis); - int get _durationMillis native 'FrameInfo_durationMillis'; + /// + /// A zero duration indicates that the frame should be shown indefinitely. + final Duration duration; + /// The [Image] object for this frame. - Image get image native 'FrameInfo_image'; + /// + /// This object must be disposed by the recipient of this frame info. + /// + /// To share this image with other interested parties, use [Image.clone]. + final Image image; } /// A handle to an image codec. @@ -1657,26 +1877,52 @@ class Codec extends NativeFieldWrapperClass2 { @pragma('vm:entry-point') Codec._(); + int? _cachedFrameCount; /// Number of frames in this image. - int get frameCount native 'Codec_frameCount'; + int get frameCount => _cachedFrameCount ??= _frameCount; + int get _frameCount native 'Codec_frameCount'; + int? _cachedRepetitionCount; /// Number of times to repeat the animation. /// /// * 0 when the animation should be played once. /// * -1 for infinity repetitions. - int get repetitionCount native 'Codec_repetitionCount'; + int get repetitionCount => _cachedRepetitionCount ??= _repetitionCount; + 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() { - return _futurize(_getNextFrame); + /// + /// The caller of this method is responsible for disposing the + /// [FrameInfo.image] on the returned object. + Future getNextFrame() async { + if (_cachedFrame == null || frameCount != 1) { + final Completer completer = Completer.sync(); + final String? error = _getNextFrame((_Image? image, int durationMilliseconds) { + if (image == null) { + throw Exception('Codec failed to produce an image, possibly due to invalid image data.'); + } + _cachedFrame = FrameInfo._( + image: Image._(image), + duration: Duration(milliseconds: durationMilliseconds), + ); + completer.complete(); + }); + if (error != null) { + throw Exception(error); + } + await completer.future; + } + return _cachedFrame!; } /// Returns an error message on failure, null on success. - String _getNextFrame(_Callback callback) native 'Codec_getNextFrame'; + String? _getNextFrame(void Function(_Image?, int) callback) native 'Codec_getNextFrame'; /// Release the resources used by this object. The object is no longer usable /// after this method is called. @@ -3239,10 +3485,10 @@ class ImageShader extends Shader { if (matrix4.length != 16) throw ArgumentError('"matrix4" must have 16 entries.'); _constructor(); - _initWithImage(image, tmx.index, tmy.index, matrix4); + _initWithImage(image._image, tmx.index, tmy.index, matrix4); } void _constructor() native 'ImageShader_constructor'; - void _initWithImage(Image image, int tmx, int tmy, Float64List matrix4) native 'ImageShader_initWithImage'; + void _initWithImage(_Image image, int tmx, int tmy, Float64List matrix4) native 'ImageShader_initWithImage'; } /// Defines how a list of points is interpreted when drawing a set of triangles. @@ -3843,9 +4089,9 @@ class Canvas extends NativeFieldWrapperClass2 { assert(image != null); // image is checked on the engine side assert(_offsetIsValid(offset)); assert(paint != null); // ignore: unnecessary_null_comparison - _drawImage(image, offset.dx, offset.dy, paint._objects, paint._data); + _drawImage(image._image, offset.dx, offset.dy, paint._objects, paint._data); } - void _drawImage(Image image, + void _drawImage(_Image image, double x, double y, List? paintObjects, @@ -3866,7 +4112,7 @@ class Canvas extends NativeFieldWrapperClass2 { assert(_rectIsValid(src)); assert(_rectIsValid(dst)); assert(paint != null); // ignore: unnecessary_null_comparison - _drawImageRect(image, + _drawImageRect(image._image, src.left, src.top, src.right, @@ -3878,7 +4124,7 @@ class Canvas extends NativeFieldWrapperClass2 { paint._objects, paint._data); } - void _drawImageRect(Image image, + void _drawImageRect(_Image image, double srcLeft, double srcTop, double srcRight, @@ -3909,7 +4155,7 @@ class Canvas extends NativeFieldWrapperClass2 { assert(_rectIsValid(center)); assert(_rectIsValid(dst)); assert(paint != null); // ignore: unnecessary_null_comparison - _drawImageNine(image, + _drawImageNine(image._image, center.left, center.top, center.right, @@ -3921,7 +4167,7 @@ class Canvas extends NativeFieldWrapperClass2 { paint._objects, paint._data); } - void _drawImageNine(Image image, + void _drawImageNine(_Image image, double centerLeft, double centerTop, double centerRight, @@ -4198,7 +4444,7 @@ class Canvas extends NativeFieldWrapperClass2 { final Float32List? cullRectBuffer = cullRect?._value32; _drawAtlas( - paint._objects, paint._data, atlas, rstTransformBuffer, rectBuffer, + paint._objects, paint._data, atlas._image, rstTransformBuffer, rectBuffer, colorBuffer, (blendMode ?? BlendMode.src).index, cullRectBuffer ); } @@ -4367,14 +4613,14 @@ class Canvas extends NativeFieldWrapperClass2 { throw ArgumentError('If non-null, "colors" length must be one fourth the length of "rstTransforms" and "rects".'); _drawAtlas( - paint._objects, paint._data, atlas, rstTransforms, rects, + paint._objects, paint._data, atlas._image, rstTransforms, rects, colors, (blendMode ?? BlendMode.src).index, cullRect?._value32 ); } void _drawAtlas(List? paintObjects, ByteData paintData, - Image atlas, + _Image atlas, Float32List rstTransforms, Float32List rects, Int32List? colors, @@ -4428,11 +4674,13 @@ class Picture extends NativeFieldWrapperClass2 { if (width <= 0 || height <= 0) throw Exception('Invalid image dimensions.'); return _futurize( - (_Callback callback) => _toImage(width, height, callback) + (_Callback callback) => _toImage(width, height, (_Image image) { + callback(Image._(image)); + }), ); } - String _toImage(int width, int height, _Callback callback) native 'Picture_toImage'; + String _toImage(int width, int height, _Callback<_Image> 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.h b/lib/ui/painting/codec.h index 2e0c746eac2d6..4740a3488d476 100644 --- a/lib/ui/painting/codec.h +++ b/lib/ui/painting/codec.h @@ -6,7 +6,7 @@ #define FLUTTER_LIB_UI_PAINTING_CODEC_H_ #include "flutter/lib/ui/dart_wrapper.h" -#include "flutter/lib/ui/painting/frame_info.h" +#include "flutter/lib/ui/ui_dart_state.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" 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 184b132d17791..0000000000000 --- a/lib/ui/painting/frame_info.h +++ /dev/null @@ -1,37 +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: - 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.cc b/lib/ui/painting/image.cc index 64474227404a9..1f813096b8e73 100644 --- a/lib/ui/painting/image.cc +++ b/lib/ui/painting/image.cc @@ -14,7 +14,14 @@ namespace flutter { typedef CanvasImage Image; -IMPLEMENT_WRAPPERTYPEINFO(ui, Image); +// Since _Image is a private class, we can't use IMPLEMENT_WRAPPERTYPEINFO +static const tonic::DartWrapperInfo kDartWrapperInfo_ui_Image = { + "ui", + "_Image", + sizeof(Image), +}; +const tonic::DartWrapperInfo& Image::dart_wrapper_info_ = + kDartWrapperInfo_ui_Image; #define FOR_EACH_BINDING(V) \ V(Image, width) \ diff --git a/lib/ui/painting/image_dispose_unittests.cc b/lib/ui/painting/image_dispose_unittests.cc index 3763d70539f95..63737cc3d0fec 100644 --- a/lib/ui/painting/image_dispose_unittests.cc +++ b/lib/ui/painting/image_dispose_unittests.cc @@ -19,11 +19,11 @@ namespace testing { class ImageDisposeTest : public ShellTest { public: template - T* GetNativePeer(Dart_NativeArguments args, int index) { - auto handle = Dart_GetNativeArgument(args, index); + T* GetNativePeer(Dart_Handle handle) { intptr_t peer = 0; - EXPECT_FALSE(Dart_IsError(Dart_GetNativeInstanceField( - handle, tonic::DartWrappable::kPeerIndex, &peer))); + auto native_handle = Dart_GetNativeInstanceField( + handle, tonic::DartWrappable::kPeerIndex, &peer); + EXPECT_FALSE(Dart_IsError(native_handle)) << Dart_GetError(native_handle); return reinterpret_cast(peer); } @@ -42,8 +42,14 @@ class ImageDisposeTest : public ShellTest { TEST_F(ImageDisposeTest, ImageReleasedAfterFrame) { auto native_capture_image_and_picture = [&](Dart_NativeArguments args) { - CanvasImage* image = GetNativePeer(args, 0); - Picture* picture = GetNativePeer(args, 1); + auto image_handle = Dart_GetNativeArgument(args, 0); + auto native_image_handle = + Dart_GetField(image_handle, Dart_NewStringFromCString("_image")); + ASSERT_FALSE(Dart_IsError(native_image_handle)) + << Dart_GetError(native_image_handle); + ASSERT_FALSE(Dart_IsNull(native_image_handle)); + CanvasImage* image = GetNativePeer(native_image_handle); + Picture* picture = GetNativePeer(Dart_GetNativeArgument(args, 1)); ASSERT_FALSE(image->image()->unique()); ASSERT_FALSE(picture->picture()->unique()); current_image_ = image->image(); diff --git a/lib/ui/painting/image_encoding_unittests.cc b/lib/ui/painting/image_encoding_unittests.cc index 194c9501af9cb..452c92ba9aa09 100644 --- a/lib/ui/painting/image_encoding_unittests.cc +++ b/lib/ui/painting/image_encoding_unittests.cc @@ -21,6 +21,10 @@ fml::AutoResetWaitableEvent message_latch; TEST_F(ShellTest, EncodeImageGivesExternalTypedData) { auto nativeEncodeImage = [&](Dart_NativeArguments args) { auto image_handle = Dart_GetNativeArgument(args, 0); + image_handle = + Dart_GetField(image_handle, Dart_NewStringFromCString("_image")); + ASSERT_FALSE(Dart_IsError(image_handle)) << Dart_GetError(image_handle); + ASSERT_FALSE(Dart_IsNull(image_handle)); auto format_handle = Dart_GetNativeArgument(args, 1); auto callback_handle = Dart_GetNativeArgument(args, 2); diff --git a/lib/ui/painting/multi_frame_codec.cc b/lib/ui/painting/multi_frame_codec.cc index c40561836011a..2ab5ce6d108c4 100644 --- a/lib/ui/painting/multi_frame_codec.cc +++ b/lib/ui/painting/multi_frame_codec.cc @@ -5,6 +5,7 @@ #include "flutter/lib/ui/painting/multi_frame_codec.h" #include "flutter/fml/make_copyable.h" +#include "flutter/lib/ui/painting/image.h" #include "third_party/dart/runtime/include/dart_api.h" #include "third_party/skia/include/core/SkPixelRef.h" #include "third_party/tonic/logging/dart_invoke.h" @@ -24,7 +25,8 @@ MultiFrameCodec::State::State(std::shared_ptr generator) nextFrameIndex_(0) {} static void InvokeNextFrameCallback( - fml::RefPtr frameInfo, + fml::RefPtr image, + int duration, std::unique_ptr callback, size_t trace_id) { std::shared_ptr dart_state = callback->dart_state().lock(); @@ -34,11 +36,8 @@ static void InvokeNextFrameCallback( return; } tonic::DartState::Scope scope(dart_state); - if (!frameInfo) { - tonic::DartInvoke(callback->value(), {Dart_Null()}); - } else { - tonic::DartInvoke(callback->value(), {ToDart(frameInfo)}); - } + tonic::DartInvoke(callback->value(), + {tonic::ToDart(image), tonic::ToDart(duration)}); } // Copied the source bitmap to the destination. If this cannot occur due to @@ -139,22 +138,24 @@ void MultiFrameCodec::State::GetNextFrameAndInvokeCallback( fml::WeakPtr resourceContext, fml::RefPtr unref_queue, size_t trace_id) { - fml::RefPtr frameInfo = NULL; + fml::RefPtr image = nullptr; + int duration = 0; sk_sp skImage = GetNextFrameImage(resourceContext); if (skImage) { - fml::RefPtr image = CanvasImage::Create(); + image = CanvasImage::Create(); image->set_image({skImage, std::move(unref_queue)}); SkCodec::FrameInfo skFrameInfo{0}; generator_->getFrameInfo(nextFrameIndex_, &skFrameInfo); - frameInfo = - fml::MakeRefCounted(std::move(image), skFrameInfo.fDuration); + duration = 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); - })); + ui_task_runner->PostTask(fml::MakeCopyable([callback = std::move(callback), + image = std::move(image), + duration, trace_id]() mutable { + InvokeNextFrameCallback(std::move(image), duration, std::move(callback), + trace_id); + })); } Dart_Handle MultiFrameCodec::getNextFrame(Dart_Handle callback_handle) { diff --git a/lib/ui/painting/single_frame_codec.cc b/lib/ui/painting/single_frame_codec.cc index dbe603a5f043c..3d9d51f02a2dd 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" @@ -34,7 +33,8 @@ Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle callback_handle) { } if (status_ == Status::kComplete) { - tonic::DartInvoke(callback_handle, {tonic::ToDart(cached_frame_)}); + tonic::DartInvoke(callback_handle, + {tonic::ToDart(cached_image_), tonic::ToDart(0)}); return Dart_Null(); } @@ -82,8 +82,7 @@ Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle callback_handle) { auto canvas_image = fml::MakeRefCounted(); canvas_image->set_image(std::move(image)); - codec->cached_frame_ = fml::MakeRefCounted( - std::move(canvas_image), 0 /* duration */); + codec->cached_image_ = std::move(canvas_image); } // The cached frame is now available and should be returned to any @@ -91,9 +90,10 @@ 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(), + {tonic::ToDart(codec->cached_image_), tonic::ToDart(0)}); } codec->pending_callbacks_.clear(); }); @@ -109,9 +109,8 @@ Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle callback_handle) { size_t SingleFrameCodec::GetAllocationSize() const { const auto& data_size = descriptor_->GetAllocationSize(); - const auto frame_byte_size = (cached_frame_ && cached_frame_->image()) - ? cached_frame_->image()->GetAllocationSize() - : 0; + const auto frame_byte_size = + cached_image_ ? cached_image_->GetAllocationSize() : 0; return data_size + frame_byte_size + sizeof(this); } diff --git a/lib/ui/painting/single_frame_codec.h b/lib/ui/painting/single_frame_codec.h index 587bea6fed588..9685c047e3efd 100644 --- a/lib/ui/painting/single_frame_codec.h +++ b/lib/ui/painting/single_frame_codec.h @@ -7,7 +7,7 @@ #include "flutter/fml/macros.h" #include "flutter/lib/ui/painting/codec.h" -#include "flutter/lib/ui/painting/frame_info.h" +#include "flutter/lib/ui/painting/image.h" #include "flutter/lib/ui/painting/image_decoder.h" #include "flutter/lib/ui/painting/image_descriptor.h" @@ -39,7 +39,7 @@ class SingleFrameCodec : public Codec { fml::RefPtr descriptor_; uint32_t target_width_; uint32_t target_height_; - fml::RefPtr cached_frame_; + fml::RefPtr cached_image_; std::vector pending_callbacks_; FML_FRIEND_MAKE_REF_COUNTED(SingleFrameCodec); diff --git a/lib/web_ui/lib/src/engine/canvaskit/image.dart b/lib/web_ui/lib/src/engine/canvaskit/image.dart index 3147b41e0743b..7ef931939889f 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/image.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/image.dart @@ -52,6 +52,11 @@ class CkAnimatedImage implements ui.Image { box.delete(); } + @override + ui.Image clone() => this; + + @override + List? debugGetOpenHandleStackTraces() => null; int get frameCount => _skAnimatedImage.getFrameCount(); /// Decodes the next frame and returns the frame duration. @@ -117,6 +122,12 @@ class CkImage implements ui.Image { box.delete(); } + @override + ui.Image clone() => this; + + @override + List? debugGetOpenHandleStackTraces() => null; + @override int get width => skImage.width(); diff --git a/lib/web_ui/lib/src/engine/html_image_codec.dart b/lib/web_ui/lib/src/engine/html_image_codec.dart index a380deb69cdb9..1d99c65c1a228 100644 --- a/lib/web_ui/lib/src/engine/html_image_codec.dart +++ b/lib/web_ui/lib/src/engine/html_image_codec.dart @@ -122,6 +122,12 @@ class HtmlImage implements ui.Image { // releasing the object url. } + @override + ui.Image clone() => this; + + @override + List? debugGetOpenHandleStackTraces() => null; + @override final int width; diff --git a/lib/web_ui/lib/src/ui/painting.dart b/lib/web_ui/lib/src/ui/painting.dart index 0215e2f39e8a9..0b49f966d4fd0 100644 --- a/lib/web_ui/lib/src/ui/painting.dart +++ b/lib/web_ui/lib/src/ui/painting.dart @@ -330,6 +330,10 @@ abstract class Image { Future toByteData({ImageByteFormat format = ImageByteFormat.rawRgba}); void dispose(); + Image clone() => this; + + List? debugGetOpenHandleStackTraces() => null; + @override String toString() => '[$width\u00D7$height]'; } diff --git a/lib/web_ui/test/golden_tests/engine/recording_canvas_golden_test.dart b/lib/web_ui/test/golden_tests/engine/recording_canvas_golden_test.dart index 63c727a395009..e2cb9fcc6aa27 100644 --- a/lib/web_ui/test/golden_tests/engine/recording_canvas_golden_test.dart +++ b/lib/web_ui/test/golden_tests/engine/recording_canvas_golden_test.dart @@ -722,6 +722,12 @@ class TestImage implements Image { @override void dispose() {} + + @override + Image clone() => this; + + @override + List/*?*/ debugGetOpenHandleStackTraces() => []; } Paragraph createTestParagraph() { diff --git a/testing/dart/image_dispose_test.dart b/testing/dart/image_dispose_test.dart new file mode 100644 index 0000000000000..dc89643906360 --- /dev/null +++ b/testing/dart/image_dispose_test.dart @@ -0,0 +1,104 @@ +// Copyright 2019 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. + +// @dart = 2.6 +import 'dart:io'; +import 'dart:typed_data'; +import 'dart:ui'; + +import 'package:path/path.dart' as path; +import 'package:test/test.dart'; + +void main() { + bool assertsEnabled = false; + assert(() { + assertsEnabled = true; + return true; + }()); + + test('Handles are distinct', () async { + final Uint8List bytes = await readFile('2x2.png'); + final Codec codec = await instantiateImageCodec(bytes); + final FrameInfo frame = await codec.getNextFrame(); + + expect(frame.image.width, 2); + expect(frame.image.height, 2); + final Image handle1 = frame.image.clone(); + expect(handle1.width, frame.image.width); + expect(handle1.height, frame.image.height); + + final Image handle2 = handle1.clone(); + expect(handle1 != handle2, true); + expect(handle1 != frame.image, true); + expect(frame.image == frame.image, true); + + frame.image.dispose(); + }); + + test('Canvas can paint image from handle and byte data from handle', () async { + final Uint8List bytes = await readFile('2x2.png'); + final Codec codec = await instantiateImageCodec(bytes); + final FrameInfo frame = await codec.getNextFrame(); + + expect(frame.image.width, 2); + expect(frame.image.height, 2); + final Image handle1 = frame.image.clone(); + + final PictureRecorder recorder = PictureRecorder(); + final Canvas canvas = Canvas(recorder); + + const Rect rect = Rect.fromLTRB(0, 0, 2, 2); + canvas.drawImage(handle1, Offset.zero, Paint()); + canvas.drawImageRect(handle1, rect, rect, Paint()); + canvas.drawImageNine(handle1, rect, rect, Paint()); + canvas.drawAtlas(handle1, [], [], [], BlendMode.src, rect, Paint()); + canvas.drawRawAtlas(handle1, Float32List(0), Float32List(0), Int32List(0), BlendMode.src, rect, Paint()); + + final Picture picture = recorder.endRecording(); + + final Image rasterizedImage = await picture.toImage(2, 2); + final ByteData sourceData = await frame.image.toByteData(); + final ByteData handleData = await handle1.toByteData(); + final ByteData rasterizedData = await rasterizedImage.toByteData(); + + expect(sourceData.buffer.asUint8List(), equals(handleData.buffer.asUint8List())); + expect(sourceData.buffer.asUint8List(), equals(rasterizedData.buffer.asUint8List())); + }); + + test('Records stack traces', () async { + final Uint8List bytes = await readFile('2x2.png'); + final Codec codec = await instantiateImageCodec(bytes); + final FrameInfo frame = await codec.getNextFrame(); + + final Image handle1 = frame.image.clone(); + final Image handle2 = handle1.clone(); + + List stackTraces = frame.image.debugGetOpenHandleStackTraces(); + expect(stackTraces.length, 3); + expect(stackTraces, equals(handle1.debugGetOpenHandleStackTraces())); + + handle1.dispose(); + stackTraces = frame.image.debugGetOpenHandleStackTraces(); + expect(stackTraces.length, 2); + expect(stackTraces, equals(handle2.debugGetOpenHandleStackTraces())); + + handle2.dispose(); + stackTraces = frame.image.debugGetOpenHandleStackTraces(); + expect(stackTraces.length, 1); + expect(stackTraces, equals(frame.image.debugGetOpenHandleStackTraces())); + + frame.image.dispose(); + expect(frame.image.debugGetOpenHandleStackTraces(), isEmpty); + }, skip: !assertsEnabled); +} + +Future readFile(String fileName) async { + final File file = File(path.join( + 'flutter', + 'testing', + 'resources', + fileName, + )); + return await file.readAsBytes(); +}