From 1e2545c0a3114737eb6b47745f7389bd23ce7aec Mon Sep 17 00:00:00 2001 From: Casey Hillers Date: Tue, 14 Mar 2023 20:08:30 -0700 Subject: [PATCH] Revert "Added wide-gamut color support for `ui.Image.toByteData` and `ui.Image.colorSpace` (#40031)" This reverts commit 7675de7149f5a5f700525b7db5f1335113a1ff55. --- 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, 8 insertions(+), 153 deletions(-) diff --git a/lib/ui/dart_ui.cc b/lib/ui/dart_ui.cc index 57242c11ac9aa..aabdaa43faf3e 100644 --- a/lib/ui/dart_ui.cc +++ b/lib/ui/dart_ui.cc @@ -190,7 +190,6 @@ 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 b932a2510006a..ee2d173259216 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -1568,31 +1568,6 @@ 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, @@ -1616,21 +1591,6 @@ 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 @@ -1766,10 +1726,6 @@ 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, @@ -1781,29 +1737,6 @@ 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. /// @@ -1970,9 +1903,6 @@ 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 a0b646ad795fc..2a9ab500c8e41 100644 --- a/lib/ui/painting/image.cc +++ b/lib/ui/painting/image.cc @@ -7,9 +7,6 @@ #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" @@ -38,16 +35,4 @@ 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 666b8ab622d60..35b710160ed54 100644 --- a/lib/ui/painting/image.h +++ b/lib/ui/painting/image.h @@ -14,12 +14,6 @@ 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); @@ -43,8 +37,6 @@ 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 ff243184fbdbe..ea087a044c227 100644 --- a/lib/ui/painting/image_encoding.cc +++ b/lib/ui/painting/image_encoding.cc @@ -38,7 +38,6 @@ enum ImageByteFormat { kRawRGBA, kRawStraightRGBA, kRawUnmodified, - kRawExtendedRgba128, kPNG, }; @@ -124,21 +123,19 @@ sk_sp EncodeImage(const sk_sp& raster_image, return nullptr; }; return png_image; - } - case kRawRGBA: + } break; + case kRawRGBA: { return CopyImageByteData(raster_image, kRGBA_8888_SkColorType, kPremul_SkAlphaType); - - case kRawStraightRGBA: + } break; + case kRawStraightRGBA: { return CopyImageByteData(raster_image, kRGBA_8888_SkColorType, kUnpremul_SkAlphaType); - - case kRawUnmodified: + } break; + case kRawUnmodified: { return CopyImageByteData(raster_image, raster_image->colorType(), raster_image->alphaType()); - case kRawExtendedRgba128: - return CopyImageByteData(raster_image, kRGBA_F32_SkColorType, - kUnpremul_SkAlphaType); + } break; } 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 d87a81b791232..ec429a2cda90f 100644 --- a/lib/ui/painting/image_encoding_impeller.cc +++ b/lib/ui/painting/image_encoding_impeller.cc @@ -163,16 +163,4 @@ 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 924b18e4eb989..fafe8fc9992b4 100644 --- a/lib/ui/painting/image_encoding_impeller.h +++ b/lib/ui/painting/image_encoding_impeller.h @@ -17,8 +17,6 @@ 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 d15d83fc4b926..0ec5a827e70c6 100644 --- a/lib/web_ui/lib/painting.dart +++ b/lib/web_ui/lib/painting.dart @@ -351,8 +351,6 @@ abstract class Image { List? debugGetOpenHandleStackTraces() => null; - ColorSpace get colorSpace => ColorSpace.sRGB; - @override String toString() => '[$width\u00D7$height]'; } @@ -433,11 +431,6 @@ 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 e43157da53138..220bfccfd2c29 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/image.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/image.dart @@ -378,9 +378,6 @@ 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 f1f184b527c26..d5db33fcbffc1 100644 --- a/lib/web_ui/lib/src/engine/html_image_codec.dart +++ b/lib/web_ui/lib/src/engine/html_image_codec.dart @@ -204,9 +204,6 @@ 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 a019591f3dde3..3ca0b738b14e8 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,9 +23,6 @@ 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 b032c45ccc0e0..eaad748b4b8de 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' hide ColorSpace; +import 'package:ui/src/engine.dart'; import 'package:ui/ui.dart' hide TextStyle; import 'package:web_engine_tester/golden_tester.dart'; @@ -780,9 +780,6 @@ 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 4c18bf4957688..79176c52f6b43 100644 --- a/testing/dart/encoding_test.dart +++ b/testing/dart/encoding_test.dart @@ -67,21 +67,6 @@ 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 {