From 84b879c966ecc8c9ff61c2c637402046d5602cfb Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 23 Sep 2020 17:18:31 -0700 Subject: [PATCH 1/2] isCloneOf for Image --- lib/ui/painting.dart | 11 +++++++++ .../lib/src/engine/canvaskit/image.dart | 6 +++++ .../lib/src/engine/html_image_codec.dart | 3 +++ .../engine/recording_canvas_golden_test.dart | 3 +++ testing/dart/image_dispose_test.dart | 23 +++++++++++++++++++ 5 files changed, 46 insertions(+) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 993029a1e6998..0caa161a7b1f4 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -1740,6 +1740,17 @@ class Image { return Image._(_image); } + /// Returns true if `other` shares the same underlying image memory as this, + /// even if this or `other` is [dispose]d. + /// + /// This method may return false for two images that were decoded from the + /// same underlying asset, if they are not sharing the same memory. For + /// example, if the same file is decoded using [instantiateImageCodec] twice, + /// or the same bytes are decoded using [decodeImageFromPixels] twice, there + /// will be two distinct [Image]s that render the same but do not share + /// underlying memory, and so will not be treated as clones of each other. + bool isCloneOf(Image other) => other._image == _image; + @override String toString() => _image.toString(); } diff --git a/lib/web_ui/lib/src/engine/canvaskit/image.dart b/lib/web_ui/lib/src/engine/canvaskit/image.dart index 7ef931939889f..badaa8e6042e4 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/image.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/image.dart @@ -55,6 +55,9 @@ class CkAnimatedImage implements ui.Image { @override ui.Image clone() => this; + @override + bool isCloneOf(ui.Image other) => other == this; + @override List? debugGetOpenHandleStackTraces() => null; int get frameCount => _skAnimatedImage.getFrameCount(); @@ -125,6 +128,9 @@ class CkImage implements ui.Image { @override ui.Image clone() => this; + @override + bool isCloneOf(ui.Image other) => other == this; + @override List? debugGetOpenHandleStackTraces() => null; 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 1d99c65c1a228..6a6a7e6aaf45b 100644 --- a/lib/web_ui/lib/src/engine/html_image_codec.dart +++ b/lib/web_ui/lib/src/engine/html_image_codec.dart @@ -125,6 +125,9 @@ class HtmlImage implements ui.Image { @override ui.Image clone() => this; + @override + bool isCloneOf(ui.Image other) => other == this; + @override List? debugGetOpenHandleStackTraces() => null; 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 e2cb9fcc6aa27..c7854c961a798 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 @@ -726,6 +726,9 @@ class TestImage implements Image { @override Image clone() => this; + @override + bool isCloneOf(Image other) => other == this; + @override List/*?*/ debugGetOpenHandleStackTraces() => []; } diff --git a/testing/dart/image_dispose_test.dart b/testing/dart/image_dispose_test.dart index dc89643906360..3e74fb3eebd07 100644 --- a/testing/dart/image_dispose_test.dart +++ b/testing/dart/image_dispose_test.dart @@ -91,6 +91,29 @@ void main() { frame.image.dispose(); expect(frame.image.debugGetOpenHandleStackTraces(), isEmpty); }, skip: !assertsEnabled); + + test('Clones can be compared', () 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(); + + expect(handle1.isCloneOf(handle2), true); + expect(handle2.isCloneOf(handle1), true); + expect(handle1.isCloneOf(frame.image), true); + + handle1.dispose(); + expect(handle1.isCloneOf(handle2), true); + expect(handle2.isCloneOf(handle1), true); + expect(handle1.isCloneOf(frame.image), true); + + final Codec codec2 = await instantiateImageCodec(bytes); + final FrameInfo frame2 = await codec2.getNextFrame(); + + expect(frame2.image.isCloneOf(frame.image), false); + }); } Future readFile(String fileName) async { From 9660d698dc9e30b41879de9a94d1c0f52f38d70b Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 23 Sep 2020 20:16:04 -0700 Subject: [PATCH 2/2] Update docs, add missing impl --- lib/ui/painting.dart | 8 ++++++-- lib/web_ui/lib/src/ui/painting.dart | 2 ++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 0caa161a7b1f4..3eb3f4fdd3683 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -1671,6 +1671,10 @@ class Image { /// It is safe to pass an [Image] handle to another object or method if the /// current holder no longer needs it. /// + /// To check whether two [Image] references are refering to the same + /// underlying image memory, use [isCloneOf] rather than the equality operator + /// or [identical]. + /// /// The following example demonstrates valid usage. /// /// ```dart @@ -1740,8 +1744,8 @@ class Image { return Image._(_image); } - /// Returns true if `other` shares the same underlying image memory as this, - /// even if this or `other` is [dispose]d. + /// Returns true if `other` is a [clone] of this and thus shares the same + /// underlying image memory, even if this or `other` is [dispose]d. /// /// This method may return false for two images that were decoded from the /// same underlying asset, if they are not sharing the same memory. For diff --git a/lib/web_ui/lib/src/ui/painting.dart b/lib/web_ui/lib/src/ui/painting.dart index 0b49f966d4fd0..ed0a7b9c81601 100644 --- a/lib/web_ui/lib/src/ui/painting.dart +++ b/lib/web_ui/lib/src/ui/painting.dart @@ -332,6 +332,8 @@ abstract class Image { Image clone() => this; + bool isCloneOf(Image other) => other == this; + List? debugGetOpenHandleStackTraces() => null; @override