Skip to content

GitLab

T
tangled
Commit 1bd1b58e authored by Miguel Casas's avatar Miguel Casas Committed by Commit Bot
Browse files
Options

ozone/drm: wire EDID transfer and primaries to DisplaySnapshot 

This CL teaches drm_util to wire the TransferID and PrimaryID
from the parsed EDID to the gfx::ColorSpace. ColorSpace also
gets a new constructor here, to allow for specifying of a given
TransferID by name (which might be more performant than detailing
the actual function).

Histograms are added, unit tests extended.

Also took the chance to fix a teeny mistake in the EDID blob used
for edid_parser_unittest.cc, and that was landed in the predecessor
CL.

Bug: 1012846
Change-Id: I38c906f7b1a4522681766f829e99a654eb1848da
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1853866
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: default avatarStephen Chenney <schenney@chromium.org>
Reviewed-by: default avatarccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707662}
parent b80af864
Show whitespace changes
Inline Side-by-side
Showing with 115 additions and 10 deletions
tools/metrics/histograms/histograms.xml
View file @ 1bd1b58e
......@@ -36182,20 +36182,45 @@ uploading your change for review.
</summary>
</histogram>
<histogram name="DrmUtil.CreateDisplaySnapshot.BitsPerChannel" units="bits"
expires_after="M89">
<owner>andrescj@chromium.org</owner>
<owner>mcasas@chromium.org</owner>
<owner>chromeos-gfx@google.com</owner>
<summary>
Number of bits per channel described by a parsed EDID blob. This UMA is
recorded whenever the color space is extracted from an EDID blob.
</summary>
</histogram>
<histogram name="DrmUtil.CreateDisplaySnapshot.HasEdidBlob" enum="Boolean"
expires_after="2020-07-24">
<owner>andrescj@chromium.org</owner>
<owner>mcasas@chromium.org</owner>
<owner>chromeos-gfx@google.com</owner>
<summary>
Whether an EDID blob was detected. This UMA is recorded whenever we attempt
to parse the EDID from a display.
</summary>
</histogram>
<histogram name="DrmUtil.CreateDisplaySnapshot.IsHDR" enum="Boolean"
expires_after="M89">
<owner>andrescj@chromium.org</owner>
<owner>mcasas@chromium.org</owner>
<owner>chromeos-gfx@google.com</owner>
<summary>
Whether a EDID blob contained an HDR transfer function (e.g. SMPT SE2084 or
HLG). This UMA is recorded whenever the color space is extracted from an
EDID blob.
</summary>
</histogram>
<histogram name="DrmUtil.GetColorSpaceFromEdid.ChecksOutcome"
enum="EdidColorSpaceChecksOutcome" expires_after="2020-07-24">
<owner>andrescj@chromium.org</owner>
<owner>mcasas@chromium.org</owner>
<owner>chromeos-gfx@google.com</owner>
<summary>
When attempting to get the color space from an EDID blob, whether the sanity
checks passed (and if not, which check failed). This UMA is recorded
ui/display/util/edid_parser_unittest.cc
View file @ 1bd1b58e
......@@ -165,7 +165,7 @@ constexpr unsigned char kHDRMetadata[] =
"\x00\x53\x41\x4d\x53\x55\x4e\x47\x0a\x20\x20\x20\x20\x20\x01\x5a"
"\x02\x03\x4f\xf0\x53\x5f\x10\x1f\x04\x13\x05\x14\x20\x21\x22\x5d"
"\x5e\x62\x63\x64\x07\x16\x03\x12\x2c\x09\x07\x07\x15\x07\x50\x3d"
"\x04\xc0\x57\x07\x00\x83\x01\x00\x00\xe2\x00\x0f\xe3\x05\x03\x01"
"\x04\xc0\x57\x07\x00\x83\x01\x00\x00\xe2\x00\x0f\xe3\x05\x83\x01"
"\x6e\x03\x0c\x00\x30\x00\xb8\x3c\x20\x00\x80\x01\x02\x03\x04\xe3"
"\x06\x0d\x01\xe5\x0e\x60\x61\x65\x66\xe5\x01\x8b\x84\x90\x01\x01"
"\x1d\x80\xd0\x72\x1c\x16\x20\x10\x2c\x25\x80\x50\x1d\x74\x00\x00"
......@@ -414,7 +414,8 @@ struct TestParams {
21442559853606400,
"SAM",
"0DF6",
{gfx::ColorSpace::PrimaryID::BT709, gfx::ColorSpace::PrimaryID::SMPTE170M},
{gfx::ColorSpace::PrimaryID::BT709, gfx::ColorSpace::PrimaryID::SMPTE170M,
gfx::ColorSpace::PrimaryID::BT2020},
{gfx::ColorSpace::TransferID::BT709,
gfx::ColorSpace::TransferID::SMPTEST2084,
gfx::ColorSpace::TransferID::ARIB_STD_B67},
......
ui/gfx/color_space.cc
View file @ 1bd1b58e
......@@ -93,6 +93,15 @@ ColorSpace ColorSpace::CreateCustom(const skcms_Matrix3x3& to_XYZD50,
return result;
}
// static
ColorSpace ColorSpace::CreateCustom(const skcms_Matrix3x3& to_XYZD50,
TransferID transfer) {
ColorSpace result(ColorSpace::PrimaryID::CUSTOM, transfer,
ColorSpace::MatrixID::RGB, ColorSpace::RangeID::FULL);
result.SetCustomPrimaries(to_XYZD50);
return result;
}
void ColorSpace::SetCustomPrimaries(const skcms_Matrix3x3& to_XYZD50) {
const PrimaryID kIDsToCheck[] = {
PrimaryID::BT709,
......
ui/gfx/color_space.h
View file @ 1bd1b58e
......@@ -151,6 +151,7 @@ class COLOR_SPACE_EXPORT ColorSpace {
const skcms_TransferFunction& fn,
MatrixID matrix,
RangeID full_range);
explicit ColorSpace(const SkColorSpace& sk_color_space);
// Returns true if this is not the default-constructor object.
......@@ -167,6 +168,8 @@ class COLOR_SPACE_EXPORT ColorSpace {
}
static ColorSpace CreateCustom(const skcms_Matrix3x3& to_XYZD50,
const skcms_TransferFunction& fn);
static ColorSpace CreateCustom(const skcms_Matrix3x3& to_XYZD50,
TransferID transfer);
static constexpr ColorSpace CreateXYZD50() {
return ColorSpace(PrimaryID::XYZ_D50, TransferID::LINEAR, MatrixID::RGB,
RangeID::FULL);
......@@ -220,7 +223,7 @@ class COLOR_SPACE_EXPORT ColorSpace {
size_t GetHash() const;
std::string ToString() const;
// Returns true if the decoded values can be outside of the 0.0-1.0 range.
// Returns true if the transfer function is an HDR one (SMPTE 2084, HLG, etc).
bool IsHDR() const;
// Returns true if the encoded values can be outside of the 0.0-1.0 range.
......
ui/ozone/platform/drm/common/drm_util.cc
View file @ 1bd1b58e
......@@ -15,7 +15,7 @@
#include <utility>
#include "base/containers/flat_map.h"
#include "base/metrics/histogram_macros.h"
#include "base/metrics/histogram_functions.h"
#include "ui/display/types/display_constants.h"
#include "ui/display/types/display_mode.h"
#include "ui/display/util/edid_parser.h"
......@@ -30,7 +30,7 @@ static const size_t kDefaultCursorHeight = 64;
// Used in the GetColorSpaceFromEdid function to collect data on whether the
// color space extracted from an EDID blob passed the sanity checks.
void EmitEdidColorSpaceChecksOutcomeUma(EdidColorSpaceChecksOutcome outcome) {
UMA_HISTOGRAM_ENUMERATION("DrmUtil.GetColorSpaceFromEdid.ChecksOutcome",
base::UmaHistogramEnumeration("DrmUtil.GetColorSpaceFromEdid.ChecksOutcome",
outcome);
}
......@@ -466,7 +466,7 @@ std::unique_ptr<display::DisplaySnapshot> CreateDisplaySnapshot(
ScopedDrmPropertyBlobPtr edid_blob(
GetDrmPropertyBlob(fd, info->connector(), "EDID"));
UMA_HISTOGRAM_BOOLEAN("DrmUtil.CreateDisplaySnapshot.HasEdidBlob",
base::UmaHistogramBoolean("DrmUtil.CreateDisplaySnapshot.HasEdidBlob",
!!edid_blob);
std::vector<uint8_t> edid;
if (edid_blob) {
......@@ -482,7 +482,11 @@ std::unique_ptr<display::DisplaySnapshot> CreateDisplaySnapshot(
has_overscan =
edid_parser.has_overscan_flag() && edid_parser.overscan_flag();
display_color_space = GetColorSpaceFromEdid(edid_parser);
base::UmaHistogramBoolean("DrmUtil.CreateDisplaySnapshot.IsHDR",
display_color_space.IsHDR());
bits_per_channel = std::max(edid_parser.bits_per_channel(), 0);
base::UmaHistogramCounts100("DrmUtil.CreateDisplaySnapshot.BitsPerChannel",
bits_per_channel);
} else {
VLOG(1) << "Failed to get EDID blob for connector "
<< info->connector()->connector_id;
......@@ -701,9 +705,29 @@ gfx::ColorSpace GetColorSpaceFromEdid(const display::EdidParser& edid_parser) {
EdidColorSpaceChecksOutcome::kErrorBadGamma);
return gfx::ColorSpace();
}
EmitEdidColorSpaceChecksOutcomeUma(EdidColorSpaceChecksOutcome::kSuccess);
gfx::ColorSpace::TransferID transfer_id =
gfx::ColorSpace::TransferID::INVALID;
if (base::Contains(edid_parser.supported_color_primary_ids(),
gfx::ColorSpace::PrimaryID::BT2020)) {
if (base::Contains(edid_parser.supported_color_transfer_ids(),
gfx::ColorSpace::TransferID::SMPTEST2084)) {
transfer_id = gfx::ColorSpace::TransferID::SMPTEST2084;
} else if (base::Contains(edid_parser.supported_color_transfer_ids(),
gfx::ColorSpace::TransferID::ARIB_STD_B67)) {
transfer_id = gfx::ColorSpace::TransferID::ARIB_STD_B67;
}
} else if (gamma == 2.2f) {
transfer_id = gfx::ColorSpace::TransferID::GAMMA22;
} else if (gamma == 2.4f) {
transfer_id = gfx::ColorSpace::TransferID::GAMMA24;
}
if (transfer_id != gfx::ColorSpace::TransferID::INVALID)
return gfx::ColorSpace::CreateCustom(color_space_as_matrix, transfer_id);
skcms_TransferFunction transfer = {gamma, 1.f, 0.f, 0.f, 0.f, 0.f, 0.f};
EmitEdidColorSpaceChecksOutcomeUma(EdidColorSpaceChecksOutcome::kSuccess);
return gfx::ColorSpace::CreateCustom(color_space_as_matrix, transfer);
}
......
ui/ozone/platform/drm/common/drm_util_unittest.cc
View file @ 1bd1b58e
......@@ -64,6 +64,25 @@ const unsigned char kEve[] =
"\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";
// A Samsung monitor that supports HDR metadata.
constexpr unsigned char kHDR[] =
"\x00\xff\xff\xff\xff\xff\xff\x00\x4c\x2d\xf6\x0d\x00\x0e\x00\x01"
"\x01\x1b\x01\x03\x80\x5f\x36\x78\x0a\x23\xad\xa4\x54\x4d\x99\x26"
"\x0f\x47\x4a\xbd\xef\x80\x71\x4f\x81\xc0\x81\x00\x81\x80\x95\x00"
"\xa9\xc0\xb3\x00\x01\x01\x04\x74\x00\x30\xf2\x70\x5a\x80\xb0\x58"
"\x8a\x00\x50\x1d\x74\x00\x00\x1e\x02\x3a\x80\x18\x71\x38\x2d\x40"
"\x58\x2c\x45\x00\x50\x1d\x74\x00\x00\x1e\x00\x00\x00\xfd\x00\x18"
"\x4b\x0f\x51\x1e\x00\x0a\x20\x20\x20\x20\x20\x20\x00\x00\x00\xfc"
"\x00\x53\x41\x4d\x53\x55\x4e\x47\x0a\x20\x20\x20\x20\x20\x01\x5a"
"\x02\x03\x4f\xf0\x53\x5f\x10\x1f\x04\x13\x05\x14\x20\x21\x22\x5d"
"\x5e\x62\x63\x64\x07\x16\x03\x12\x2c\x09\x07\x07\x15\x07\x50\x3d"
"\x04\xc0\x57\x07\x00\x83\x01\x00\x00\xe2\x00\x0f\xe3\x05\x83\x01"
"\x6e\x03\x0c\x00\x30\x00\xb8\x3c\x20\x00\x80\x01\x02\x03\x04\xe3"
"\x06\x0d\x01\xe5\x0e\x60\x61\x65\x66\xe5\x01\x8b\x84\x90\x01\x01"
"\x1d\x80\xd0\x72\x1c\x16\x20\x10\x2c\x25\x80\x50\x1d\x74\x00\x00"
"\x9e\x66\x21\x56\xaa\x51\x00\x1e\x30\x46\x8f\x33\x00\x50\x1d\x74"
"\x00\x00\x1e\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xbd";
// Partially valid EDID: gamma information is marked as non existent.
const unsigned char kEdidWithNoGamma[] =
"\x00\xFF\xFF\xFF\xFF\xFF\xFF\x00\x22\xF0\x76\x26\x01\x01\x01\x01"
......@@ -391,6 +410,30 @@ TEST_F(DrmUtilTest, GetColorSpaceFromEdid) {
EdidColorSpaceChecksOutcome::kSuccess),
3);
// Test with a display that supports HDR.
constexpr SkColorSpacePrimaries expected_hdr_primaries = {.fRX = 0.640625f,
.fRY = 0.330078f,
.fGX = 0.300781f,
.fGY = 0.600586f,
.fBX = 0.150391f,
.fBY = 0.060547f,
.fWX = 0.280273f,
.fWY = 0.290039f};
skcms_Matrix3x3 expected_hdr_toXYZ50_matrix;
expected_hdr_primaries.toXYZD50(&expected_hdr_toXYZ50_matrix);
const std::vector<uint8_t> hdr_edid(kHDR, kHDR + base::size(kHDR) - 1);
const gfx::ColorSpace expected_hdr_color_space =
gfx::ColorSpace::CreateCustom(expected_hdr_toXYZ50_matrix,
gfx::ColorSpace::TransferID::SMPTEST2084);
EXPECT_TRUE(expected_hdr_color_space.IsHDR());
EXPECT_EQ(expected_hdr_color_space.ToString(),
GetColorSpaceFromEdid(display::EdidParser(hdr_edid)).ToString());
histogram_tester.ExpectBucketCount(
"DrmUtil.GetColorSpaceFromEdid.ChecksOutcome",
static_cast<base::HistogramBase::Sample>(
EdidColorSpaceChecksOutcome::kSuccess),
4);
// Test with gamma marked as non-existent.
const std::vector<uint8_t> no_gamma_edid(
kEdidWithNoGamma, kEdidWithNoGamma + base::size(kEdidWithNoGamma) - 1);
......@@ -403,7 +446,7 @@ TEST_F(DrmUtilTest, GetColorSpaceFromEdid) {
EdidColorSpaceChecksOutcome::kErrorBadGamma),
1);
histogram_tester.ExpectTotalCount(
"DrmUtil.GetColorSpaceFromEdid.ChecksOutcome", 4);
"DrmUtil.GetColorSpaceFromEdid.ChecksOutcome", 5);
}
TEST_F(DrmUtilTest, GetInvalidColorSpaceFromEdid) {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment