Skip to content

Commit

Permalink
gfx::ColorSpace: Clean up SDR white level handling
Browse files Browse the repository at this point in the history
gfx::ColorSpace no longer stores an SDR white level for PQ and HLG
content. Remove the ability to set or query it.

Rename SCRGBLinear to SRGBLinear, to match the CSS color name, and to
avoid confusion with SCRGBLinear80Nits.

Bug: 1286076
Change-Id: Iccdc0c426008e23fe89323a5f5def53770fc26b8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3642542
Commit-Queue: ccameron chromium <ccameron@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1002748}
  • Loading branch information
ccameron-chromium authored and Chromium LUCI CQ committed May 12, 2022
1 parent f968507 commit a1fc9b1
Show file tree
Hide file tree
Showing 20 changed files with 45 additions and 220 deletions.
2 changes: 1 addition & 1 deletion components/viz/service/display/display_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ TEST_F(DisplayTest, DisplayDamaged) {
settings.partial_swap_enabled = true;
SetUpSoftwareDisplay(settings);
gfx::ColorSpace color_space_1 = gfx::ColorSpace::CreateXYZD50();
gfx::ColorSpace color_space_2 = gfx::ColorSpace::CreateSCRGBLinear();
gfx::ColorSpace color_space_2 = gfx::ColorSpace::CreateSRGBLinear();
gfx::DisplayColorSpaces color_spaces_1(color_space_1);
gfx::DisplayColorSpaces color_spaces_2(color_space_2);

Expand Down
9 changes: 0 additions & 9 deletions components/viz/service/display/gl_renderer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3700,15 +3700,6 @@ void GLRenderer::SetUseProgram(const ProgramKey& program_key_no_color,
absl::optional<gfx::HDRMetadata> hdr_metadata) {
DCHECK(dst_color_space.IsValid());
gfx::ColorSpace adjusted_src_color_space = src_color_space;
if (adjust_src_white_level && src_color_space.IsHDR()) {
// TODO(b/183236148): consider using the destination's HDR static metadata
// in current_frame()->display_color_spaces.hdr_static_metadata() and the
// color volume metadata in |src_hdr_metadata| for the tone mapping; e.g.
// the content might be mastered in 0-1000 nits but the display only be able
// to represent 0 to 500.
adjusted_src_color_space = src_color_space.GetWithSDRWhiteLevel(
current_frame()->display_color_spaces.GetSDRMaxLuminanceNits());
}

ProgramKey program_key = program_key_no_color;
const gfx::ColorTransform* color_transform =
Expand Down
8 changes: 1 addition & 7 deletions components/viz/service/display/renderer_pixeltest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4726,12 +4726,6 @@ class ColorTransformPixelTest
}
this->display_color_spaces_ =
gfx::DisplayColorSpaces(this->dst_color_space_);
float sdr_max_luminance_nits =
this->display_color_spaces_.GetSDRMaxLuminanceNits();
if (src_color_space_.GetSDRWhiteLevel(&sdr_max_luminance_nits)) {
this->display_color_spaces_.SetSDRMaxLuminanceNits(
sdr_max_luminance_nits);
}
this->premultiplied_alpha_ = std::get<3>(GetParam());
}

Expand Down Expand Up @@ -4773,7 +4767,7 @@ class ColorTransformPixelTest
}

gfx::ColorTransform::Options options;
options.sdr_max_luminance_nits = gfx::ColorSpace::kDefaultSDRWhiteLevelV2;
options.sdr_max_luminance_nits = gfx::ColorSpace::kDefaultSDRWhiteLevel;
std::unique_ptr<gfx::ColorTransform> transform =
gfx::ColorTransform::NewColorTransform(this->src_color_space_,
this->dst_color_space_, options);
Expand Down
6 changes: 3 additions & 3 deletions components/viz/service/display/surface_aggregator_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6000,13 +6000,13 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, ColorSpaceTestWin) {
gfx::BufferFormat::RGBA_8888);
display_color_spaces.SetOutputColorSpaceAndBufferFormat(
gfx::ContentColorUsage::kWideColorGamut, true /* needs_alpha */,
gfx::ColorSpace::CreateSCRGBLinear(), gfx::BufferFormat::RGBA_8888);
gfx::ColorSpace::CreateSRGBLinear(), gfx::BufferFormat::RGBA_8888);
display_color_spaces.SetOutputColorSpaceAndBufferFormat(
gfx::ContentColorUsage::kHDR, false /* needs_alpha */,
gfx::ColorSpace::CreateHDR10(), gfx::BufferFormat::BGRA_1010102);
display_color_spaces.SetOutputColorSpaceAndBufferFormat(
gfx::ContentColorUsage::kHDR, true /* needs_alpha */,
gfx::ColorSpace::CreateSCRGBLinear(), gfx::BufferFormat::RGBA_F16);
gfx::ColorSpace::CreateSRGBLinear(), gfx::BufferFormat::RGBA_F16);

std::vector<Pass> passes = {
Pass(quads[0], CompositorRenderPassId{2}, kSurfaceSize),
Expand Down Expand Up @@ -6085,7 +6085,7 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, ColorSpaceTestWin) {
gfx::BufferFormat::BGRA_1010102);
display_color_spaces.SetOutputColorSpaceAndBufferFormat(
gfx::ContentColorUsage::kHDR, true /* needs_alpha */,
gfx::ColorSpace::CreateSCRGBLinear(), gfx::BufferFormat::RGBA_F16);
gfx::ColorSpace::CreateSRGBLinear(), gfx::BufferFormat::RGBA_F16);

// Opaque content renders to the appropriate space directly.
passes[1].has_transparent_background = false;
Expand Down
2 changes: 1 addition & 1 deletion media/gpu/android/video_frame_factory_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ class VideoFrameFactoryImplTest : public testing::Test {
gfx::Size coded_size{100, 100};
gfx::Rect visible_rect{coded_size};
gfx::Size natural_size{coded_size};
gfx::ColorSpace color_space{gfx::ColorSpace::CreateSCRGBLinear()};
gfx::ColorSpace color_space{gfx::ColorSpace::CreateSRGBLinear()};
} video_frame_params_;

void RequestVideoFrame() {
Expand Down
4 changes: 2 additions & 2 deletions media/gpu/windows/d3d11_copying_texture_wrapper_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ TEST_P(D3D11CopyingTexture2DWrapperTest,
// TODO: check |gpu_task_runner_|.

MailboxHolderArray mailboxes;
gfx::ColorSpace input_color_space = gfx::ColorSpace::CreateSCRGBLinear();
gfx::ColorSpace input_color_space = gfx::ColorSpace::CreateSRGBLinear();
gfx::ColorSpace output_color_space;
EXPECT_EQ(wrapper
->Init(gpu_task_runner_, CreateMockHelperCB(),
Expand Down Expand Up @@ -292,7 +292,7 @@ TEST_P(D3D11CopyingTexture2DWrapperTest, HDRMetadataIsSentToVideoProcessor) {
MockVideoProcessorProxy* processor_raw = processor.get();
auto wrapper = std::make_unique<CopyingTexture2DWrapper>(
gfx::Size(100, 200), ExpectTextureWrapper(), std::move(processor),
nullptr, gfx::ColorSpace::CreateSCRGBLinear());
nullptr, gfx::ColorSpace::CreateSRGBLinear());

const DXGI_HDR_METADATA_HDR10 dxgi_metadata =
gl::HDRMetadataHelperWin::HDRMetadataToDXGI(metadata);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,7 @@ TEST(MediaQueryEvaluatorTest, CachedDynamicRange) {
}
{
data.device_supports_hdr =
gfx::DisplayColorSpaces(gfx::ColorSpace::CreateSCRGBLinear())
gfx::DisplayColorSpaces(gfx::ColorSpace::CreateSRGBLinear())
.SupportsHDR();
MediaValues* media_values = MakeGarbageCollected<MediaValuesCached>(data);
MediaQueryEvaluator media_query_evaluator(media_values);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ gfx::ColorSpace PredefinedColorSpaceToGfxColorSpace(
return gfx::ColorSpace(gfx::ColorSpace::PrimaryID::BT2020,
gfx::ColorSpace::TransferID::PQ);
case PredefinedColorSpace::kSRGBLinear:
return gfx::ColorSpace::CreateSCRGBLinear();
return gfx::ColorSpace::CreateSRGBLinear();
}
NOTREACHED();
}
Expand Down
2 changes: 1 addition & 1 deletion ui/display/mac/screen_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ DisplayMac BuildDisplayForScreen(NSScreen* screen) {
display.set_color_spaces(display_color_spaces);
}
display_color_spaces.SetSDRMaxLuminanceNits(
gfx::ColorSpace::kDefaultSDRWhiteLevelV2);
gfx::ColorSpace::kDefaultSDRWhiteLevel);

if (enable_hdr) {
display.set_color_depth(Display::kHDR10BitsPerPixel);
Expand Down
2 changes: 1 addition & 1 deletion ui/display/manager/display_change_observer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ TEST_P(DisplayChangeObserverTest, HDRDisplayColorSpaces) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature(features::kUseHDRTransferFunction);

const auto display_color_space = gfx::ColorSpace::CreateHDR10(100.0f);
const auto display_color_space = gfx::ColorSpace::CreateHDR10();
const std::unique_ptr<DisplaySnapshot> display_snapshot =
FakeDisplaySnapshot::Builder()
.SetId(123)
Expand Down
2 changes: 1 addition & 1 deletion ui/display/util/display_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ gfx::ColorSpace ForcedColorProfileStringToColorSpace(const std::string& value) {
if (value == "display-p3-d65")
return gfx::ColorSpace::CreateDisplayP3D65();
if (value == "scrgb-linear")
return gfx::ColorSpace::CreateSCRGBLinear();
return gfx::ColorSpace::CreateSRGBLinear();
if (value == "hdr10")
return gfx::ColorSpace::CreateHDR10();
if (value == "extended-srgb")
Expand Down
6 changes: 3 additions & 3 deletions ui/display/win/screen_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ gfx::DisplayColorSpaces GetForcedDisplayColorSpaces() {
// space is scRGB linear (defaults to 80 nits) or PQ (defaults to 100 nits).
const auto& color_space = GetForcedDisplayColorProfile();
auto display_color_spaces = CreateDisplayColorSpaces(
color_space, gfx::ColorSpace::kDefaultSDRWhiteLevelV2);
color_space, gfx::ColorSpace::kDefaultSDRWhiteLevel);
// Use the forced color profile's buffer format for all content usages.
if (color_space.GetTransferID() == gfx::ColorSpace::TransferID::PQ) {
display_color_spaces.SetOutputBufferFormats(
Expand Down Expand Up @@ -303,7 +303,7 @@ Display CreateDisplayFromDisplayInfo(
hdr_max_luminance_relative =
dxgi_output_desc->max_luminance / sdr_white_level;
if (!dxgi_output_desc->hdr_enabled)
sdr_white_level = gfx::ColorSpace::kDefaultSDRWhiteLevelV2;
sdr_white_level = gfx::ColorSpace::kDefaultSDRWhiteLevel;
}
hdr_max_luminance_relative = std::max(hdr_max_luminance_relative,
kMinHDRCapableMaxLuminanceRelative);
Expand All @@ -312,7 +312,7 @@ Display CreateDisplayFromDisplayInfo(
} else {
color_spaces = CreateDisplayColorSpaces(
color_profile_reader->GetDisplayColorSpace(display.id()),
gfx::ColorSpace::kDefaultSDRWhiteLevelV2);
gfx::ColorSpace::kDefaultSDRWhiteLevel);
}
if (color_spaces.SupportsHDR()) {
// These are (ab)used by pages via media query APIs to detect HDR support.
Expand Down
2 changes: 1 addition & 1 deletion ui/gfx/android/android_surface_control_compat.cc
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ uint64_t ColorSpaceToADataSpace(const gfx::ColorSpace& color_space) {
if (!color_space.IsValid() || color_space == gfx::ColorSpace::CreateSRGB())
return ADATASPACE_SRGB;

if (color_space == gfx::ColorSpace::CreateSCRGBLinear())
if (color_space == gfx::ColorSpace::CreateSRGBLinear())
return ADATASPACE_SCRGB_LINEAR;

if (color_space == gfx::ColorSpace::CreateDisplayP3D65())
Expand Down
82 changes: 3 additions & 79 deletions ui/gfx/color_space.cc
Original file line number Diff line number Diff line change
Expand Up @@ -81,23 +81,6 @@ skcms_TransferFunction GetHLGSkTransferFunction(float sdr_white_level) {
return fn;
}

float GetSDRWhiteLevelFromPQSkTransferFunction(
const skcms_TransferFunction& fn) {
DCHECK_EQ(fn.g, SkNamedTransferFn::kPQ.g);
const double ws_a = static_cast<double>(fn.a) / SkNamedTransferFn::kPQ.a;
const double w_a = pow(ws_a, fn.f);
const double sdr_white_level_a = 10000.0f / w_a;
return sdr_white_level_a;
}

float GetSDRWhiteLevelFromHLGSkTransferFunction(
const skcms_TransferFunction& fn) {
DCHECK_EQ(fn.g, SkNamedTransferFn::kHLG.g);
if (fn.f == 0)
return ColorSpace::kDefaultSDRWhiteLevel;
return 1.0f / ((fn.f + 1) / ColorSpace::kDefaultSDRWhiteLevel);
}

bool PrimaryIdContainsSRGB(ColorSpace::PrimaryID id) {
DCHECK(id != ColorSpace::PrimaryID::INVALID &&
id != ColorSpace::PrimaryID::CUSTOM);
Expand Down Expand Up @@ -152,10 +135,8 @@ ColorSpace::ColorSpace(const SkColorSpace& sk_color_space, bool is_hdr)
SetCustomTransferFunction(fn, is_hdr);
} else if (skcms_TransferFunction_isHLGish(&fn)) {
transfer_ = TransferID::HLG;
transfer_params_[0] = GetSDRWhiteLevelFromHLGSkTransferFunction(fn);
} else if (skcms_TransferFunction_isPQish(&fn)) {
transfer_ = TransferID::PQ;
transfer_params_[0] = GetSDRWhiteLevelFromPQSkTransferFunction(fn);
} else {
// Construct an invalid result: Unable to extract necessary parameters
return;
Expand All @@ -174,29 +155,6 @@ bool ColorSpace::IsValid() const {
matrix_ != MatrixID::INVALID && range_ != RangeID::INVALID;
}

// static
ColorSpace ColorSpace::CreateSCRGBLinear(float sdr_white_level) {
skcms_TransferFunction fn = {0};
fn.g = 1.0f;
fn.a = kDefaultScrgbLinearSdrWhiteLevel / sdr_white_level;
return ColorSpace(PrimaryID::BT709, TransferID::CUSTOM_HDR, MatrixID::RGB,
RangeID::FULL, nullptr, &fn);
}

// static
ColorSpace ColorSpace::CreateHDR10(float sdr_white_level) {
ColorSpace result(PrimaryID::BT2020, TransferID::PQ, MatrixID::RGB,
RangeID::FULL);
result.transfer_params_[0] = sdr_white_level;
return result;
}

// static
ColorSpace ColorSpace::CreateHLG() {
return ColorSpace(PrimaryID::BT2020, TransferID::HLG, MatrixID::RGB,
RangeID::FULL);
}

// static
ColorSpace ColorSpace::CreatePiecewiseHDR(
PrimaryID primaries,
Expand Down Expand Up @@ -325,9 +283,6 @@ size_t ColorSpace::TransferParamCount(TransferID transfer) {
return 7;
case TransferID::PIECEWISE_HDR:
return 2;
case TransferID::PQ:
case TransferID::HLG:
return 1;
default:
return 0;
}
Expand Down Expand Up @@ -570,8 +525,7 @@ std::string ColorSpace::ToString() const {
GetTransferFunction(&fn);
if (fn.g == 1.0f && fn.a > 0.0f && fn.b == 0.0f && fn.c == 0.0f &&
fn.d == 0.0f && fn.e == 0.0f && fn.f == 0.0f) {
ss << "LINEAR_HDR (slope " << fn.a << ", SDR white point "
<< kDefaultScrgbLinearSdrWhiteLevel / fn.a << " nits)";
ss << "LINEAR_HDR (slope " << fn.a << ")";
break;
}
ss << fn.c << "*x + " << fn.f << " if |x| < " << fn.d << " else sign(x)*("
Expand Down Expand Up @@ -694,25 +648,6 @@ ColorSpace ColorSpace::GetWithMatrixAndRange(MatrixID matrix,
return result;
}

ColorSpace ColorSpace::GetWithSDRWhiteLevel(float sdr_white_level) const {
ColorSpace result = *this;
if (transfer_ == TransferID::PQ || transfer_ == TransferID::HLG) {
result.transfer_params_[0] = sdr_white_level;
} else if (transfer_ == TransferID::SCRGB_LINEAR_80_NITS) {
skcms_TransferFunction fn = {0};
GetTransferFunction(&fn, sdr_white_level);
result.transfer_ = TransferID::CUSTOM_HDR;
result.SetCustomTransferFunction(fn, false);
} else if (transfer_ == TransferID::LINEAR_HDR) {
result.transfer_ = TransferID::CUSTOM_HDR;
skcms_TransferFunction fn = {0};
fn.g = 1.f;
fn.a = kDefaultScrgbLinearSdrWhiteLevel / sdr_white_level;
result.SetCustomTransferFunction(fn, false);
}
return result;
}

sk_sp<SkColorSpace> ColorSpace::ToSkColorSpace(
absl::optional<float> sdr_white_level) const {
// Handle only valid, full-range RGB spaces.
Expand All @@ -737,11 +672,11 @@ sk_sp<SkColorSpace> ColorSpace::ToSkColorSpace(
break;
case TransferID::HLG:
transfer_fn = GetHLGSkTransferFunction(
sdr_white_level.value_or(kDefaultSDRWhiteLevelV2));
sdr_white_level.value_or(kDefaultSDRWhiteLevel));
break;
case TransferID::PQ:
transfer_fn = GetPQSkTransferFunction(
sdr_white_level.value_or(kDefaultSDRWhiteLevelV2));
sdr_white_level.value_or(kDefaultSDRWhiteLevel));
break;
default:
if (!GetTransferFunction(&transfer_fn, sdr_white_level)) {
Expand Down Expand Up @@ -1172,17 +1107,6 @@ bool ColorSpace::GetInverseTransferFunction(
return true;
}

bool ColorSpace::GetSDRWhiteLevel(float* sdr_white_level) const {
if (transfer_ != TransferID::PQ && transfer_ != TransferID::HLG) {
return false;
}
if (transfer_params_[0] == 0.0f)
*sdr_white_level = kDefaultSDRWhiteLevel;
else
*sdr_white_level = transfer_params_[0];
return true;
}

bool ColorSpace::GetPiecewiseHDRParams(float* sdr_joint,
float* hdr_level) const {
if (transfer_ != TransferID::PIECEWISE_HDR)
Expand Down

0 comments on commit a1fc9b1

Please sign in to comment.