Load and apply a vcgt table from an ICC file to the internal display

ash/display/display_color_manager_chromeos.cc

+// Copyright 2015 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 "ash/display/display_color_manager_chromeos.h"
+
+#include "ash/display/display_info.h"

[oshima, 2015/04/15 23:56:13]

do you need this?

[robert.bradford, 2015/04/17 16:42:45]

Done.

+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/command_line.h"
+#include "base/files/file_path.h"
+#include "base/logging.h"
+#include "chromeos/chromeos_switches.h"
+#include "content/public/browser/browser_thread.h"
+#include "third_party/qcms/src/qcms.h"
+#include "ui/display/types/display_snapshot.h"
+#include "ui/gfx/display.h"
+#include "ui/gfx/screen.h"
+
+using content::BrowserThread;
+
+namespace ash {
+
+namespace {
+
+DisplayColorManager::ColorCalibrationData* ParseFile(
+    const base::FilePath& path) {
+  qcms_profile* display_profile = qcms_profile_from_path(path.value().c_str());
+
+  if (!display_profile) {
+    LOG(WARNING) << "Unable to load ICC file: " << path.value();
+    return nullptr;
+  }
+
+  size_t vcgt_size = qcms_profile_vcgt_size(display_profile);
+  if (!vcgt_size) {
+    LOG(WARNING) << "No vcgt table in ICC file: " << path.value();
+    return nullptr;
+  }
+
+  DisplayColorManager::ColorCalibrationData* data =
+      new DisplayColorManager::ColorCalibrationData();
+  data->r.resize(vcgt_size);
+  data->g.resize(vcgt_size);
+  data->b.resize(vcgt_size);
+  if (!qcms_profile_vcgt_channels(display_profile, &data->r[0], &data->g[0],
+                                  &data->b[0])) {
+    LOG(WARNING) << "Unable to get vcgt data";
+    delete data;
+    qcms_profile_release(display_profile);
+    return nullptr;
+  }
+
+  qcms_profile_release(display_profile);
+  return data;

[oshima, 2015/04/15 23:56:13]

how big the data is? If it's small, just using struct would be simpler, and less
error prone.

[robert.bradford, 2015/04/17 16:42:46]

The data will be 3 * 256 * 2 bytes. 

+}
+
+}  // namespace
+
+DisplayColorManager::DisplayColorManager(ui::DisplayConfigurator* configurator)
+    : configurator_(configurator) {
+  configurator_->AddObserver(this);
+  LoadInternalFromCommandLine();
+}
+
+DisplayColorManager::~DisplayColorManager() {
+  configurator_->RemoveObserver(this);
+
+  for (auto it : calibration_map_) {
+    delete it.second;
+    calibration_map_.erase(it.first);
+  }
+}
+
+void DisplayColorManager::OnDisplayModeChanged(
+    const ui::DisplayConfigurator::DisplayStateList& display_states) {
+  for (const ui::DisplaySnapshot* state : display_states) {
+    ApplyDisplayColorCalibration(state->display_id());
+  }

[oshima, 2015/04/15 23:56:13]

nit: nuke {}

[robert.bradford, 2015/04/17 16:42:46]

Done.

+}
+
+void DisplayColorManager::ApplyDisplayColorCalibration(uint64_t display_id) {
+  if (calibration_map_.find(display_id) != calibration_map_.end()) {
+    ColorCalibrationData* ramp = calibration_map_[display_id];
+    if (!configurator_->SetGammaRamp(display_id, ramp->r, ramp->g, ramp->b)) {
+      LOG(WARNING) << "Error applying gamma ramp";
+    }

[oshima, 2015/04/15 23:56:13]

ditto

[robert.bradford, 2015/04/17 16:42:45]

Done.

+  }
+}
+
+void DisplayColorManager::LoadInternalFromCommandLine() {
+  const base::CommandLine* command_line =
+      base::CommandLine::ForCurrentProcess();
+  if (command_line->HasSwitch(
+          chromeos::switches::kInternalDisplayColorProfileFile)) {
+    const base::FilePath& path = command_line->GetSwitchValuePath(
+        chromeos::switches::kInternalDisplayColorProfileFile);
+    VLOG(1) << "Loading ICC file : " << path.value()
+            << "for internal display id: " << gfx::Display::InternalDisplayId();
+    BrowserThread::PostTask(
+        BrowserThread::FILE, FROM_HERE,
+        base::Bind(&DisplayColorManager::LoadFromPath, this, path,
+                   gfx::Display::InternalDisplayId()));

[oshima, 2015/04/15 23:56:13]

FILE thread is obsolete. Please use SequencedWorkerPool and PostTaskAndReply.

Also since this file will depend on content/ please move this to
ash/content/display

[robert.bradford, 2015/04/17 16:42:45]

Ended up using base::PostTaskAndReplyWithResult with BrowserThread::BlockingPool
which allows me to handle success or not of loading the file.

+  }
+}
+
+void DisplayColorManager::UpdateCalibrationData(
+    uint64_t display_id,
+    scoped_ptr<ColorCalibrationData> data) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  // The map takes over ownership of the underlying memory.
+  calibration_map_[display_id] = data.release();
+}
+
+void DisplayColorManager::LoadFromPath(const base::FilePath& path,
+                                       uint64_t display_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::FILE);
+  scoped_ptr<ColorCalibrationData> data = make_scoped_ptr(ParseFile(path));
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&DisplayColorManager::UpdateCalibrationData, this, display_id,
+                 base::Passed(&data)));
+}
+
+DisplayColorManager::ColorCalibrationData::ColorCalibrationData() {
+}
+
+DisplayColorManager::ColorCalibrationData::~ColorCalibrationData() {
+}
+
+}  // namespace ash