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

Added float32 support to decodeImageFromPixels #40068

Merged
merged 4 commits into from
Mar 15, 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
10 changes: 8 additions & 2 deletions lib/ui/painting.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1653,13 +1653,19 @@ enum ImageByteFormat {
enum PixelFormat {
/// Each pixel is 32 bits, with the highest 8 bits encoding red, the next 8
/// bits encoding green, the next 8 bits encoding blue, and the lowest 8 bits
/// encoding alpha.
/// encoding alpha. Premultiplied alpha is used.
rgba8888,

/// Each pixel is 32 bits, with the highest 8 bits encoding blue, the next 8
/// bits encoding green, the next 8 bits encoding red, and the lowest 8 bits
/// encoding alpha.
/// encoding alpha. Premultiplied alpha is used.
bgra8888,

/// Each pixel is 128 bits, where each color component is a 32 bit float that
/// is normalized across the sRGB gamut. The first float is the red
/// component, followed by: green, blue and alpha. Premultiplied alpha isn't
/// used, matching [ImageByteFormat.rawExtendedRgba128].
rgbaFloat32,
}

/// Signature for [Image] lifecycle events.
Expand Down
23 changes: 21 additions & 2 deletions lib/ui/painting/image_decoder_impeller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,12 @@ ImageDecoderImpeller::ImageDecoderImpeller(
ImageDecoderImpeller::~ImageDecoderImpeller() = default;

static SkColorType ChooseCompatibleColorType(SkColorType type) {
return kRGBA_8888_SkColorType;
switch (type) {
case kRGBA_F32_SkColorType:
return kRGBA_F16_SkColorType;
Comment on lines +90 to +91
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not

Suggested change
case kRGBA_F32_SkColorType:
return kRGBA_F16_SkColorType;
case kRGBA_F32_SkColorType:
case kRGBA_F16_SkColorType:
return kRGBA_F16_SkColorType;

Copy link
Member Author

Choose a reason for hiding this comment

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

There isn't a way to send in kRGBA_F16_SkColorType directly. If we decide to implement that it will probably be better to do that in its own pr with its own tests.

default:
return kRGBA_8888_SkColorType;
}
}

static SkAlphaType ChooseCompatibleAlphaType(SkAlphaType type) {
Expand Down Expand Up @@ -176,12 +181,26 @@ std::shared_ptr<SkBitmap> ImageDecoderImpeller::DecompressTexture(
FML_DLOG(ERROR) << "Could not decompress image.";
return nullptr;
}
} else {
} else if (image_info.colorType() == base_image_info.colorType()) {
bitmap->setInfo(image_info);
auto pixel_ref = SkMallocPixelRef::MakeWithData(
image_info, descriptor->row_bytes(), descriptor->data());
bitmap->setPixelRef(pixel_ref, 0, 0);
bitmap->setImmutable();
} else {
auto temp_bitmap = std::make_shared<SkBitmap>();
temp_bitmap->setInfo(base_image_info);
auto pixel_ref = SkMallocPixelRef::MakeWithData(
base_image_info, descriptor->row_bytes(), descriptor->data());
temp_bitmap->setPixelRef(pixel_ref, 0, 0);

if (!bitmap->tryAllocPixels(image_info)) {
FML_DLOG(ERROR)
<< "Could not allocate intermediate for pixel conversion.";
return nullptr;
}
temp_bitmap->readPixels(bitmap->pixmap());
bitmap->setImmutable();
}

if (bitmap->dimensions() == target_size) {
Expand Down
25 changes: 25 additions & 0 deletions lib/ui/painting/image_decoder_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,31 @@ TEST_F(ImageDecoderFixtureTest, ImpellerWideGamutDisplayP3) {
#endif // IMPELLER_SUPPORTS_RENDERING
}

TEST_F(ImageDecoderFixtureTest, ImpellerPixelConversion32F) {
auto info = SkImageInfo::Make(10, 10, SkColorType::kRGBA_F32_SkColorType,
SkAlphaType::kUnpremul_SkAlphaType);
SkBitmap bitmap;
bitmap.allocPixels(info, 10 * 16);
auto data = SkData::MakeWithoutCopy(bitmap.getPixels(), 10 * 10 * 16);
auto image = SkImage::MakeFromBitmap(bitmap);
ASSERT_TRUE(image != nullptr);
ASSERT_EQ(SkISize::Make(10, 10), image->dimensions());
ASSERT_EQ(nullptr, image->colorSpace());

auto descriptor = fml::MakeRefCounted<ImageDescriptor>(
std::move(data), image->imageInfo(), 10 * 16);

#if IMPELLER_SUPPORTS_RENDERING
std::shared_ptr<SkBitmap> decompressed =
ImageDecoderImpeller::DecompressTexture(
descriptor.get(), SkISize::Make(100, 100), {100, 100},
/*supports_wide_gamut=*/true);
ASSERT_TRUE(decompressed);
ASSERT_EQ(decompressed->colorType(), kRGBA_F16_SkColorType);
ASSERT_EQ(decompressed->colorSpace(), nullptr);
#endif // IMPELLER_SUPPORTS_RENDERING
}

namespace {
float DecodeBGR10(uint32_t x) {
const float max = 1.25098f;
Expand Down
9 changes: 7 additions & 2 deletions lib/ui/painting/image_descriptor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,17 +85,22 @@ void ImageDescriptor::initRaw(Dart_Handle descriptor_handle,
int row_bytes,
PixelFormat pixel_format) {
SkColorType color_type = kUnknown_SkColorType;
SkAlphaType alpha_type = kPremul_SkAlphaType;
switch (pixel_format) {
case PixelFormat::kRGBA8888:
color_type = kRGBA_8888_SkColorType;
break;
case PixelFormat::kBGRA8888:
color_type = kBGRA_8888_SkColorType;
break;
case PixelFormat::kRGBAFloat32:
// `PixelFormat.rgbaFloat32` is documented to not use premultiplied alpha.
color_type = kRGBA_F32_SkColorType;
alpha_type = kUnpremul_SkAlphaType;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are these unpremul but the other ones are premul?

Copy link
Member Author

Choose a reason for hiding this comment

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

From the Dart API perspective it's not the only one that uses unpremultiplied, rawStraightRgba also returns unpremultiplied alpha. I'm not sure what flow of the code that goes through to achieve it, but I'm maintaining the old behavior here for old flows. I chose to make this unpremultiplied since that is the most expected result and the benefits of using premultiplied alpha don't exist here since we have to do a conversion anyway because we can't use f32 directly, we have to convert to f16.

Copy link
Contributor

Choose a reason for hiding this comment

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

Consider adding a comment about that.

Copy link
Member Author

Choose a reason for hiding this comment

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

I got a bit confused between toByteData and decodeImageFromPixels. The input to toByteData was documented as non-premultiplied, but the input to decodeImageFromPixels wasn't. I've updated all the documentation for ui.PixelFormat to be explicit about pixel format. I also added a comment in this code. The important thing is I wanted ImageByteFormat.rawExtendedRgba128 and PixelFormat.rgbaFloat32 to match so people can easily download, tweak, then reupload.

break;
}
FML_DCHECK(color_type != kUnknown_SkColorType);
auto image_info =
SkImageInfo::Make(width, height, color_type, kPremul_SkAlphaType);
auto image_info = SkImageInfo::Make(width, height, color_type, alpha_type);
auto descriptor = fml::MakeRefCounted<ImageDescriptor>(
data->data(), std::move(image_info),
row_bytes == -1 ? std::nullopt : std::optional<size_t>(row_bytes));
Expand Down
1 change: 1 addition & 0 deletions lib/ui/painting/image_descriptor.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ class ImageDescriptor : public RefCountedDartWrappable<ImageDescriptor> {
enum PixelFormat {
kRGBA8888,
kBGRA8888,
kRGBAFloat32,
};

/// @brief Asynchronously initializes an ImageDescriptor for an encoded
Expand Down
37 changes: 37 additions & 0 deletions testing/dart/encoding_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,43 @@ const Color _kBlack = Color.fromRGBO(0, 0, 0, 1.0);
const Color _kGreen = Color.fromRGBO(0, 255, 0, 1.0);

void main() {
test('decodeImageFromPixels float32', () async {
const int width = 2;
const int height = 2;
final Float32List pixels = Float32List(width * height * 4);
final List<List<double>> pixels2d = <List<double>>[
<double>[1, 0, 0, 1],
<double>[0, 1, 0, 1],
<double>[0, 0, 1, 1],
<double>[1, 1, 1, 0],
];
int offset = 0;
for (final List<double> color in pixels2d) {
pixels[offset + 0] = color[0];
pixels[offset + 1] = color[1];
pixels[offset + 2] = color[2];
pixels[offset + 3] = color[3];
offset += 4;
}

final Completer<Image> completer = Completer<Image>();
decodeImageFromPixels(
Uint8List.view(pixels.buffer), width, height, PixelFormat.rgbaFloat32,
(Image result) {
completer.complete(result);
});

final Image image = await completer.future;
final ByteData data =
(await image.toByteData(format: ImageByteFormat.rawStraightRgba))!;
final Uint32List readPixels = Uint32List.view(data.buffer);
expect(width * height, readPixels.length);
expect(readPixels[0], 0xff0000ff);
expect(readPixels[1], 0xff00ff00);
expect(readPixels[2], 0xffff0000);
expect(readPixels[3], 0x00ffffff);
});

test('Image.toByteData RGBA format works with simple image', () async {
final Image image = await Square4x4Image.image;
final ByteData data = (await image.toByteData())!;
Expand Down