Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Added wide-gamut color support for ui.Image.toByteData and ui.Image.colorSpace #40031

Merged
merged 20 commits into from
Mar 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/ui/dart_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) \
Expand Down
70 changes: 70 additions & 0 deletions lib/ui/painting.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// should use something like [ImageByteFormat.rawExtendedRgba128] so that
/// should use something like [ImageByteFormat.rawExtendedRgba128] so that

Nit, use stronger phrasing. Consider something like:

"An image in the extendedSRGB color space must use ImageByteFormat.rawExtendedRgba128 to avoid losing colors. Using ImageByteFormat blah blah will cause loss of color information" And define what loss of color information means if its a well defined process

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, reworded and I think that information would be better to be situated at toByteData so I added it there.

/// 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 @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is straight alpha premultiplied or un-premultiplied (post multiplied)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's un-premultiplied, "straight" is the nomenclature we use in the other fields.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this matches the encoding of the dart:ui Color class, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dart:ui Color class is using integer values for components, this is float32.

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

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 @@ -37,6 +43,8 @@ class CanvasImage final : public RefCountedDartWrappable<CanvasImage> {

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

int colorSpace();

private:
CanvasImage();

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

Expand Down Expand Up @@ -123,19 +124,21 @@ sk_sp<SkData> EncodeImage(const sk_sp<SkImage>& 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.";
Expand Down
12 changes: 12 additions & 0 deletions lib/ui/painting/image_encoding_impeller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -163,4 +163,16 @@ 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: 2 additions & 0 deletions lib/ui/painting/image_encoding_impeller.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ 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: 7 additions & 0 deletions lib/web_ui/lib/painting.dart
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,8 @@ abstract class Image {

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

ColorSpace get colorSpace => ColorSpace.sRGB;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like all concrete subclasses are implementing this getter. I think this means that the getter in the base class can be abstract.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason all of the other places had to implement it themselves is because they are using implements instead of extends. I don't know if there was anyone that was using extends, but if they were, I wouldn't want them to have to implement this property.


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

enum ColorSpace {
sRGB,
extendedSRGB,
}

enum ImageByteFormat {
rawRgba,
rawStraightRgba,
Expand Down
3 changes: 3 additions & 0 deletions lib/web_ui/lib/src/engine/canvaskit/image.dart
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,9 @@ 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: 3 additions & 0 deletions lib/web_ui/lib/src/engine/html_image_codec.dart
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,9 @@ class HtmlImage implements ui.Image {
}
}

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

DomHTMLImageElement cloneImageElement() {
if (!_didClone) {
_didClone = true;
Expand Down
3 changes: 3 additions & 0 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,6 +23,9 @@ class SkwasmImage implements ui.Image {
throw UnimplementedError();
}

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

@override
void dispose() {
throw UnimplementedError();
Expand Down
5 changes: 4 additions & 1 deletion 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';
import 'package:ui/src/engine.dart' hide ColorSpace;
import 'package:ui/ui.dart' hide TextStyle;
import 'package:web_engine_tester/golden_tester.dart';

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

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

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

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