Skip to content

Commit

Permalink
ImageBitmap: Serialize SkColorSpace
Browse files Browse the repository at this point in the history 
When serializing an ImageBitmap, we coerce its SkColorSpace into a
PredefinedColorSpace enum.

This has historically not caused too many problems, because loading an
image using the ImageBitmap API would automatically convert the image
to sRGB. This bug is being fixed, and the conversion is delayed until
needed (if ever).

As a consequence of fixing the image color space bug, many diverse
SkColorSpaces are being put into ImageBitmaps. We need to be able to
accurately serialize these spaces when serializing the ImageBitmaps.

Bug: 1310811
Change-Id: I530d4268d45c548524c57554fc1cfbd70f95e9fa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3562229
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: ccameron chromium <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988161}
  • Loading branch information
@ccameron-chromium
ccameron-chromium authored and Chromium LUCI CQ committed Apr 1, 2022
1 parent 60a7d99 commit 0dd688f
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 40 deletions.
85 changes: 64 additions & 21 deletions third_party/blink/renderer/bindings/core/v8/serialization/serialized_color_params.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,40 +11,41 @@ namespace blink {

namespace {

SerializedColorSpace SerializeColorSpace(PredefinedColorSpace color_space) {
SerializedPredefinedColorSpace SerializeColorSpace(
PredefinedColorSpace color_space) {
switch (color_space) {
case PredefinedColorSpace::kSRGB:
return SerializedColorSpace::kSRGB;
return SerializedPredefinedColorSpace::kSRGB;
case PredefinedColorSpace::kRec2020:
return SerializedColorSpace::kRec2020;
return SerializedPredefinedColorSpace::kRec2020;
case PredefinedColorSpace::kP3:
return SerializedColorSpace::kP3;
return SerializedPredefinedColorSpace::kP3;
case PredefinedColorSpace::kRec2100HLG:
return SerializedColorSpace::kRec2100HLG;
return SerializedPredefinedColorSpace::kRec2100HLG;
case PredefinedColorSpace::kRec2100PQ:
return SerializedColorSpace::kRec2100PQ;
return SerializedPredefinedColorSpace::kRec2100PQ;
case PredefinedColorSpace::kSRGBLinear:
return SerializedColorSpace::kSRGBLinear;
return SerializedPredefinedColorSpace::kSRGBLinear;
}
NOTREACHED();
return SerializedColorSpace::kSRGB;
return SerializedPredefinedColorSpace::kSRGB;
}

PredefinedColorSpace DeserializeColorSpace(
SerializedColorSpace serialized_color_space) {
SerializedPredefinedColorSpace serialized_color_space) {
switch (serialized_color_space) {
case SerializedColorSpace::kLegacyObsolete:
case SerializedColorSpace::kSRGB:
case SerializedPredefinedColorSpace::kLegacyObsolete:
case SerializedPredefinedColorSpace::kSRGB:
return PredefinedColorSpace::kSRGB;
case SerializedColorSpace::kRec2020:
case SerializedPredefinedColorSpace::kRec2020:
return PredefinedColorSpace::kRec2020;
case SerializedColorSpace::kP3:
case SerializedPredefinedColorSpace::kP3:
return PredefinedColorSpace::kP3;
case SerializedColorSpace::kRec2100HLG:
case SerializedPredefinedColorSpace::kRec2100HLG:
return PredefinedColorSpace::kRec2100HLG;
case SerializedColorSpace::kRec2100PQ:
case SerializedPredefinedColorSpace::kRec2100PQ:
return PredefinedColorSpace::kRec2100PQ;
case SerializedColorSpace::kSRGBLinear:
case SerializedPredefinedColorSpace::kSRGBLinear:
return PredefinedColorSpace::kSRGBLinear;
}
NOTREACHED();
Expand Down Expand Up @@ -74,7 +75,7 @@ SerializedImageDataSettings::SerializedImageDataSettings(
}

SerializedImageDataSettings::SerializedImageDataSettings(
SerializedColorSpace color_space,
SerializedPredefinedColorSpace color_space,
SerializedImageDataStorageFormat storage_format)
: color_space_(color_space), storage_format_(storage_format) {}

Expand Down Expand Up @@ -107,9 +108,33 @@ ImageDataSettings* SerializedImageDataSettings::GetImageDataSettings() const {

SerializedImageBitmapSettings::SerializedImageBitmapSettings() = default;

SerializedImageBitmapSettings::SerializedImageBitmapSettings(SkImageInfo info) {
color_space_ = SerializeColorSpace(
PredefinedColorSpaceFromSkColorSpace(info.colorSpace()));
SerializedImageBitmapSettings::SerializedImageBitmapSettings(SkImageInfo info)
: sk_color_space_(kSerializedParametricColorSpaceLength) {
auto color_space =
info.colorSpace() ? info.refColorSpace() : SkColorSpace::MakeSRGB();
skcms_TransferFunction trfn = {};
skcms_Matrix3x3 to_xyz = {};
// The return value of `isNumericalTransferFn` is false for HLG and PQ
// transfer functions, but `trfn` is still populated appropriately. DCHECK
// that the constants for HLG and PQ have not changed.
color_space->isNumericalTransferFn(&trfn);
if (skcms_TransferFunction_isPQish(&trfn))
DCHECK_EQ(trfn.g, kSerializedPQConstant);
if (skcms_TransferFunction_isHLGish(&trfn))
DCHECK_EQ(trfn.g, kSerializedHLGConstant);
bool to_xyzd50_result = color_space->toXYZD50(&to_xyz);
DCHECK(to_xyzd50_result);
sk_color_space_.resize(16);
sk_color_space_[0] = trfn.g;
sk_color_space_[1] = trfn.a;
sk_color_space_[2] = trfn.b;
sk_color_space_[3] = trfn.c;
sk_color_space_[4] = trfn.d;
sk_color_space_[5] = trfn.e;
sk_color_space_[6] = trfn.f;
for (uint32_t i = 0; i < 3; ++i)
for (uint32_t j = 0; j < 3; ++j)
sk_color_space_[7 + 3 * i + j] = to_xyz.vals[i][j];

switch (info.colorType()) {
default:
Expand Down Expand Up @@ -145,11 +170,13 @@ SerializedImageBitmapSettings::SerializedImageBitmapSettings(SkImageInfo info) {
}

SerializedImageBitmapSettings::SerializedImageBitmapSettings(
SerializedColorSpace color_space,
SerializedPredefinedColorSpace color_space,
const Vector<double>& sk_color_space,
SerializedPixelFormat pixel_format,
SerializedOpacityMode opacity_mode,
uint32_t is_premultiplied)
: color_space_(color_space),
sk_color_space_(sk_color_space),
pixel_format_(pixel_format),
opacity_mode_(opacity_mode),
is_premultiplied_(is_premultiplied) {}
Expand All @@ -160,6 +187,22 @@ SkImageInfo SerializedImageBitmapSettings::GetSkImageInfo(
sk_sp<SkColorSpace> sk_color_space =
PredefinedColorSpaceToSkColorSpace(DeserializeColorSpace(color_space_));

if (sk_color_space_.size() == kSerializedParametricColorSpaceLength) {
skcms_TransferFunction trfn;
skcms_Matrix3x3 to_xyz;
trfn.g = static_cast<float>(sk_color_space_[0]);
trfn.a = static_cast<float>(sk_color_space_[1]);
trfn.b = static_cast<float>(sk_color_space_[2]);
trfn.c = static_cast<float>(sk_color_space_[3]);
trfn.d = static_cast<float>(sk_color_space_[4]);
trfn.e = static_cast<float>(sk_color_space_[5]);
trfn.f = static_cast<float>(sk_color_space_[6]);
for (uint32_t i = 0; i < 3; ++i)
for (uint32_t j = 0; j < 3; ++j)
to_xyz.vals[i][j] = static_cast<float>(sk_color_space_[7 + 3 * i + j]);
sk_color_space = SkColorSpace::MakeRGB(trfn, to_xyz);
}

SkColorType sk_color_type = kRGBA_8888_SkColorType;
switch (pixel_format_) {
case SerializedPixelFormat::kNative8_LegacyObsolete:
Expand Down
44 changes: 34 additions & 10 deletions third_party/blink/renderer/bindings/core/v8/serialization/serialized_color_params.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@ namespace blink {
// of the serialized/deserialized ImageData and ImageBitmap objects.
enum class ImageSerializationTag : uint32_t {
kEndTag = 0,
// followed by a SerializedColorSpace enum.
// followed by a SerializedPredefinedColorSpace enum. This is used for
// ImageData, and only for backwards compatibility for ImageBitmap. For
// ImageBitmap, the tag kParametricColorSpaceTag should be used (and
// overrides kPredefinedColorSpaceTag if both are are specified, although
// both should not be specified).
kPredefinedColorSpaceTag = 1,
// followed by a SerializedPixelFormat enum, used only for ImageBitmap.
kCanvasPixelFormatTag = 2,
Expand All @@ -27,12 +31,22 @@ enum class ImageSerializationTag : uint32_t {
kIsPremultipliedTag = 5,
// followed by 1 if the image is known to be opaque (alpha = 1 everywhere)
kCanvasOpacityModeTag = 6,

kLast = kCanvasOpacityModeTag,
// followed by 16 doubles, for a parametric color space definition. The first
// 7 doubles are the parameters g,a,b,c,d,e,f, in that order, for a transfer
// function to convert encoded to linear space as
// f(x) = c*x + f : 0 <= x < d
// = (a*x + b)^g + e : d <= x
// The PQ transfer function is indicated by g=-2 and the HLG transfer
// function is indicated by g=-3. The next 9 doubles are a 3x3 matrix in
// row-major order that converts an r,g,b triple in the linear color space to
// x,y,z in the XYZ D50 color space.
kParametricColorSpaceTag = 7,

kLast = kParametricColorSpaceTag,
};

// This enumeration specifies the values used to serialize PredefinedColorSpace.
enum class SerializedColorSpace : uint32_t {
enum class SerializedPredefinedColorSpace : uint32_t {
// Legacy sRGB color space is deprecated as of M65. Objects in legacy color
// space will be serialized as sRGB since the legacy behavior is now merged
// with sRGB color space. Deserialized objects with legacy color space also
Expand Down Expand Up @@ -78,36 +92,44 @@ enum class SerializedOpacityMode : uint32_t {
class SerializedImageDataSettings {
public:
SerializedImageDataSettings(PredefinedColorSpace, ImageDataStorageFormat);
SerializedImageDataSettings(SerializedColorSpace,
SerializedImageDataSettings(SerializedPredefinedColorSpace,
SerializedImageDataStorageFormat);

PredefinedColorSpace GetColorSpace() const;
ImageDataStorageFormat GetStorageFormat() const;
ImageDataSettings* GetImageDataSettings() const;

SerializedColorSpace GetSerializedColorSpace() const { return color_space_; }
SerializedPredefinedColorSpace GetSerializedPredefinedColorSpace() const {
return color_space_;
}
SerializedImageDataStorageFormat GetSerializedImageDataStorageFormat() const {
return storage_format_;
}

private:
SerializedColorSpace color_space_ = SerializedColorSpace::kSRGB;
SerializedPredefinedColorSpace color_space_ =
SerializedPredefinedColorSpace::kSRGB;
SerializedImageDataStorageFormat storage_format_ =
SerializedImageDataStorageFormat::kUint8Clamped;
};

constexpr uint32_t kSerializedParametricColorSpaceLength = 16;
constexpr float kSerializedPQConstant = -2.f;
constexpr float kSerializedHLGConstant = -3.f;

class SerializedImageBitmapSettings {
public:
SerializedImageBitmapSettings();
explicit SerializedImageBitmapSettings(SkImageInfo);
SerializedImageBitmapSettings(SerializedColorSpace,
SerializedImageBitmapSettings(SerializedPredefinedColorSpace,
const Vector<double>& sk_color_space,
SerializedPixelFormat,
SerializedOpacityMode,
uint32_t is_premultiplied);

SkImageInfo GetSkImageInfo(uint32_t width, uint32_t height) const;

SerializedColorSpace GetSerializedColorSpace() const { return color_space_; }
const Vector<double>& GetSerializedSkColorSpace() { return sk_color_space_; }
SerializedPixelFormat GetSerializedPixelFormat() const {
return pixel_format_;
}
Expand All @@ -117,7 +139,9 @@ class SerializedImageBitmapSettings {
uint32_t IsPremultiplied() const { return is_premultiplied_; }

private:
SerializedColorSpace color_space_ = SerializedColorSpace::kSRGB;
SerializedPredefinedColorSpace color_space_ =
SerializedPredefinedColorSpace::kSRGB;
Vector<double> sk_color_space_;
SerializedPixelFormat pixel_format_ = SerializedPixelFormat::kRGBA8;
SerializedOpacityMode opacity_mode_ = SerializedOpacityMode::kNonOpaque;
bool is_premultiplied_ = true;
Expand Down
27 changes: 21 additions & 6 deletions third_party/blink/renderer/bindings/core/v8/serialization/v8_script_value_deserializer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,9 @@ ScriptWrappable* V8ScriptValueDeserializer::ReadDOMObject(
return file_list;
}
case kImageBitmapTag: {
SerializedColorSpace canvas_color_space = SerializedColorSpace::kSRGB;
SerializedPredefinedColorSpace predefined_color_space =
SerializedPredefinedColorSpace::kSRGB;
Vector<double> sk_color_space;
SerializedPixelFormat canvas_pixel_format =
SerializedPixelFormat::kNative8_LegacyObsolete;
SerializedOpacityMode canvas_opacity_mode =
Expand All @@ -336,8 +338,17 @@ ScriptWrappable* V8ScriptValueDeserializer::ReadDOMObject(
is_done = true;
break;
case ImageSerializationTag::kPredefinedColorSpaceTag:
if (!ReadUint32Enum<SerializedColorSpace>(&canvas_color_space))
if (!ReadUint32Enum<SerializedPredefinedColorSpace>(
&predefined_color_space)) {
return nullptr;
}
break;
case ImageSerializationTag::kParametricColorSpaceTag:
sk_color_space.resize(kSerializedParametricColorSpaceLength);
for (double& value : sk_color_space) {
if (!ReadDouble(&value))
return nullptr;
}
break;
case ImageSerializationTag::kCanvasPixelFormatTag:
if (!ReadUint32Enum<SerializedPixelFormat>(&canvas_pixel_format))
Expand Down Expand Up @@ -368,7 +379,8 @@ ScriptWrappable* V8ScriptValueDeserializer::ReadDOMObject(
!ReadUint32(&byte_length) || !ReadRawBytes(byte_length, &pixels))
return nullptr;
SkImageInfo info =
SerializedImageBitmapSettings(canvas_color_space, canvas_pixel_format,
SerializedImageBitmapSettings(predefined_color_space, sk_color_space,
canvas_pixel_format,
canvas_opacity_mode, is_premultiplied)
.GetSkImageInfo(width, height);
base::CheckedNumeric<uint32_t> computed_byte_length =
Expand All @@ -394,7 +406,8 @@ ScriptWrappable* V8ScriptValueDeserializer::ReadDOMObject(
return transferred_image_bitmaps[index].Get();
}
case kImageDataTag: {
SerializedColorSpace canvas_color_space = SerializedColorSpace::kSRGB;
SerializedPredefinedColorSpace predefined_color_space =
SerializedPredefinedColorSpace::kSRGB;
SerializedImageDataStorageFormat image_data_storage_format =
SerializedImageDataStorageFormat::kUint8Clamped;
uint32_t width = 0, height = 0;
Expand All @@ -410,7 +423,8 @@ ScriptWrappable* V8ScriptValueDeserializer::ReadDOMObject(
is_done = true;
break;
case ImageSerializationTag::kPredefinedColorSpaceTag:
if (!ReadUint32Enum<SerializedColorSpace>(&canvas_color_space))
if (!ReadUint32Enum<SerializedPredefinedColorSpace>(
&predefined_color_space))
return nullptr;
break;
case ImageSerializationTag::kImageDataStorageFormatTag:
Expand All @@ -422,6 +436,7 @@ ScriptWrappable* V8ScriptValueDeserializer::ReadDOMObject(
case ImageSerializationTag::kOriginCleanTag:
case ImageSerializationTag::kIsPremultipliedTag:
case ImageSerializationTag::kCanvasOpacityModeTag:
case ImageSerializationTag::kParametricColorSpaceTag:
// Does not apply to ImageData.
return nullptr;
}
Expand All @@ -437,7 +452,7 @@ ScriptWrappable* V8ScriptValueDeserializer::ReadDOMObject(
return nullptr;
}

SerializedImageDataSettings settings(canvas_color_space,
SerializedImageDataSettings settings(predefined_color_space,
image_data_storage_format);
ImageData* image_data = ImageData::ValidateAndCreate(
width, height, absl::nullopt, settings.GetImageDataSettings(),
Expand Down
9 changes: 6 additions & 3 deletions third_party/blink/renderer/bindings/core/v8/serialization/v8_script_value_serializer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -466,8 +466,11 @@ bool V8ScriptValueSerializer::WriteDOMObject(ScriptWrappable* wrappable,
WriteTag(kImageBitmapTag);
SkImageInfo info = image_bitmap->GetBitmapSkImageInfo();
SerializedImageBitmapSettings color_params(info);
WriteUint32Enum(ImageSerializationTag::kPredefinedColorSpaceTag);
WriteUint32Enum(color_params.GetSerializedColorSpace());
WriteUint32Enum(ImageSerializationTag::kParametricColorSpaceTag);
DCHECK_EQ(color_params.GetSerializedSkColorSpace().size(),
kSerializedParametricColorSpaceLength);
for (const auto& value : color_params.GetSerializedSkColorSpace())
WriteDouble(value);
WriteUint32Enum(ImageSerializationTag::kCanvasPixelFormatTag);
WriteUint32Enum(color_params.GetSerializedPixelFormat());
WriteUint32Enum(ImageSerializationTag::kCanvasOpacityModeTag);
Expand Down Expand Up @@ -499,7 +502,7 @@ bool V8ScriptValueSerializer::WriteDOMObject(ScriptWrappable* wrappable,
image_data->GetPredefinedColorSpace(),
image_data->GetImageDataStorageFormat());
WriteUint32Enum(ImageSerializationTag::kPredefinedColorSpaceTag);
WriteUint32Enum(settings.GetSerializedColorSpace());
WriteUint32Enum(settings.GetSerializedPredefinedColorSpace());
WriteUint32Enum(ImageSerializationTag::kImageDataStorageFormatTag);
WriteUint32Enum(settings.GetSerializedImageDataStorageFormat());
WriteUint32Enum(ImageSerializationTag::kEndTag);
Expand Down

0 comments on commit 0dd688f

Please sign in to comment.