color: Don't use QCMS for transforms unless necessary

ui/gfx/color_transform_unittest.cc

 // Copyright (c) 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/logging.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/gfx/color_space.h"
 #include "ui/gfx/color_transform.h"
 #include "ui/gfx/icc_profile.h"
 #include "ui/gfx/test/icc_profiles.h"
@@ skipping 81 matching lines @@
   ICCProfile srgb_icc_profile = ICCProfileForTestingSRGB();
   ColorSpace srgb_fromicc = srgb_icc_profile.GetColorSpace();
   ColorSpace srgb_default = gfx::ColorSpace::CreateSRGB();
   ColorSpace xyzd50 = gfx::ColorSpace::CreateXYZD50();
 
   value_fromicc = value_default = ColorTransform::TriStim(0.1f, 0.5f, 0.9f);
 
   std::unique_ptr<ColorTransform> toxyzd50_fromicc(
       ColorTransform::NewColorTransform(
           srgb_fromicc, xyzd50, ColorTransform::Intent::INTENT_ABSOLUTE));
-  // This will have 1 step, namely, the QCMS transform.
-  EXPECT_EQ(toxyzd50_fromicc->NumberOfStepsForTesting(), 1u);
+  // This will be converted to a transfer function and then linear transform.
+  EXPECT_EQ(toxyzd50_fromicc->NumberOfStepsForTesting(), 2u);
   toxyzd50_fromicc->Transform(&value_fromicc, 1);
 
   std::unique_ptr<ColorTransform> toxyzd50_default(
       ColorTransform::NewColorTransform(
           srgb_default, xyzd50, ColorTransform::Intent::INTENT_ABSOLUTE));
   // This will have a transfer function and then linear transform.
   EXPECT_EQ(toxyzd50_default->NumberOfStepsForTesting(), 2u);
   toxyzd50_default->Transform(&value_default, 1);
 
   EXPECT_NEAR(value_fromicc.x(), value_default.x(), kEpsilon);
@@ skipping 73 matching lines @@
   EXPECT_NEAR(tmp.y(), 1.0f, kEpsilon);
   EXPECT_NEAR(tmp.z(), 0.0f, kEpsilon);
 
   tmp = ColorTransform::TriStim(0.0f, 0.0f, 1.0f);
   t->Transform(&tmp, 1);
   EXPECT_NEAR(tmp.x(), 0.0f, kEpsilon);
   EXPECT_NEAR(tmp.y(), 0.0f, kEpsilon);
   EXPECT_NEAR(tmp.z(), 1.0f, kEpsilon);
 }
 
 TEST(SimpleColorSpace, UnknownToSRGB) {

[hubbe, 2017/02/16 01:28:29]

The changes in this test confuse me.

1. They seem to indicate that the default behavior of ColorSpace is now sRGB
instead of BT709. I don't understand why that would be based on this change.

2. If the default was to change, I prefer to change the construction of
"unknown" to match what would happen to a video which has not set it's color
space.

[ccameron, 2017/02/16 01:58:47]

I moved this to the previous color patch and rebased it. But to discuss here.

What to do when we get a default-constructor gfx::ColorSpace passed to
ColorTransform?

I'm voting for "do no transformation" at the moment. Assuming sRGB would
probably be "not too bad" for everything video. Assuming BT709 would be "not too
bad" for video, and a disaster for everything else.

When we get close to shipping "color correct everything", we should probably
DCHECK that the transform's spaces are valid.

   ColorSpace unknown;
   ColorSpace sRGB = ColorSpace::CreateSRGB();
   std::unique_ptr<ColorTransform> t(ColorTransform::NewColorTransform(
       unknown, sRGB, ColorTransform::Intent::INTENT_PERCEPTUAL));
 
-  ColorTransform::TriStim tmp(16.0f / 255.0f, 0.5f, 0.5f);
+  ColorTransform::TriStim input;
+  ColorTransform::TriStim tmp;
+  input = ColorTransform::TriStim(16.0f / 255.0f, 0.5f, 0.5f);
+  tmp = input;
   t->Transform(&tmp, 1);
-  EXPECT_NEAR(tmp.x(), 0.0f, 0.001f);
-  EXPECT_NEAR(tmp.y(), 0.0f, 0.001f);
-  EXPECT_NEAR(tmp.z(), 0.0f, 0.001f);
+  EXPECT_NEAR(tmp.x(), input.x(), 0.001f);
+  EXPECT_NEAR(tmp.y(), input.y(), 0.001f);
+  EXPECT_NEAR(tmp.z(), input.z(), 0.001f);
 
-  tmp = ColorTransform::TriStim(235.0f / 255.0f, 0.5f, 0.5f);
+  input = ColorTransform::TriStim(235.0f / 255.0f, 0.5f, 0.5f);
+  tmp = input;
   t->Transform(&tmp, 1);
-  EXPECT_NEAR(tmp.x(), 1.0f, 0.001f);
-  EXPECT_NEAR(tmp.y(), 1.0f, 0.001f);
-  EXPECT_NEAR(tmp.z(), 1.0f, 0.001f);
+  EXPECT_NEAR(tmp.x(), input.x(), 0.001f);
+  EXPECT_NEAR(tmp.y(), input.y(), 0.001f);
+  EXPECT_NEAR(tmp.z(), input.z(), 0.001f);
 
   // Test a blue color
-  tmp = ColorTransform::TriStim(128.0f / 255.0f, 240.0f / 255.0f, 0.5f);
+  input = ColorTransform::TriStim(128.0f / 255.0f, 240.0f / 255.0f, 0.5f);
+  tmp = input;
   t->Transform(&tmp, 1);
-  EXPECT_GT(tmp.z(), tmp.x());
-  EXPECT_GT(tmp.z(), tmp.y());
+  EXPECT_NEAR(tmp.x(), input.x(), 0.001f);
+  EXPECT_NEAR(tmp.y(), input.y(), 0.001f);
+  EXPECT_NEAR(tmp.z(), input.z(), 0.001f);
 }
 
 class TransferTest : public testing::TestWithParam<ColorSpace::TransferID> {};
 
 TEST_P(TransferTest, basicTest) {
   gfx::ColorSpace space_with_transfer(ColorSpace::PrimaryID::BT709, GetParam(),
                                       ColorSpace::MatrixID::RGB,
                                       ColorSpace::RangeID::FULL);
   gfx::ColorSpace space_linear(
       ColorSpace::PrimaryID::BT709, ColorSpace::TransferID::LINEAR,
@@ skipping 90 matching lines @@
 
 INSTANTIATE_TEST_CASE_P(
     C,
     ColorSpaceTest,
     testing::Combine(testing::ValuesIn(all_primaries),
                      testing::Values(ColorSpace::TransferID::BT709),
                      testing::ValuesIn(all_matrices),
                      testing::ValuesIn(all_ranges),
                      testing::ValuesIn(intents)));
 }  // namespace