Add service function to parse color information from monitor EDID

This CL adds a ParseChromaticityCoordinates() service function to the
bunch of edid_parser.cc functions, and adds unittests with a few real
EDIDs collected in the wild.

ParseOutputDeviceData() in that very file is minimally refactored
for early-return.

R=edid_parser_fuzzer

Bug: 771345
Change-Id: I956609d381b6fb199021b725f3838f55fb1fda48
Reviewed-on: https://chromium-review.googlesource.com/699351
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506876}

ui/display/BUILD.gn

@@ -164,6 +164,7 @@ test("display_unittests") {
     ":test_support",
     "//base",
     "//base/test:test_support",
+    "//cc/base",
     "//mojo/edk/test:run_all_unittests",
     "//testing/gmock",
     "//testing/gtest",

ui/display/util/BUILD.gn

@@ -19,6 +19,7 @@ component("util") {
 
   deps = [
     "//base",
+    "//skia",
     "//ui/display/types",
     "//ui/gfx/geometry",
   ]
@@ -46,6 +47,7 @@ fuzzer_test("edid_parser_fuzzer") {
   deps = [
     ":util",
     "//base",
+    "//skia",
     "//ui/gfx/geometry",
   ]
   seed_corpus = "fuzz_corpus"

ui/display/util/DEPS

+include_rules = [
+  "+third_party/skia/include/core/SkColorSpace.h",
+]

ui/display/util/edid_parser.cc

@@ -11,6 +11,7 @@
 #include "base/hash.h"
 #include "base/strings/string_util.h"
 #include "base/sys_byteorder.h"
+#include "third_party/skia/include/core/SkColorSpace.h"
 #include "ui/display/util/display_util.h"
 #include "ui/gfx/geometry/size.h"
 
@@ -39,23 +40,21 @@ bool GetDisplayIdFromEDID(const std::vector<uint8_t>& edid,
   ParseOutputDeviceData(edid, &manufacturer_id, &product_code, &product_name,
                         nullptr, nullptr);
 
-  if (manufacturer_id != 0) {
-    // Generates product specific value from product_name instead of product
-    // code.
-    // See crbug.com/240341
-    uint32_t product_code_hash =
-        product_name.empty() ? 0 : base::Hash(product_name);
-    // An ID based on display's index will be assigned later if this call
-    // fails.
-    *display_id_out =
-        GenerateDisplayID(manufacturer_id, product_code_hash, output_index);
-    // product_id is 64-bit signed so it can store -1 as kInvalidProductID and
-    // not match a valid product id which will all be in the lowest 32-bits.
-    if (product_id_out)
-      *product_id_out = GetProductID(manufacturer_id, product_code);
-    return true;
-  }
-  return false;
+  if (manufacturer_id == 0)
+    return false;
+
+  // Generates product specific value from product_name instead of product code.
+  // See crbug.com/240341
+  uint32_t product_code_hash =
+      product_name.empty() ? 0 : base::Hash(product_name);
+  // An ID based on display's index will be assigned later if this call fails.
+  *display_id_out =
+      GenerateDisplayID(manufacturer_id, product_code_hash, output_index);
+  // |product_id_out| is 64-bit signed so it can store -1 as kInvalidProductID
+  // and not match a valid product id which will all be in the lowest 32-bits.
+  if (product_id_out)
+    *product_id_out = GetProductID(manufacturer_id, product_code);
+  return true;
 }
 
 bool ParseOutputDeviceData(const std::vector<uint8_t>& edid,
@@ -260,4 +259,84 @@ bool ParseOutputOverscanFlag(const std::vector<uint8_t>& edid,
   return false;
 }
 
+bool ParseChromaticityCoordinates(const std::vector<uint8_t>& edid,
+                                  SkColorSpacePrimaries* primaries) {
+  DCHECK(primaries);
+
+  // Offsets, lengths, positions and masks are taken from [1] (or [2]).
+  // [1] http://en.wikipedia.org/wiki/Extended_display_identification_data
+  // [2] "VESA Enhanced EDID Standard " Release A, Revision 1, Feb 2000, Sec 3.7
+  //  "Phosphor or Filter Chromaticity: 10 bytes"
+  const unsigned int kChromaticityOffset = 25;
+  const unsigned int kChromaticityLength = 10;
+
+  const unsigned int kRedGreenLsbOffset = 25;
+  const unsigned int kRedxLsbPosition = 6;
+  const unsigned int kRedyLsbPosition = 4;
+  const unsigned int kGreenxLsbPosition = 3;
+  const unsigned int kGreenyLsbPosition = 0;
+
+  const unsigned int kBlueWhiteLsbOffset = 26;
+  const unsigned int kBluexLsbPosition = 6;
+  const unsigned int kBlueyLsbPosition = 4;
+  const unsigned int kWhitexLsbPosition = 3;
+  const unsigned int kWhiteyLsbPosition = 0;
+
+  // All LSBits parts are 2 bits wide.
+  const unsigned int kLsbMask = 0x3;
+
+  const unsigned int kRedxMsbOffset = 27;
+  const unsigned int kRedyMsbOffset = 28;
+  const unsigned int kGreenxMsbOffset = 29;
+  const unsigned int kGreenyMsbOffset = 30;
+  const unsigned int kBluexMsbOffset = 31;
+  const unsigned int kBlueyMsbOffset = 32;
+  const unsigned int kWhitexMsbOffset = 33;
+  const unsigned int kWhiteyMsbOffset = 34;
+
+  static_assert(
+      kChromaticityOffset + kChromaticityLength == kWhiteyMsbOffset + 1,
+      "EDID Parameter section length error");
+
+  if (edid.size() < kChromaticityOffset + kChromaticityLength) {
+    LOG(ERROR) << "too short EDID data: chromaticity coordinates";
+    return false;
+  }
+
+  const uint8_t red_green_lsbs = edid[kRedGreenLsbOffset];
+  const uint8_t blue_white_lsbs = edid[kBlueWhiteLsbOffset];
+
+  // Recompose the 10b values by appropriately mixing the 8 MSBs and the 2 LSBs,
+  // then rescale to 1024;
+  primaries->fRX = ((edid[kRedxMsbOffset] << 2) +
+                    ((red_green_lsbs >> kRedxLsbPosition) & kLsbMask)) /
+                   1024.0f;
+  primaries->fRY = ((edid[kRedyMsbOffset] << 2) +
+                    ((red_green_lsbs >> kRedyLsbPosition) & kLsbMask)) /
+                   1024.0f;
+  primaries->fGX = ((edid[kGreenxMsbOffset] << 2) +
+                    ((red_green_lsbs >> kGreenxLsbPosition) & kLsbMask)) /
+                   1024.0f;
+  primaries->fGY = ((edid[kGreenyMsbOffset] << 2) +
+                    ((red_green_lsbs >> kGreenyLsbPosition) & kLsbMask)) /
+                   1024.0f;
+  primaries->fBX = ((edid[kBluexMsbOffset] << 2) +
+                    ((blue_white_lsbs >> kBluexLsbPosition) & kLsbMask)) /
+                   1024.0f;
+  primaries->fBY = ((edid[kBlueyMsbOffset] << 2) +
+                    ((blue_white_lsbs >> kBlueyLsbPosition) & kLsbMask)) /
+                   1024.0f;
+  primaries->fWX = ((edid[kWhitexMsbOffset] << 2) +
+                    ((blue_white_lsbs >> kWhitexLsbPosition) & kLsbMask)) /
+                   1024.0f;
+  primaries->fWY = ((edid[kWhiteyMsbOffset] << 2) +
+                    ((blue_white_lsbs >> kWhiteyLsbPosition) & kLsbMask)) /
+                   1024.0f;
+
+  // TODO(mcasas): Up to two additional White Point coordinates can be provided
+  // in a Display Descriptor.Read them if we are not satisfied with |fWX| or
+  // |FWy|. https://crbug.com/771345.
+  return true;
+}
+
 }  // namespace display

ui/display/util/edid_parser.h

@@ -10,12 +10,15 @@
 #include <string>
 #include <vector>
 
+#include "base/compiler_specific.h"
 #include "ui/display/util/display_util_export.h"
 
 namespace gfx {
 class Size;
 }
 
+struct SkColorSpacePrimaries;
+
 // EDID (Extended Display Identification Data) is a format for monitor
 // metadata. This provides a parser for the data.
 
@@ -47,6 +50,12 @@ DISPLAY_UTIL_EXPORT bool ParseOutputOverscanFlag(
     const std::vector<uint8_t>& edid,
     bool* flag);
 
+// Extracts from |edid| the |primaries| chromaticity coordinates (CIE xy
+// coordinates for Red, Green and Blue channels and for the White Point).
+DISPLAY_UTIL_EXPORT bool ParseChromaticityCoordinates(
+    const std::vector<uint8_t>& edid,
+    SkColorSpacePrimaries* primaries) WARN_UNUSED_RESULT;
+
 }  // namespace display
 
 #endif // UI_DISPLAY_UTIL_EDID_PARSER_H_

ui/display/util/edid_parser_unittest.cc

@@ -10,8 +10,12 @@
 
 #include "base/macros.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/skia/include/core/SkColorSpace.h"
 #include "ui/gfx/geometry/size.h"
 
+using ::testing::AssertionFailure;
+using ::testing::AssertionSuccess;
+
 namespace display {
 
 namespace {
@@ -98,10 +102,79 @@ const unsigned char kLP2565B[] =
     "\x50\x20\x4C\x50\x32\x34\x36\x35\x0A\x20\x20\x20\x00\x00\x00\xFF"
     "\x00\x43\x4E\x4B\x38\x30\x32\x30\x34\x48\x4D\x0A\x20\x20\x00\x45";
 
+// HP z32x monitor.
+const unsigned char kHPz32x[] =
+    "\x00\xFF\xFF\xFF\xFF\xFF\xFF\x00\x22\xF0\x75\x32\x01\x01\x01\x01"
+    "\x1B\x1B\x01\x04\xB5\x46\x27\x78\x3A\x8D\x15\xAC\x51\x32\xB8\x26"
+    "\x0B\x50\x54\x21\x08\x00\xD1\xC0\xA9\xC0\x81\xC0\xD1\x00\xB3\x00"
+    "\x95\x00\xA9\x40\x81\x80\x4D\xD0\x00\xA0\xF0\x70\x3E\x80\x30\x20"
+    "\x35\x00\xB9\x88\x21\x00\x00\x1A\x00\x00\x00\xFD\x00\x18\x3C\x1E"
+    "\x87\x3C\x00\x0A\x20\x20\x20\x20\x20\x20\x00\x00\x00\xFC\x00\x48"
+    "\x50\x20\x5A\x33\x32\x78\x0A\x20\x20\x20\x20\x20\x00\x00\x00\xFF"
+    "\x00\x43\x4E\x43\x37\x32\x37\x30\x4D\x57\x30\x0A\x20\x20\x01\x46"
+    "\x02\x03\x18\xF1\x4B\x10\x1F\x04\x13\x03\x12\x02\x11\x01\x05\x14"
+    "\x23\x09\x07\x07\x83\x01\x00\x00\xA3\x66\x00\xA0\xF0\x70\x1F\x80"
+    "\x30\x20\x35\x00\xB9\x88\x21\x00\x00\x1A\x56\x5E\x00\xA0\xA0\xA0"
+    "\x29\x50\x30\x20\x35\x00\xB9\x88\x21\x00\x00\x1A\xEF\x51\x00\xA0"
+    "\xF0\x70\x19\x80\x30\x20\x35\x00\xB9\x88\x21\x00\x00\x1A\xE2\x68"
+    "\x00\xA0\xA0\x40\x2E\x60\x20\x30\x63\x00\xB9\x88\x21\x00\x00\x1C"
+    "\x28\x3C\x80\xA0\x70\xB0\x23\x40\x30\x20\x36\x00\xB9\x88\x21\x00"
+    "\x00\x1A\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x3E";
+
+// Chromebook Samus internal display.
+const unsigned char kSamus[] =
+    "\x00\xff\xff\xff\xff\xff\xff\x00\x30\xe4\x2e\x04\x00\x00\x00\x00"
+    "\x00\x18\x01\x04\xa5\x1b\x12\x96\x02\x4f\xd5\xa2\x59\x52\x93\x26"
+    "\x17\x50\x54\x00\x00\x00\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01"
+    "\x01\x01\x01\x01\x01\x01\x6d\x6f\x00\x9e\xa0\xa4\x31\x60\x30\x20"
+    "\x3a\x00\x10\xb5\x10\x00\x00\x19\x00\x00\x00\x00\x00\x00\x00\x00"
+    "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xfe\x00\x4c"
+    "\x47\x20\x44\x69\x73\x70\x6c\x61\x79\x0a\x20\x20\x00\x00\x00\xfe"
+    "\x00\x4c\x50\x31\x32\x39\x51\x45\x32\x2d\x53\x50\x41\x31\x00\x6c";
+
+// Chromebook Eve internal display.
+const unsigned char kEve[] =
+    "\x00\xff\xff\xff\xff\xff\xff\x00\x4d\x10\x8a\x14\x00\x00\x00\x00"
+    "\x16\x1b\x01\x04\xa5\x1a\x11\x78\x06\xde\x50\xa3\x54\x4c\x99\x26"
+    "\x0f\x50\x54\x00\x00\x00\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01"
+    "\x01\x01\x01\x01\x01\x01\xbb\x62\x60\xa0\x90\x40\x2e\x60\x30\x20"
+    "\x3a\x00\x03\xad\x10\x00\x00\x18\x00\x00\x00\x10\x00\x00\x00\x00"
+    "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x10\x00\x00"
+    "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xfc"
+    "\x00\x4c\x51\x31\x32\x33\x50\x31\x4a\x58\x33\x32\x0a\x20\x00\xb6";
+
 void Reset(gfx::Size* pixel, gfx::Size* size) {
   pixel->SetSize(0, 0);
   size->SetSize(0, 0);
 }
+// Chromaticity primaries in EDID are specified with 10 bits precision.
+const static float kPrimariesPrecision = 0.001f;
+
+::testing::AssertionResult SkColorSpacePrimariesEquals(
+    const char* lhs_expr,
+    const char* rhs_expr,
+    const SkColorSpacePrimaries& lhs,
+    const SkColorSpacePrimaries& rhs) {
+  // TODO(mcasas): consider using MathUtil::ApproximatelyEqual() when and if
+  // this is available in //base, https://crbug.com/771345
+  if (std::fabs(lhs.fRX - rhs.fRX) > kPrimariesPrecision)
+    return AssertionFailure() << "fRX: " << lhs.fRX << " != " << rhs.fRX;
+  if (std::fabs(lhs.fRY - rhs.fRY) > kPrimariesPrecision)
+    return AssertionFailure() << "fRY: " << lhs.fRY << " != " << rhs.fRY;
+  if (std::fabs(lhs.fGX - rhs.fGX) > kPrimariesPrecision)
+    return AssertionFailure() << "fGX: " << lhs.fGX << " != " << rhs.fGX;
+  if (std::fabs(lhs.fGY - rhs.fGY) > kPrimariesPrecision)
+    return AssertionFailure() << "fGY: " << lhs.fGY << " != " << rhs.fGY;
+  if (std::fabs(lhs.fBX - rhs.fBX) > kPrimariesPrecision)
+    return AssertionFailure() << "fBX: " << lhs.fBX << " != " << rhs.fBX;
+  if (std::fabs(lhs.fBY - rhs.fBY) > kPrimariesPrecision)
+    return AssertionFailure() << "fBY: " << lhs.fBY << " != " << rhs.fBY;
+  if (std::fabs(lhs.fWX - rhs.fWX) > kPrimariesPrecision)
+    return AssertionFailure() << "fWX: " << lhs.fWX << " != " << rhs.fWX;
+  if (std::fabs(lhs.fWY - rhs.fWY) > kPrimariesPrecision)
+    return AssertionFailure() << "fWY: " << lhs.fWY << " != " << rhs.fWY;
+  return AssertionSuccess();
+}
 
 }  // namespace
 
@@ -272,4 +345,58 @@ TEST(EDIDParserTest, GetDisplayIdFailure) {
   EXPECT_EQ(-1, product_id);
 }
 
+TEST(EDIDParserTest, ParseChromaticityCoordinates) {
+  const std::vector<uint8_t> edid_normal_display(
+      kNormalDisplay, kNormalDisplay + charsize(kNormalDisplay));
+  SkColorSpacePrimaries primaries_normal_display;
+  EXPECT_TRUE(ParseChromaticityCoordinates(edid_normal_display,
+                                           &primaries_normal_display));
+  // Color primaries associated to |kNormalDisplay|; they don't correspond to
+  // any standard color gamut.
+  const SkColorSpacePrimaries kNormalDisplayPrimaries = {
+      0.6777f, 0.3086f, 0.2080f, 0.6923f, 0.1465f, 0.0546f, 0.3125f, 0.3291f};
+  EXPECT_PRED_FORMAT2(SkColorSpacePrimariesEquals, primaries_normal_display,
+                      kNormalDisplayPrimaries);
+
+  const std::vector<uint8_t> edid_internal_display(
+      kInternalDisplay, kInternalDisplay + charsize(kInternalDisplay));
+  SkColorSpacePrimaries primaries_internal_display;
+  EXPECT_TRUE(ParseChromaticityCoordinates(edid_internal_display,
+                                           &primaries_internal_display));
+  // Color primaries associated to |kInternalDisplay|; they don't correspond to
+  // any standard color gamut.
+  const SkColorSpacePrimaries kInternalDisplayPrimaries = {
+      0.5849f, 0.3603f, 0.3769f, 0.5654f, 0.1552f, 0.0996f, 0.3125f, 0.3291f};
+  EXPECT_PRED_FORMAT2(SkColorSpacePrimariesEquals, primaries_internal_display,
+                      kInternalDisplayPrimaries);
+
+  const std::vector<uint8_t> edid_hpz32x(kHPz32x, kHPz32x + charsize(kHPz32x));
+  SkColorSpacePrimaries primaries_hpz32x;
+  EXPECT_TRUE(ParseChromaticityCoordinates(edid_hpz32x, &primaries_hpz32x));
+  // Color primaries associated to |kHPz32x|; they don't correspond to
+  // any standard color gamut.
+  const SkColorSpacePrimaries kHPz32xPrimaries = {
+      0.6738f, 0.3164f, 0.1962f, 0.7197f, 0.1484f, 0.0439f, 0.3144f, 0.3291f};
+  EXPECT_PRED_FORMAT2(SkColorSpacePrimariesEquals, primaries_hpz32x,
+                      kHPz32xPrimaries);
+
+  const std::vector<uint8_t> edid_samus(kSamus, kSamus + charsize(kSamus));
+  SkColorSpacePrimaries primaries_samus;
+  EXPECT_TRUE(ParseChromaticityCoordinates(edid_samus, &primaries_samus));
+  // Color primaries associated to Samus Chromebook, very similar to BT.709.
+  const SkColorSpacePrimaries kSamusPrimaries = {
+      0.6337f, 0.3476f, 0.3212f, 0.5771f, 0.1513f, 0.0908f, 0.3144f, 0.3291f};
+  EXPECT_PRED_FORMAT2(SkColorSpacePrimariesEquals, primaries_samus,
+                      kSamusPrimaries);
+
+  const std::vector<uint8_t> edid_eve(kEve, kEve + charsize(kEve));
+  SkColorSpacePrimaries primaries_eve;
+  EXPECT_TRUE(ParseChromaticityCoordinates(edid_eve, &primaries_eve));
+  // Color primaries associated to Eve Chromebook, very similar to BT.709.
+  const SkColorSpacePrimaries kEvePrimaries = {
+      0.6396f, 0.3291f, 0.2998f, 0.5996f, 0.1494f, 0.0595f, 0.3144f, 0.3281f};
+  EXPECT_PRED_FORMAT2(SkColorSpacePrimariesEquals, primaries_eve,
+                      kEvePrimaries);
+}
+
 }  // namespace display