Commit 7e0a617a authored by Miguel Casas's avatar Miguel Casas Committed by Commit Bot

ui: fix SDR/HDR headroom and SDR contrast ratio

After consultation with PMs, we have decided to have a bit more
brightness for SDR content (ToT: 50%, with this CL: 75%), hence
sacrificing a bit the HDR wow effect. This only applies for the
moment to kohaku (Samsung Galaxy Chromebook) and external
HDR-capable, DP-connected displays.

At the same time, using only 50%, or 75% of the brightness range
when in SDR, kills a bit the contrast ratio, which is perceived
as colors "not vivid", or greyish. This CL fixes that by adding
a bit (1.25) of gamma correction in this case in DrmDisplay.

With this CL, SDR colours look good (not washed out), and there's
no perceived brightness change in the SDR content during SDR-HDR
transitions.

Bug: 958166
Change-Id: I627514e8e31129b8b8d96e528e1e51ad8bea3d92
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2317508Reviewed-by: default avatarccameron <ccameron@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791310}
parent 714946fe
......@@ -97,7 +97,7 @@ gfx::DisplayColorSpaces FillDisplayColorSpaces(
gfx::ColorSpace::CreateSRGB(), DisplaySnapshot::PrimaryFormat());
if (allow_high_bit_depth) {
constexpr float kSDRJoint = 0.5;
constexpr float kSDRJoint = 0.75;
constexpr float kHDRLevel = 3.0;
const auto primary_id = snapshot_color_space.GetPrimaryID();
gfx::ColorSpace hdr_color_space;
......
......@@ -85,12 +85,13 @@ std::vector<drmModeModeInfo> GetDrmModeVector(drmModeConnector* connector) {
return modes;
}
void FillLinearValues(std::vector<display::GammaRampRGBEntry>* table,
size_t table_size,
float max_value) {
void FillPowerFunctionValues(std::vector<display::GammaRampRGBEntry>* table,
size_t table_size,
float max_value,
float exponent) {
for (size_t i = 0; i < table_size; i++) {
const uint16_t v =
max_value * std::numeric_limits<uint16_t>::max() * i / (table_size - 1);
const uint16_t v = max_value * std::numeric_limits<uint16_t>::max() *
pow((static_cast<float>(i) + 1) / table_size, exponent);
struct display::GammaRampRGBEntry gamma_entry = {v, v, v};
table->push_back(gamma_entry);
}
......@@ -271,12 +272,15 @@ void DrmDisplay::SetColorSpace(const gfx::ColorSpace& color_space) {
if (current_color_space_.IsHDR())
return CommitGammaCorrection(degamma, gamma);
// TODO(mcasas) This should be the same value as in DisplayChangeObservers's
// FillDisplayColorSpaces, move to a common place.
constexpr float kHDRLevel = 2.0;
// TODO(mcasas) This should be the inverse value of DisplayChangeObservers's
// FillDisplayColorSpaces's kHDRLevel, move to a common place.
constexpr float kSDRLevel = 0.75;
// TODO(mcasas): Retrieve this from the |drm_| HardwareDisplayPlaneManager.
constexpr size_t kNumGammaSamples = 16ul;
FillLinearValues(&gamma, kNumGammaSamples, 1.0 / kHDRLevel);
constexpr size_t kNumGammaSamples = 64ul;
// Only using kSDRLevel of the available values shifts the contrast ratio, we
// restore it via a smaller local gamma correction using this exponent.
constexpr float kExponent = 1.2;
FillPowerFunctionValues(&gamma, kNumGammaSamples, kSDRLevel, kExponent);
CommitGammaCorrection(degamma, gamma);
}
......
......@@ -19,26 +19,21 @@
using ::testing::_;
using ::testing::SizeIs;
// Verifies that the argument goes from 0 to the maximum uint16_t times |scale|.
MATCHER_P(MatchesLinearRamp, scale, "") {
// Verifies that the argument goes from 0 to the maximum uint16_t times |scale|
// following a power function with |exponent|.
MATCHER_P2(MatchesPowerFunction, scale, exponent, "") {
EXPECT_FALSE(arg.empty());
EXPECT_EQ(arg.front().r, 0);
EXPECT_EQ(arg.front().g, 0);
EXPECT_EQ(arg.front().b, 0);
const uint16_t max_value = std::numeric_limits<uint16_t>::max() * scale;
const auto middle_element = arg[arg.size() / 2];
const uint16_t middle_value = max_value * (arg.size() / 2) / (arg.size() - 1);
EXPECT_NEAR(middle_element.r, middle_value, 1);
EXPECT_NEAR(middle_element.g, middle_value, 1);
EXPECT_NEAR(middle_element.b, middle_value, 1);
const uint16_t last_value = max_value;
EXPECT_EQ(arg.back().r, last_value);
EXPECT_EQ(arg.back().g, last_value);
EXPECT_EQ(arg.back().b, last_value);
float i = 1.0;
for (const auto rgb_value : arg) {
const uint16_t expected_value = max_value * pow(i / arg.size(), exponent);
i++;
EXPECT_NEAR(rgb_value.r, expected_value, 1.0);
EXPECT_NEAR(rgb_value.g, expected_value, 1.0);
EXPECT_NEAR(rgb_value.b, expected_value, 1.0);
}
return true;
}
......@@ -154,10 +149,11 @@ TEST_F(DrmDisplayTest, SetColorSpace) {
drm_display_.SetColorSpace(kHDRColorSpace);
const auto kSDRColorSpace = gfx::ColorSpace::CreateREC709();
constexpr float kHDRLevel = 2.0;
EXPECT_CALL(
*plane_manager,
SetGammaCorrection(_, SizeIs(0), MatchesLinearRamp(1.0 / kHDRLevel)));
constexpr float kSDRLevel = 0.75;
constexpr float kExponent = 1.2;
EXPECT_CALL(*plane_manager,
SetGammaCorrection(_, SizeIs(0),
MatchesPowerFunction(kSDRLevel, kExponent)));
drm_display_.SetColorSpace(kSDRColorSpace);
}
......@@ -181,10 +177,11 @@ TEST_F(DrmDisplayTest, SetEmptyGammaCorrectionHDRDisplay) {
ON_CALL(*plane_manager, SetGammaCorrection(_, _, _))
.WillByDefault(::testing::Return(true));
constexpr float kHDRLevel = 2.0;
EXPECT_CALL(
*plane_manager,
SetGammaCorrection(_, SizeIs(0), MatchesLinearRamp(1.0 / kHDRLevel)));
constexpr float kSDRLevel = 0.75;
constexpr float kExponent = 1.2;
EXPECT_CALL(*plane_manager,
SetGammaCorrection(_, SizeIs(0),
MatchesPowerFunction(kSDRLevel, kExponent)));
drm_display_.SetGammaCorrection(std::vector<display::GammaRampRGBEntry>(),
std::vector<display::GammaRampRGBEntry>());
}
......
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