color: Clarify default behaviors

ui/gfx/color_space.cc

Index: ui/gfx/color_space.cc
diff --git a/ui/gfx/color_space.cc b/ui/gfx/color_space.cc
index 0f5a42b601841ca529d42c456df504512c708914..ff8ba0d298bc2b32ccbd354b0497e612188c3151 100644
--- a/ui/gfx/color_space.cc
+++ b/ui/gfx/color_space.cc
@@ -15,31 +15,123 @@
 
 namespace gfx {
 
-ColorSpace::PrimaryID ColorSpace::PrimaryIDFromInt(int primary_id) {
-  if (primary_id < 0 || primary_id > static_cast<int>(PrimaryID::LAST))
-    return PrimaryID::UNKNOWN;
-  if (primary_id > static_cast<int>(PrimaryID::LAST_STANDARD_VALUE) &&
-      primary_id < 1000)
-    return PrimaryID::UNKNOWN;
-  return static_cast<PrimaryID>(primary_id);
+ColorSpace::PrimaryID ColorSpace::PrimaryIDFromH264(int primary_id) {
+  // These values should match the H264 specification (see Table E-3
+  // Colour Primaries in https://www.itu.int/rec/T-REC-H.264/en).
+  switch (primary_id) {
+    case 0:  // RESERVED0
+      break;
+    case 1:
+      return PrimaryID::BT709;
+    case 2:  // UNSPECIFIED
+      break;
+    case 3:  // RESERVED
+      break;
+    case 4:
+      return PrimaryID::BT470M;
+    case 5:
+      return PrimaryID::BT470BG;
+    case 6:
+      return PrimaryID::SMPTE170M;
+    case 7:
+      return PrimaryID::SMPTE240M;
+    case 8:
+      return PrimaryID::FILM;
+    case 9:
+      return PrimaryID::BT2020;
+    case 10:
+      return PrimaryID::SMPTEST428_1;
+    case 11:
+      return PrimaryID::SMPTEST431_2;
+    case 12:
+      return PrimaryID::SMPTEST432_1;
+    default:
+      break;
+  }
+  // All unknown values default to BT709.

[hubbe, 2017/02/15 23:30:43]

This limits our ability to make smart guesses based on the values that *are*
set.

Like, if a video uses the BT601 primaries, it should probably use the BT601
matrix as well. However, with this code we won't be able to tell the difference
between an unset, invalid or default value anymore.

+  return PrimaryID::BT709;
 }
 
-ColorSpace::TransferID ColorSpace::TransferIDFromInt(int transfer_id) {
-  if (transfer_id < 0 || transfer_id > static_cast<int>(TransferID::LAST))
-    return TransferID::UNKNOWN;
-  if (transfer_id > static_cast<int>(TransferID::LAST_STANDARD_VALUE) &&
-      transfer_id < 1000)
-    return TransferID::UNKNOWN;
-  return static_cast<TransferID>(transfer_id);
+ColorSpace::TransferID ColorSpace::TransferIDFromH264(int transfer_id) {
+  switch (transfer_id) {
+    case 0:  // RESERVED0
+      break;
+    case 1:
+      return TransferID::BT709;
+    case 2:  // UNSPECIFIED
+      break;
+    case 3:  // RESERVED
+      break;
+    case 4:
+      return TransferID::GAMMA22;
+    case 5:
+      return TransferID::GAMMA28;
+    case 6:
+      return TransferID::SMPTE170M;
+    case 7:
+      return TransferID::SMPTE240M;
+    case 8:
+      return TransferID::LINEAR;
+    case 9:
+      return TransferID::LOG;
+    case 10:
+      return TransferID::LOG_SQRT;
+    case 11:
+      return TransferID::IEC61966_2_4;
+    case 12:
+      return TransferID::BT1361_ECG;
+    case 13:
+      return TransferID::IEC61966_2_1;
+    case 14:
+      return TransferID::BT2020_10;
+    case 15:
+      return TransferID::BT2020_12;
+    case 16:
+      return TransferID::SMPTEST2084;
+    case 17:
+      return TransferID::SMPTEST428_1;
+    case 18:
+      return TransferID::ARIB_STD_B67;
+    default:
+      break;
+  }
+  // All unknown values default to BT709
+  return TransferID::BT709;
 }
 
-ColorSpace::MatrixID ColorSpace::MatrixIDFromInt(int matrix_id) {
-  if (matrix_id < 0 || matrix_id > static_cast<int>(MatrixID::LAST))
-    return MatrixID::UNKNOWN;
-  if (matrix_id > static_cast<int>(MatrixID::LAST_STANDARD_VALUE) &&
-      matrix_id < 1000)
-    return MatrixID::UNKNOWN;
-  return static_cast<MatrixID>(matrix_id);
+ColorSpace::MatrixID ColorSpace::MatrixIDFromH264(int matrix_id) {
+  // The first 0-255 values should match the H264 specification (see Table E-5
+  // Matrix Coefficients in https://www.itu.int/rec/T-REC-H.264/en).
+  switch (matrix_id) {
+    case 0:
+      return MatrixID::RGB;
+    case 1:
+      return MatrixID::BT709;
+    case 2:  // UNSPECIFIED
+      break;
+    case 3:  // RESERVED
+      break;
+    case 4:
+      return MatrixID::FCC;
+    case 5:
+      return MatrixID::BT470BG;
+    case 6:
+      return MatrixID::SMPTE170M;
+    case 7:
+      return MatrixID::SMPTE240M;
+    case 8:
+      return MatrixID::YCOCG;
+    case 9:
+      return MatrixID::BT2020_NCL;
+    case 10:
+      return MatrixID::BT2020_CL;
+    case 11:
+      return MatrixID::YDZDX;
+    default:
+      break;
+  }
+  // All unknown values default to BT709
+  return MatrixID::BT709;
 }
 
 ColorSpace::ColorSpace() {}
@@ -60,12 +152,6 @@ ColorSpace::ColorSpace(PrimaryID primaries,
       matrix_(matrix),
       range_(range) {}
 
-ColorSpace::ColorSpace(int primaries, int transfer, int matrix, RangeID range)
-    : primaries_(PrimaryIDFromInt(primaries)),
-      transfer_(TransferIDFromInt(transfer)),
-      matrix_(MatrixIDFromInt(matrix)),
-      range_(range) {}
-
 ColorSpace::ColorSpace(const ColorSpace& other)
     : primaries_(other.primaries_),
       transfer_(other.transfer_),
@@ -86,7 +172,7 @@ ColorSpace::ColorSpace(const ColorSpace& other)
 ColorSpace::~ColorSpace() = default;
 
 bool ColorSpace::IsValid() const {
-  return *this != gfx::ColorSpace();
+  return primaries_ != PrimaryID::INVALID && transfer_ != TransferID::INVALID;
 }
 
 // static
@@ -197,10 +283,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 ||
-      transfer_ == TransferID::UNSPECIFIED) {
+  if (!IsValid())
     return nullptr;
-  }
 
   // Handle only full-range RGB spaces.
   if (matrix_ != MatrixID::RGB) {
@@ -249,10 +333,10 @@ void ColorSpace::GetPrimaryMatrix(SkMatrix44* to_XYZD50) const {
       to_XYZD50->set3x3RowMajorf(custom_primary_matrix_);
       return;
 
-    case ColorSpace::PrimaryID::RESERVED0:
-    case ColorSpace::PrimaryID::RESERVED:
-    case ColorSpace::PrimaryID::UNSPECIFIED:
-    case ColorSpace::PrimaryID::UNKNOWN:
+    case ColorSpace::PrimaryID::INVALID:
+      to_XYZD50->setIdentity();
+      return;
+
     case ColorSpace::PrimaryID::BT709:
       // BT709 is our default case. Put it after the switch just
       // in case we somehow get an id which is not listed in the switch.
@@ -414,11 +498,6 @@ bool ColorSpace::GetTransferFunction(SkColorSpaceTransferFn* fn) const {
     case ColorSpace::TransferID::GAMMA28:
       fn->fG = 2.8f;
       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:
     case ColorSpace::TransferID::SMPTE170M:
     case ColorSpace::TransferID::BT2020_10:
@@ -458,6 +537,7 @@ bool ColorSpace::GetTransferFunction(SkColorSpaceTransferFn* fn) const {
     case ColorSpace::TransferID::LOG_SQRT:
     case ColorSpace::TransferID::SMPTEST2084:
     case ColorSpace::TransferID::SMPTEST2084_NON_HDR:
+    case ColorSpace::TransferID::INVALID:
       break;
   }
 
@@ -480,9 +560,6 @@ void ColorSpace::GetTransferMatrix(SkMatrix44* matrix) const {
       return;
 
     case ColorSpace::MatrixID::BT709:
-    case ColorSpace::MatrixID::UNSPECIFIED:
-    case ColorSpace::MatrixID::RESERVED:
-    case ColorSpace::MatrixID::UNKNOWN:
       Kr = 0.2126f;
       Kb = 0.0722f;
       break;
@@ -562,7 +639,6 @@ void ColorSpace::GetTransferMatrix(SkMatrix44* matrix) const {
 void ColorSpace::GetRangeAdjustMatrix(SkMatrix44* matrix) const {
   switch (range_) {
     case RangeID::FULL:
-    case RangeID::UNSPECIFIED:
       matrix->setIdentity();
       return;
 
@@ -578,8 +654,6 @@ void ColorSpace::GetRangeAdjustMatrix(SkMatrix44* matrix) const {
       break;
 
     case MatrixID::BT709:
-    case MatrixID::UNSPECIFIED:
-    case MatrixID::RESERVED:
     case MatrixID::FCC:
     case MatrixID::BT470BG:
     case MatrixID::SMPTE170M:
@@ -587,7 +661,6 @@ void ColorSpace::GetRangeAdjustMatrix(SkMatrix44* matrix) const {
     case MatrixID::BT2020_NCL:
     case MatrixID::BT2020_CL:
     case MatrixID::YDZDX:
-    case MatrixID::UNKNOWN:
       matrix->setScale(255.0f/219.0f, 255.0f/224.0f, 255.0f/224.0f);
       matrix->postTranslate(-16.0f/219.0f, -15.5f/224.0f, -15.5f/224.0f);
       break;