Skip to content

Commit

Permalink
Revert "Added wide-gamut color support for ui.Image.toByteData and …
Browse files Browse the repository at this point in the history
…`ui.Image.colorSpace` (#40031)" (#40295)

Revert "Added wide-gamut color support for `ui.Image.toByteData` and `ui.Image.colorSpace`"
  • Loading branch information
Casey Hillers authored Mar 15, 2023
1 parent 620431d commit d9770af
Show file tree
Hide file tree
Showing 13 changed files with 8 additions and 153 deletions.
1 change: 0 additions & 1 deletion lib/ui/dart_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) \
Expand Down
70 changes: 0 additions & 70 deletions lib/ui/painting.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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.
///
Expand Down Expand Up @@ -1970,9 +1903,6 @@ class _Image extends NativeFieldWrapperClass1 {

final Set<Image> _handles = <Image>{};

@Native<Int32 Function(Pointer<Void>)>(symbol: 'Image::colorSpace')
external int get colorSpace;

@override
String toString() => '[$width\u00D7$height]';
}
Expand Down
15 changes: 0 additions & 15 deletions lib/ui/painting/image.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@
#include <algorithm>
#include <limits>

#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"
Expand Down Expand Up @@ -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
8 changes: 0 additions & 8 deletions lib/ui/painting/image.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,6 @@

namespace flutter {

// Must be kept in sync with painting.dart.
enum ColorSpace {
kSRGB,
kExtendedSRGB,
};

class CanvasImage final : public RefCountedDartWrappable<CanvasImage> {
DEFINE_WRAPPERTYPEINFO();
FML_FRIEND_MAKE_REF_COUNTED(CanvasImage);
Expand All @@ -43,8 +37,6 @@ class CanvasImage final : public RefCountedDartWrappable<CanvasImage> {

void set_image(sk_sp<DlImage> image) { image_ = image; }

int colorSpace();

private:
CanvasImage();

Expand Down
17 changes: 7 additions & 10 deletions lib/ui/painting/image_encoding.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ enum ImageByteFormat {
kRawRGBA,
kRawStraightRGBA,
kRawUnmodified,
kRawExtendedRgba128,
kPNG,
};

Expand Down Expand Up @@ -124,21 +123,19 @@ sk_sp<SkData> EncodeImage(const sk_sp<SkImage>& 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.";
Expand Down
12 changes: 0 additions & 12 deletions lib/ui/painting/image_encoding_impeller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -163,16 +163,4 @@ void ImageEncodingImpeller::ConvertImageToRaster(
});
}

int ImageEncodingImpeller::GetColorSpace(
const std::shared_ptr<impeller::Texture>& 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
2 changes: 0 additions & 2 deletions lib/ui/painting/image_encoding_impeller.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ namespace flutter {

class ImageEncodingImpeller {
public:
static int GetColorSpace(const std::shared_ptr<impeller::Texture>& 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.
Expand Down
7 changes: 0 additions & 7 deletions lib/web_ui/lib/painting.dart
Original file line number Diff line number Diff line change
Expand Up @@ -351,8 +351,6 @@ abstract class Image {

List<StackTrace>? debugGetOpenHandleStackTraces() => null;

ColorSpace get colorSpace => ColorSpace.sRGB;

@override
String toString() => '[$width\u00D7$height]';
}
Expand Down Expand Up @@ -433,11 +431,6 @@ class ImageFilter {
engine.renderer.composeImageFilters(outer: outer, inner: inner);
}

enum ColorSpace {
sRGB,
extendedSRGB,
}

enum ImageByteFormat {
rawRgba,
rawStraightRgba,
Expand Down
3 changes: 0 additions & 3 deletions lib/web_ui/lib/src/engine/canvaskit/image.dart
Original file line number Diff line number Diff line change
Expand Up @@ -378,9 +378,6 @@ class CkImage implements ui.Image, StackTraceDebugger {
}
}

@override
ui.ColorSpace get colorSpace => ui.ColorSpace.sRGB;

Future<ByteData> _readPixelsFromSkImage(ui.ImageByteFormat format) {
final SkAlphaType alphaType = format == ui.ImageByteFormat.rawStraightRgba ? canvasKit.AlphaType.Unpremul : canvasKit.AlphaType.Premul;
final ByteData? data = _encodeImage(
Expand Down
3 changes: 0 additions & 3 deletions lib/web_ui/lib/src/engine/html_image_codec.dart
Original file line number Diff line number Diff line change
Expand Up @@ -204,9 +204,6 @@ class HtmlImage implements ui.Image {
}
}

@override
ui.ColorSpace get colorSpace => ui.ColorSpace.sRGB;

DomHTMLImageElement cloneImageElement() {
if (!_didClone) {
_didClone = true;
Expand Down
3 changes: 0 additions & 3 deletions lib/web_ui/lib/src/engine/skwasm/skwasm_impl/image.dart
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@ class SkwasmImage implements ui.Image {
throw UnimplementedError();
}

@override
ui.ColorSpace get colorSpace => ui.ColorSpace.sRGB;

@override
void dispose() {
throw UnimplementedError();
Expand Down
5 changes: 1 addition & 4 deletions lib/web_ui/test/html/recording_canvas_golden_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -780,9 +780,6 @@ class TestImage implements Image {

@override
List<StackTrace>/*?*/ debugGetOpenHandleStackTraces() => <StackTrace>[];

@override
ColorSpace get colorSpace => ColorSpace.sRGB;
}

Paragraph createTestParagraph() {
Expand Down
15 changes: 0 additions & 15 deletions testing/dart/encoding_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -67,21 +67,6 @@ void main() {
final List<int> 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 {
Expand Down

0 comments on commit d9770af

Please sign in to comment.