From abe0788526759ea1453f00759fec4012f4eb1fcc Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 15 Mar 2023 09:13:37 -0700 Subject: [PATCH 1/2] Reland "Added wide-gamut color support for ui.Image.toByteData and ui.Image.colorSpace" --- lib/ui/dart_ui.cc | 1 + lib/ui/painting.dart | 70 +++++++++++++++++++ lib/ui/painting/image.cc | 15 ++++ lib/ui/painting/image.h | 8 +++ lib/ui/painting/image_encoding.cc | 17 +++-- lib/ui/painting/image_encoding_impeller.cc | 12 ++++ lib/ui/painting/image_encoding_impeller.h | 2 + lib/web_ui/lib/painting.dart | 7 ++ .../lib/src/engine/canvaskit/image.dart | 3 + .../lib/src/engine/html_image_codec.dart | 3 + .../src/engine/skwasm/skwasm_impl/image.dart | 3 + .../html/recording_canvas_golden_test.dart | 5 +- testing/dart/encoding_test.dart | 15 ++++ 13 files changed, 153 insertions(+), 8 deletions(-) diff --git a/lib/ui/dart_ui.cc b/lib/ui/dart_ui.cc index aabdaa43faf3e..57242c11ac9aa 100644 --- a/lib/ui/dart_ui.cc +++ b/lib/ui/dart_ui.cc @@ -190,6 +190,7 @@ typedef CanvasPath Path; V(Image, width, 1) \ V(Image, height, 1) \ V(Image, toByteData, 3) \ + V(Image, colorSpace, 1) \ V(ImageDescriptor, bytesPerPixel, 1) \ V(ImageDescriptor, dispose, 1) \ V(ImageDescriptor, height, 1) \ diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index ee2d173259216..b932a2510006a 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -1568,6 +1568,31 @@ class Paint { } } +/// The color space describes the colors that are available to an [Image]. +/// +/// This value can help decide which [ImageByteFormat] to use with +/// [Image.toByteData]. Images that are in the [extendedSRGB] color space +/// should use something like [ImageByteFormat.rawExtendedRgba128] so that +/// colors outside of the sRGB gamut aren't lost. +/// +/// This is also the result of [Image.colorSpace]. +/// +/// See also: https://en.wikipedia.org/wiki/Color_space +enum ColorSpace { + /// The sRGB color space. + /// + /// You may know this as the standard color space for the web or the color + /// space of non-wide-gamut Flutter apps. + /// + /// See also: https://en.wikipedia.org/wiki/SRGB + sRGB, + /// A color space that is backwards compatible with sRGB but can represent + /// colors outside of that gamut with values outside of [0..1]. In order to + /// see the extended values an [ImageByteFormat] like + /// [ImageByteFormat.rawExtendedRgba128] must be used. + extendedSRGB, +} + /// The format in which image bytes should be returned when using /// [Image.toByteData]. // We do not expect to add more encoding formats to the ImageByteFormat enum, @@ -1591,6 +1616,21 @@ enum ImageByteFormat { /// image may use a single 8-bit channel for each pixel. rawUnmodified, + /// Raw extended range RGBA format. + /// + /// Unencoded bytes, in RGBA row-primary form with straight alpha, 32 bit + /// float (IEEE 754 binary32) per channel. + /// + /// Example usage: + /// + /// ```dart + /// final ByteData data = + /// (await image.toByteData(format: ImageByteFormat.rawExtendedRgba128))!; + /// final Float32List floats = Float32List.view(data.buffer); + /// print('r:${floats[0]} g:${floats[1]} b:${floats[2]} a:${floats[3]}'); + /// ``` + rawExtendedRgba128, + /// PNG format. /// /// A loss-less compression format for images. This format is well suited for @@ -1726,6 +1766,10 @@ class Image { /// The [format] argument specifies the format in which the bytes will be /// returned. /// + /// Using [ImageByteFormat.rawRgba] on an image in the color space + /// [ColorSpace.extendedSRGB] will result in the gamut being squished to fit + /// into the sRGB gamut, resulting in the loss of wide-gamut colors. + /// /// Returns a future that completes with the binary image data or an error /// if encoding fails. // We do not expect to add more encoding formats to the ImageByteFormat enum, @@ -1737,6 +1781,29 @@ class Image { return _image.toByteData(format: format); } + /// The color space that is used by the [Image]'s colors. + /// + /// This value is a consequence of how the [Image] has been created. For + /// example, loading a PNG that is in the Display P3 color space will result + /// in a [ColorSpace.extendedSRGB] image. + /// + /// On rendering backends that don't support wide gamut colors (anything but + /// iOS impeller), wide gamut images will still report [ColorSpace.sRGB] if + /// rendering wide gamut colors isn't supported. + // Note: The docstring will become outdated as new platforms support wide + // gamut color, please keep it up to date. + ColorSpace get colorSpace { + final int colorSpaceValue = _image.colorSpace; + switch (colorSpaceValue) { + case 0: + return ColorSpace.sRGB; + case 1: + return ColorSpace.extendedSRGB; + default: + throw UnsupportedError('Unrecognized color space: $colorSpaceValue'); + } + } + /// If asserts are enabled, returns the [StackTrace]s of each open handle from /// [clone], in creation order. /// @@ -1903,6 +1970,9 @@ class _Image extends NativeFieldWrapperClass1 { final Set _handles = {}; + @Native)>(symbol: 'Image::colorSpace') + external int get colorSpace; + @override String toString() => '[$width\u00D7$height]'; } diff --git a/lib/ui/painting/image.cc b/lib/ui/painting/image.cc index 2a9ab500c8e41..a0b646ad795fc 100644 --- a/lib/ui/painting/image.cc +++ b/lib/ui/painting/image.cc @@ -7,6 +7,9 @@ #include #include +#if IMPELLER_SUPPORTS_RENDERING +#include "flutter/lib/ui/painting/image_encoding_impeller.h" +#endif #include "flutter/lib/ui/painting/image_encoding.h" #include "third_party/tonic/converter/dart_converter.h" #include "third_party/tonic/dart_args.h" @@ -35,4 +38,16 @@ void CanvasImage::dispose() { ClearDartWrapper(); } +int CanvasImage::colorSpace() { + if (image_->skia_image()) { + return ColorSpace::kSRGB; + } else if (image_->impeller_texture()) { +#if IMPELLER_SUPPORTS_RENDERING + return ImageEncodingImpeller::GetColorSpace(image_->impeller_texture()); +#endif // IMPELLER_SUPPORTS_RENDERING + } + + return -1; +} + } // namespace flutter diff --git a/lib/ui/painting/image.h b/lib/ui/painting/image.h index 35b710160ed54..666b8ab622d60 100644 --- a/lib/ui/painting/image.h +++ b/lib/ui/painting/image.h @@ -14,6 +14,12 @@ namespace flutter { +// Must be kept in sync with painting.dart. +enum ColorSpace { + kSRGB, + kExtendedSRGB, +}; + class CanvasImage final : public RefCountedDartWrappable { DEFINE_WRAPPERTYPEINFO(); FML_FRIEND_MAKE_REF_COUNTED(CanvasImage); @@ -37,6 +43,8 @@ class CanvasImage final : public RefCountedDartWrappable { void set_image(sk_sp image) { image_ = image; } + int colorSpace(); + private: CanvasImage(); diff --git a/lib/ui/painting/image_encoding.cc b/lib/ui/painting/image_encoding.cc index ea087a044c227..ff243184fbdbe 100644 --- a/lib/ui/painting/image_encoding.cc +++ b/lib/ui/painting/image_encoding.cc @@ -38,6 +38,7 @@ enum ImageByteFormat { kRawRGBA, kRawStraightRGBA, kRawUnmodified, + kRawExtendedRgba128, kPNG, }; @@ -123,19 +124,21 @@ sk_sp EncodeImage(const sk_sp& raster_image, return nullptr; }; return png_image; - } break; - case kRawRGBA: { + } + case kRawRGBA: return CopyImageByteData(raster_image, kRGBA_8888_SkColorType, kPremul_SkAlphaType); - } break; - case kRawStraightRGBA: { + + case kRawStraightRGBA: return CopyImageByteData(raster_image, kRGBA_8888_SkColorType, kUnpremul_SkAlphaType); - } break; - case kRawUnmodified: { + + case kRawUnmodified: return CopyImageByteData(raster_image, raster_image->colorType(), raster_image->alphaType()); - } break; + case kRawExtendedRgba128: + return CopyImageByteData(raster_image, kRGBA_F32_SkColorType, + kUnpremul_SkAlphaType); } FML_LOG(ERROR) << "Unknown error encoding image."; diff --git a/lib/ui/painting/image_encoding_impeller.cc b/lib/ui/painting/image_encoding_impeller.cc index ec429a2cda90f..d87a81b791232 100644 --- a/lib/ui/painting/image_encoding_impeller.cc +++ b/lib/ui/painting/image_encoding_impeller.cc @@ -163,4 +163,16 @@ void ImageEncodingImpeller::ConvertImageToRaster( }); } +int ImageEncodingImpeller::GetColorSpace( + const std::shared_ptr& texture) { + const impeller::TextureDescriptor& desc = texture->GetTextureDescriptor(); + switch (desc.format) { + case impeller::PixelFormat::kB10G10R10XR: // intentional_fallthrough + case impeller::PixelFormat::kR16G16B16A16Float: + return ColorSpace::kExtendedSRGB; + default: + return ColorSpace::kSRGB; + } +} + } // namespace flutter diff --git a/lib/ui/painting/image_encoding_impeller.h b/lib/ui/painting/image_encoding_impeller.h index fafe8fc9992b4..924b18e4eb989 100644 --- a/lib/ui/painting/image_encoding_impeller.h +++ b/lib/ui/painting/image_encoding_impeller.h @@ -17,6 +17,8 @@ namespace flutter { class ImageEncodingImpeller { public: + static int GetColorSpace(const std::shared_ptr& texture); + /// Converts a DlImage to a SkImage. /// This should be called from the thread that corresponds to /// `dl_image->owning_context()` when gpu access is guaranteed. diff --git a/lib/web_ui/lib/painting.dart b/lib/web_ui/lib/painting.dart index 0ec5a827e70c6..d15d83fc4b926 100644 --- a/lib/web_ui/lib/painting.dart +++ b/lib/web_ui/lib/painting.dart @@ -351,6 +351,8 @@ abstract class Image { List? debugGetOpenHandleStackTraces() => null; + ColorSpace get colorSpace => ColorSpace.sRGB; + @override String toString() => '[$width\u00D7$height]'; } @@ -431,6 +433,11 @@ class ImageFilter { engine.renderer.composeImageFilters(outer: outer, inner: inner); } +enum ColorSpace { + sRGB, + extendedSRGB, +} + enum ImageByteFormat { rawRgba, rawStraightRgba, diff --git a/lib/web_ui/lib/src/engine/canvaskit/image.dart b/lib/web_ui/lib/src/engine/canvaskit/image.dart index 220bfccfd2c29..e43157da53138 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/image.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/image.dart @@ -378,6 +378,9 @@ class CkImage implements ui.Image, StackTraceDebugger { } } + @override + ui.ColorSpace get colorSpace => ui.ColorSpace.sRGB; + Future _readPixelsFromSkImage(ui.ImageByteFormat format) { final SkAlphaType alphaType = format == ui.ImageByteFormat.rawStraightRgba ? canvasKit.AlphaType.Unpremul : canvasKit.AlphaType.Premul; final ByteData? data = _encodeImage( 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 d5db33fcbffc1..f1f184b527c26 100644 --- a/lib/web_ui/lib/src/engine/html_image_codec.dart +++ b/lib/web_ui/lib/src/engine/html_image_codec.dart @@ -204,6 +204,9 @@ class HtmlImage implements ui.Image { } } + @override + ui.ColorSpace get colorSpace => ui.ColorSpace.sRGB; + DomHTMLImageElement cloneImageElement() { if (!_didClone) { _didClone = true; diff --git a/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/image.dart b/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/image.dart index 3ca0b738b14e8..a019591f3dde3 100644 --- a/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/image.dart +++ b/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/image.dart @@ -23,6 +23,9 @@ class SkwasmImage implements ui.Image { throw UnimplementedError(); } + @override + ui.ColorSpace get colorSpace => ui.ColorSpace.sRGB; + @override void dispose() { throw UnimplementedError(); diff --git a/lib/web_ui/test/html/recording_canvas_golden_test.dart b/lib/web_ui/test/html/recording_canvas_golden_test.dart index eaad748b4b8de..b032c45ccc0e0 100644 --- a/lib/web_ui/test/html/recording_canvas_golden_test.dart +++ b/lib/web_ui/test/html/recording_canvas_golden_test.dart @@ -7,7 +7,7 @@ import 'dart:typed_data'; import 'package:test/bootstrap/browser.dart'; import 'package:test/test.dart'; -import 'package:ui/src/engine.dart'; +import 'package:ui/src/engine.dart' hide ColorSpace; import 'package:ui/ui.dart' hide TextStyle; import 'package:web_engine_tester/golden_tester.dart'; @@ -780,6 +780,9 @@ class TestImage implements Image { @override List/*?*/ debugGetOpenHandleStackTraces() => []; + + @override + ColorSpace get colorSpace => ColorSpace.sRGB; } Paragraph createTestParagraph() { diff --git a/testing/dart/encoding_test.dart b/testing/dart/encoding_test.dart index 79176c52f6b43..4c18bf4957688 100644 --- a/testing/dart/encoding_test.dart +++ b/testing/dart/encoding_test.dart @@ -67,6 +67,21 @@ void main() { final List expected = await readFile('square.png'); expect(Uint8List.view(data.buffer), expected); }); + + test('Image.toByteData ExtendedRGBA128', () async { + final Image image = await Square4x4Image.image; + final ByteData data = (await image.toByteData(format: ImageByteFormat.rawExtendedRgba128))!; + expect(image.width, _kWidth); + expect(image.height, _kWidth); + expect(data.lengthInBytes, _kWidth * _kWidth * 4 * 4); + // Top-left pixel should be black. + final Float32List floats = Float32List.view(data.buffer); + expect(floats[0], 0.0); + expect(floats[1], 0.0); + expect(floats[2], 0.0); + expect(floats[3], 1.0); + expect(image.colorSpace, ColorSpace.sRGB); + }); } class Square4x4Image { From e601995db92881f855406524e633f0e189d19919 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 15 Mar 2023 09:27:57 -0700 Subject: [PATCH 2/2] updated the code example --- lib/ui/painting.dart | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index b932a2510006a..f2aa98c0a351c 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -1624,10 +1624,20 @@ enum ImageByteFormat { /// Example usage: /// /// ```dart - /// final ByteData data = - /// (await image.toByteData(format: ImageByteFormat.rawExtendedRgba128))!; - /// final Float32List floats = Float32List.view(data.buffer); - /// print('r:${floats[0]} g:${floats[1]} b:${floats[2]} a:${floats[3]}'); + /// import 'dart:ui' as ui; + /// import 'dart:typed_data'; + /// + /// Future> getFirstPixel(ui.Image image) async { + /// final ByteData data = + /// (await image.toByteData(format: ui.ImageByteFormat.rawExtendedRgba128))!; + /// final Float32List floats = Float32List.view(data.buffer); + /// return { + /// 'r': floats[0], + /// 'g': floats[1], + /// 'b': floats[2], + /// 'a': floats[3], + /// }; + /// } /// ``` rawExtendedRgba128,