Index: ui/gfx/color_space.cc |
diff --git a/ui/gfx/color_space.cc b/ui/gfx/color_space.cc |
index 0f5a42b601841ca529d42c456df504512c708914..3c7b294e4446ad4e10004ab1a5e16f2d34764909 100644 |
--- a/ui/gfx/color_space.cc |
+++ b/ui/gfx/color_space.cc |
@@ -86,7 +86,9 @@ ColorSpace::ColorSpace(const ColorSpace& other) |
ColorSpace::~ColorSpace() = default; |
bool ColorSpace::IsValid() const { |
- return *this != gfx::ColorSpace(); |
+ return primaries_ != PrimaryID::UNSPECIFIED && |
+ transfer_ != TransferID::UNSPECIFIED && |
+ matrix_ != MatrixID::UNSPECIFIED; |
hubbe
2017/02/15 08:48:56
What if a video specifies the range but nothing el
ccameron
2017/02/15 09:34:28
Errr... I meant to put UNKNOWN here.
So, UNSPECIF
|
} |
// static |
@@ -96,6 +98,27 @@ ColorSpace ColorSpace::CreateSRGB() { |
} |
// static |
+ColorSpace ColorSpace::CreateCustom(const SkMatrix44& to_XYZD50, |
+ const SkColorSpaceTransferFn& fn) { |
+ ColorSpace result(ColorSpace::PrimaryID::CUSTOM, |
+ ColorSpace::TransferID::CUSTOM, ColorSpace::MatrixID::RGB, |
+ ColorSpace::RangeID::FULL); |
+ for (int row = 0; row < 3; ++row) { |
+ for (int col = 0; col < 3; ++col) { |
+ result.custom_primary_matrix_[3 * row + col] = to_XYZD50.get(row, col); |
+ } |
+ } |
+ result.custom_transfer_params_[0] = fn.fA; |
+ result.custom_transfer_params_[1] = fn.fB; |
+ result.custom_transfer_params_[2] = fn.fC; |
+ result.custom_transfer_params_[3] = fn.fD; |
+ result.custom_transfer_params_[4] = fn.fE; |
+ result.custom_transfer_params_[5] = fn.fF; |
+ result.custom_transfer_params_[6] = fn.fG; |
+ return result; |
+} |
+ |
+// static |
ColorSpace ColorSpace::CreateSCRGBLinear() { |
return ColorSpace(PrimaryID::BT709, TransferID::LINEAR_HDR, MatrixID::RGB, |
RangeID::FULL); |
@@ -197,10 +220,8 @@ sk_sp<SkColorSpace> ColorSpace::ToSkColorSpace() const { |
return icc_profile_sk_color_space_; |
// Unspecified color spaces correspond to the null SkColorSpace. |
- if (primaries_ == PrimaryID::UNSPECIFIED || |
hubbe
2017/02/15 08:48:56
Not sure I understand why this was the way it was
ccameron
2017/02/15 09:34:28
This is the same as before, just expressed in a wa
|
- transfer_ == TransferID::UNSPECIFIED) { |
+ if (!IsValid()) |
return nullptr; |
- } |
// Handle only full-range RGB spaces. |
if (matrix_ != MatrixID::RGB) { |
@@ -416,7 +437,6 @@ bool ColorSpace::GetTransferFunction(SkColorSpaceTransferFn* fn) const { |
return true; |
case ColorSpace::TransferID::RESERVED0: |
case ColorSpace::TransferID::RESERVED: |
- case ColorSpace::TransferID::UNSPECIFIED: |
case ColorSpace::TransferID::UNKNOWN: |
// All unknown values default to BT709 |
case ColorSpace::TransferID::BT709: |
@@ -436,6 +456,8 @@ bool ColorSpace::GetTransferFunction(SkColorSpaceTransferFn* fn) const { |
fn->fD = 0.091286342118f; |
fn->fG = 2.222222222222f; |
return true; |
+ // Unspecified values are given the sRGB transfer function. |
hubbe
2017/02/15 08:48:56
The comment above says otherwise.
ccameron
2017/02/15 09:34:28
This is the UNSPECIFIED versus UNKNOWN deal ... th
|
+ case ColorSpace::TransferID::UNSPECIFIED: |
case ColorSpace::TransferID::IEC61966_2_1: |
fn->fA = 0.947867345704f; |
fn->fB = 0.052132654296f; |
@@ -475,12 +497,12 @@ void ColorSpace::GetTransferMatrix(SkMatrix44* matrix) const { |
float Kr = 0; |
float Kb = 0; |
switch (matrix_) { |
hubbe
2017/02/15 08:48:56
I'm worried that changing all these defaults will
ccameron
2017/02/15 09:34:28
Should have been UNKNOWN ... does this fix it?
hubbe
2017/02/15 18:47:45
I don't think so. For video you we want unknown va
|
+ case ColorSpace::MatrixID::UNSPECIFIED: |
case ColorSpace::MatrixID::RGB: |
matrix->setIdentity(); |
return; |
case ColorSpace::MatrixID::BT709: |
- case ColorSpace::MatrixID::UNSPECIFIED: |
case ColorSpace::MatrixID::RESERVED: |
case ColorSpace::MatrixID::UNKNOWN: |
Kr = 0.2126f; |