Skip to content

Commit

Permalink
wire EDID data to DisplayUnitInfo Chrome api
Browse files Browse the repository at this point in the history
This CL wires 3 display/monitor fields from EDID parsing
to the Chrome private API where they surface, as described
in the DD [1], namely:
1- manufacturer id
2- product id
3- year of manufacture.

(Note that ToT Ozone DRM already parses this list's 1 and 2;
this CL adds code to parse 3). A diagram of the touched areas
is in the DD [1] extracted for quick access in:
  https://i.imgur.com/yx8H0R3.jpg (also [2]).
but in a nutshell: Ozone DRM creates a DisplaySnapshot from
the EDID, and passes it to Browser Process via Mojo and/or IPC
(being migrated); in Browser DisplayChangeObserver transforms that
snapshot into a ManagedDisplayInfo, and in turn passes it to the
DisplayManager that creates a Display that is used later on upon
request by DisplayInfoProviderChromeOS to fill in the JS data struct.

There's a bit of confusion on ToT between product_id and
product_code, so this CL follows the spec and aligns them all,
i.e. EDID parser produces a |product_code|, which is a pack of
the manufacturer id and product id.

Unittests extended, tested on soraka+simplechrome.

[1] go/js-edid-chromeos
[2] https://imgur.com/a/aqNDD

Bug: 821393
Change-Id: Ia219b4eeb92cf3eae3e58046340f1b81c34c9ea2
Reviewed-on: https://chromium-review.googlesource.com/959547
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Reviewed-by: Daniel Nicoara <dnicoara@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543512}
  • Loading branch information
yell0wd0g authored and Commit Bot committed Mar 15, 2018
1 parent b27ebb7 commit 9b90891
Show file tree
Hide file tree
Showing 28 changed files with 364 additions and 94 deletions.
20 changes: 11 additions & 9 deletions ash/display/display_color_manager_chromeos.cc
Original file line number Diff line number Diff line change
Expand Up @@ -186,14 +186,16 @@ void DisplayColorManager::OnDisplayModeChanged(

UMA_HISTOGRAM_BOOLEAN("Ash.DisplayColorManager.HasColorCorrectionMatrix",
state->has_color_correction_matrix());
if (calibration_map_[state->product_id()]) {
ApplyDisplayColorCalibration(state->display_id(), state->product_id());
if (calibration_map_[state->product_code()]) {
ApplyDisplayColorCalibration(state->display_id(), state->product_code());
} else {
const bool valid_product_id =
state->product_id() != display::DisplaySnapshot::kInvalidProductID;
const bool valid_product_code =
state->product_code() !=
display::DisplaySnapshot::kInvalidProductCode;
// TODO(mcasas): correct UMA s/Id/Code/, https://crbug.com/821393.
UMA_HISTOGRAM_BOOLEAN("Ash.DisplayColorManager.ValidProductId",
valid_product_id);
if (valid_product_id)
valid_product_code);
if (valid_product_code)
LoadCalibrationForDisplay(state);
}
}
Expand Down Expand Up @@ -225,11 +227,11 @@ void DisplayColorManager::LoadCalibrationForDisplay(
return;

quirks::QuirksManager::Get()->RequestIccProfilePath(
display->product_id(), display->display_name(),
display->product_code(), display->display_name(),
base::Bind(&DisplayColorManager::FinishLoadCalibrationForDisplay,
weak_ptr_factory_.GetWeakPtr(), display->display_id(),
display->product_id(), display->has_color_correction_matrix(),
display->type()));
display->product_code(),
display->has_color_correction_matrix(), display->type()));
}

void DisplayColorManager::FinishLoadCalibrationForDisplay(
Expand Down
1 change: 1 addition & 0 deletions ash/display/display_error_observer_chromeos_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ std::unique_ptr<display::DisplaySnapshot> CreateTestDisplaySnapshot(
display::DisplaySnapshot::DisplayModeList() /* modes */,
std::vector<uint8_t>() /* edid */, nullptr /* current_mode */,
nullptr /* native_mode */, 0 /* product_id */,
display::kInvalidYearOfManufacture,
gfx::Size() /* maximum_cursor_size */);
}

Expand Down
19 changes: 17 additions & 2 deletions chrome/browser/extensions/display_info_provider_chromeos.cc
Original file line number Diff line number Diff line change
Expand Up @@ -782,6 +782,23 @@ void DisplayInfoProviderChromeOS::UpdateDisplayUnitInfoForPlatform(
}
}

const display::ManagedDisplayInfo& display_info =
display_manager->GetDisplayInfo(display.id());
if (!display_info.manufacturer_id().empty() ||
!display_info.product_id().empty() ||
(display_info.year_of_manufacture() !=
display::kInvalidYearOfManufacture)) {
unit->edid = std::make_unique<system_display::Edid>();
if (!display_info.manufacturer_id().empty())
unit->edid->manufacturer_id = display_info.manufacturer_id();
if (!display_info.product_id().empty())
unit->edid->product_id = display_info.product_id();
if (display_info.year_of_manufacture() !=
display::kInvalidYearOfManufacture) {
unit->edid->year_of_manufacture = display_info.year_of_manufacture();
}
}

unit->display_zoom_factor =
display_manager->GetZoomFactorForDisplay(display.id());
display::ManagedDisplayMode active_mode;
Expand All @@ -793,8 +810,6 @@ void DisplayInfoProviderChromeOS::UpdateDisplayUnitInfoForPlatform(
display_manager->GetZoomFactorForDisplay(display.id()));
}

const display::ManagedDisplayInfo& display_info =
display_manager->GetDisplayInfo(display.id());
const float device_dpi = display_info.device_dpi();
unit->dpi_x = device_dpi * display.size().width() /
display_info.bounds_in_native().width();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1948,5 +1948,28 @@ TEST_F(DisplayInfoProviderChromeosTest, SetMIXEDMode) {
}
}

TEST_F(DisplayInfoProviderChromeosTest, GetEdid) {
UpdateDisplay("500x600, 400x200");
const char* kManufacturerId = "GOO";
const char* kProductId = "GL";
constexpr int32_t kYearOfManufacture = 2018;

display::ManagedDisplayInfo info(
GetDisplayManager()->active_display_list()[0].id(), "",
false /* has_overscan */);
info.SetBounds(gfx::Rect(0, 0, 500, 600));
info.set_manufacturer_id(kManufacturerId);
info.set_product_id(kProductId);
info.set_year_of_manufacture(kYearOfManufacture);
GetDisplayManager()->OnNativeDisplaysChanged({info});

DisplayUnitInfoList result = GetAllDisplaysInfo();
ASSERT_EQ(1u, result.size());
ASSERT_TRUE(result[0].edid);
EXPECT_EQ(kManufacturerId, result[0].edid->manufacturer_id);
EXPECT_EQ(kProductId, result[0].edid->product_id);
EXPECT_EQ(kYearOfManufacture, result[0].edid->year_of_manufacture);
}

} // namespace
} // namespace extensions
18 changes: 18 additions & 0 deletions extensions/common/api/system_display.idl
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,31 @@ namespace system.display {
long offset;
};

// EDID extracted parameters. Field description refers to "VESA ENHANCED
// EXTENDED DISPLAY IDENTIFICATION DATA STANDARD (Defines EDID Structure
// Version 1, Revision 4)" Release A, Revision 2 September 25, 2006.
// https://www.vesa.org/vesa-standards
dictionary Edid {
// 3 character manufacturer code. See Sec. 3.4.1 page 21. Required in v1.4.
DOMString manufacturerId;

// 2 byte manufacturer-assigned code, Sec. 3.4.2 page 21. Required in v1.4.
DOMString productId;

// Year of manufacturer, Sec. 3.4.4 page 22. Required in v1.4.
long yearOfManufacture;
};

dictionary DisplayUnitInfo {
// The unique identifier of the display.
DOMString id;

// The user-friendly name (e.g. "HP LCD monitor").
DOMString name;

// NOTE: This is only available to Chrome OS Kiosk apps and Web UI.
Edid? edid;

// Chrome OS only. Identifier of the display that is being mirrored if
// mirroring is enabled, otherwise empty. This will be set for all displays
// (including the display being mirrored).
Expand Down
2 changes: 1 addition & 1 deletion ui/display/display.cc
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ gfx::Size Display::GetSizeInPixel() const {

std::string Display::ToString() const {
return base::StringPrintf(
"Display[%lld] bounds=%s, workarea=%s, scale=%g, %s",
"Display[%lld] bounds=[%s], workarea=[%s], scale=%g, %s.",
static_cast<long long int>(id_), bounds_.ToString().c_str(),
work_area_.ToString().c_str(), device_scale_factor_,
IsInternal() ? "internal" : "external");
Expand Down
4 changes: 1 addition & 3 deletions ui/display/display.h
Original file line number Diff line number Diff line change
Expand Up @@ -220,9 +220,7 @@ class DISPLAY_EXPORT Display final {
// True if this is a monochrome display (e.g, for accessiblity). Used by media
// query APIs.
bool is_monochrome() const { return is_monochrome_; }
void set_is_monochrome(bool is_monochrome) {
is_monochrome_ = is_monochrome;
}
void set_is_monochrome(bool is_monochrome) { is_monochrome_ = is_monochrome; }

bool operator==(const Display& rhs) const;
bool operator!=(const Display& rhs) const { return !(*this == rhs); }
Expand Down
50 changes: 31 additions & 19 deletions ui/display/manager/chromeos/display_change_observer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "ui/display/types/display_mode.h"
#include "ui/display/types/display_snapshot.h"
#include "ui/display/util/display_util.h"
#include "ui/display/util/edid_parser.h"
#include "ui/events/devices/input_device_manager.h"
#include "ui/events/devices/touchscreen_device.h"
#include "ui/strings/grit/ui_strings.h"
Expand Down Expand Up @@ -265,21 +266,21 @@ void DisplayChangeObserver::UpdateInternalDisplay(
}

ManagedDisplayInfo DisplayChangeObserver::CreateManagedDisplayInfo(
const DisplaySnapshot* state,
const DisplaySnapshot* snapshot,
const DisplayMode* mode_info) {
float device_scale_factor = 1.0f;
// Sets dpi only if the screen size is not blacklisted.
float dpi = IsDisplaySizeBlackListed(state->physical_size())
float dpi = IsDisplaySizeBlackListed(snapshot->physical_size())
? 0
: kInchInMm * mode_info->size().width() /
state->physical_size().width();
snapshot->physical_size().width();

if (state->type() == DISPLAY_CONNECTION_TYPE_INTERNAL) {
if (snapshot->type() == DISPLAY_CONNECTION_TYPE_INTERNAL) {
if (dpi)
device_scale_factor = FindDeviceScaleFactor(dpi);
} else {
ManagedDisplayMode mode;
if (display_manager_->GetSelectedModeForDisplayId(state->display_id(),
if (display_manager_->GetSelectedModeForDisplayId(snapshot->display_id(),
&mode)) {
device_scale_factor = mode.device_scale_factor();
} else {
Expand All @@ -288,8 +289,8 @@ ManagedDisplayInfo DisplayChangeObserver::CreateManagedDisplayInfo(
// from the value of |k2xThreshouldSizeSquaredFor4KInMm|
const int k2xThreshouldSizeSquaredFor4KInMm =
(40 * 40 * kInchInMm * kInchInMm) - 100;
gfx::Vector2d size_in_vec(state->physical_size().width(),
state->physical_size().height());
gfx::Vector2d size_in_vec(snapshot->physical_size().width(),
snapshot->physical_size().height());
if (size_in_vec.LengthSquared() > k2xThreshouldSizeSquaredFor4KInMm &&
mode_info->size().width() >= kMinimumWidthFor4K) {
// Make sure that additional device scale factors table has 2x.
Expand All @@ -299,35 +300,46 @@ ManagedDisplayInfo DisplayChangeObserver::CreateManagedDisplayInfo(
}
}

std::string name = (state->type() == DISPLAY_CONNECTION_TYPE_INTERNAL)
std::string name = (snapshot->type() == DISPLAY_CONNECTION_TYPE_INTERNAL)
? l10n_util::GetStringUTF8(IDS_DISPLAY_NAME_INTERNAL)
: state->display_name();
: snapshot->display_name();

if (name.empty())
name = l10n_util::GetStringUTF8(IDS_DISPLAY_NAME_UNKNOWN);

const bool has_overscan = state->has_overscan();
const int64_t id = state->display_id();
const bool has_overscan = snapshot->has_overscan();
const int64_t id = snapshot->display_id();

ManagedDisplayInfo new_info = ManagedDisplayInfo(id, name, has_overscan);
new_info.set_sys_path(state->sys_path());

if (snapshot->product_code() != DisplaySnapshot::kInvalidProductCode) {
uint16_t manufacturer_id = 0;
uint16_t product_id = 0;
SplitProductCodeInManufacturerIdAndProductId(snapshot->product_code(),
&manufacturer_id, &product_id);
new_info.set_manufacturer_id(ManufacturerIdToString(manufacturer_id));
new_info.set_product_id(ProductIdToString(product_id));
}
new_info.set_year_of_manufacture(snapshot->year_of_manufacture());

new_info.set_sys_path(snapshot->sys_path());
new_info.set_device_scale_factor(device_scale_factor);
const gfx::Rect display_bounds(state->origin(), mode_info->size());
const gfx::Rect display_bounds(snapshot->origin(), mode_info->size());
new_info.SetBounds(display_bounds);
new_info.set_native(true);
new_info.set_is_aspect_preserving_scaling(
state->is_aspect_preserving_scaling());
snapshot->is_aspect_preserving_scaling());
if (dpi)
new_info.set_device_dpi(dpi);
new_info.set_color_space(state->color_space());
new_info.set_color_space(snapshot->color_space());

ManagedDisplayInfo::ManagedDisplayModeList display_modes =
(state->type() == DISPLAY_CONNECTION_TYPE_INTERNAL)
? GetInternalManagedDisplayModeList(new_info, *state)
: GetExternalManagedDisplayModeList(*state);
(snapshot->type() == DISPLAY_CONNECTION_TYPE_INTERNAL)
? GetInternalManagedDisplayModeList(new_info, *snapshot)
: GetExternalManagedDisplayModeList(*snapshot);
new_info.SetManagedDisplayModes(display_modes);

new_info.set_maximum_cursor_size(state->maximum_cursor_size());
new_info.set_maximum_cursor_size(snapshot->maximum_cursor_size());
return new_info;
}

Expand Down
2 changes: 1 addition & 1 deletion ui/display/manager/chromeos/display_change_observer.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class DISPLAY_MANAGER_EXPORT DisplayChangeObserver
void UpdateInternalDisplay(
const DisplayConfigurator::DisplayStateList& display_states);

ManagedDisplayInfo CreateManagedDisplayInfo(const DisplaySnapshot* state,
ManagedDisplayInfo CreateManagedDisplayInfo(const DisplaySnapshot* snapshot,
const DisplayMode* mode_info);

// Both |display_configurator_| and |display_manager_| are not owned and must
Expand Down
1 change: 1 addition & 0 deletions ui/display/manager/fake_display_snapshot.cc
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,7 @@ FakeDisplaySnapshot::FakeDisplaySnapshot(int64_t display_id,
current_mode,
native_mode,
product_id,
2018 /*year_of_manufacture */,
maximum_cursor_size) {}

FakeDisplaySnapshot::~FakeDisplaySnapshot() {}
Expand Down
2 changes: 1 addition & 1 deletion ui/display/manager/fake_display_snapshot.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ class DISPLAY_MANAGER_EXPORT FakeDisplaySnapshot : public DisplaySnapshot {
bool has_overscan_ = false;
bool has_color_correction_matrix_ = false;
std::string name_;
int64_t product_id_ = DisplaySnapshot::kInvalidProductID;
int64_t product_id_ = DisplaySnapshot::kInvalidProductCode;
gfx::Size maximum_cursor_size_ = gfx::Size(64, 64);
DisplayModeList modes_;
const DisplayMode* current_mode_ = nullptr;
Expand Down
5 changes: 5 additions & 0 deletions ui/display/manager/managed_display_info.cc
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@ ManagedDisplayInfo ManagedDisplayInfo::CreateFromSpecWithID(

ManagedDisplayInfo::ManagedDisplayInfo()
: id_(kInvalidDisplayId),
year_of_manufacture_(kInvalidYearOfManufacture),
has_overscan_(false),
active_rotation_source_(Display::RotationSource::UNKNOWN),
touch_support_(Display::TouchSupport::UNKNOWN),
Expand All @@ -276,6 +277,7 @@ ManagedDisplayInfo::ManagedDisplayInfo(int64_t id,
bool has_overscan)
: id_(id),
name_(name),
year_of_manufacture_(kInvalidYearOfManufacture),
has_overscan_(has_overscan),
active_rotation_source_(Display::RotationSource::UNKNOWN),
touch_support_(Display::TouchSupport::UNKNOWN),
Expand Down Expand Up @@ -312,6 +314,9 @@ Display::Rotation ManagedDisplayInfo::GetRotation(

void ManagedDisplayInfo::Copy(const ManagedDisplayInfo& native_info) {
DCHECK(id_ == native_info.id_);
manufacturer_id_ = native_info.manufacturer_id_;
product_id_ = native_info.product_id_;
year_of_manufacture_ = native_info.year_of_manufacture_;
name_ = native_info.name_;
has_overscan_ = native_info.has_overscan_;

Expand Down
12 changes: 12 additions & 0 deletions ui/display/manager/managed_display_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,15 @@ class DISPLAY_MANAGER_EXPORT ManagedDisplayInfo {
maximum_cursor_size_ = size;
}

const std::string& manufacturer_id() const { return manufacturer_id_; }
void set_manufacturer_id(const std::string& id) { manufacturer_id_ = id; }

const std::string& product_id() const { return product_id_; }
void set_product_id(const std::string& id) { product_id_ = id; }

int32_t year_of_manufacture() const { return year_of_manufacture_; }
void set_year_of_manufacture(int32_t year) { year_of_manufacture_ = year; }

// Returns a string representation of the ManagedDisplayInfo, excluding
// display modes.
std::string ToString() const;
Expand All @@ -264,6 +273,9 @@ class DISPLAY_MANAGER_EXPORT ManagedDisplayInfo {
private:
int64_t id_;
std::string name_;
std::string manufacturer_id_;
std::string product_id_;
int32_t year_of_manufacture_;
base::FilePath sys_path_;
bool has_overscan_;
std::map<Display::RotationSource, Display::Rotation> rotations_;
Expand Down
4 changes: 3 additions & 1 deletion ui/display/mojo/display_snapshot.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ struct DisplaySnapshot {
bool has_current_mode;
uint64 native_mode_index;
bool has_native_mode;
int64 product_id;
// |product_code| is a combination of the manufacturer id and the product id.
int64 product_code;
int32 year_of_manufacture;
gfx.mojom.Size maximum_cursor_size;
};
2 changes: 1 addition & 1 deletion ui/display/mojo/display_snapshot_struct_traits.cc
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ bool StructTraits<display::mojom::DisplaySnapshotDataView,
data.is_aspect_preserving_scaling(), data.has_overscan(),
data.has_color_correction_matrix(), color_space, display_name, file_path,
std::move(modes), std::move(edid), current_mode, native_mode,
data.product_id(), maximum_cursor_size);
data.product_code(), data.year_of_manufacture(), maximum_cursor_size);
return true;
}

Expand Down
9 changes: 7 additions & 2 deletions ui/display/mojo/display_snapshot_struct_traits.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,14 @@ struct StructTraits<display::mojom::DisplaySnapshotDataView,
return snapshot->native_mode() != nullptr;
}

static int64_t product_id(
static int64_t product_code(
const std::unique_ptr<display::DisplaySnapshot>& snapshot) {
return snapshot->product_id();
return snapshot->product_code();
}

static int32_t year_of_manufacture(
const std::unique_ptr<display::DisplaySnapshot>& snapshot) {
return snapshot->year_of_manufacture();
}

static const gfx::Size& maximum_cursor_size(
Expand Down
Loading

0 comments on commit 9b90891

Please sign in to comment.