From eb7b04e320160621c8d51c9df323942e106b99a6 Mon Sep 17 00:00:00 2001 From: Xaver Hugl Date: Thu, 6 Jun 2024 14:27:18 +0200 Subject: [PATCH] core/colorspace: make max brightness values optional Zero is already optional, but it's easy to make mistakes that way --- src/backends/drm/drm_egl_layer_surface.cpp | 2 +- src/backends/drm/drm_output.cpp | 2 +- src/core/colorspace.cpp | 12 ++++++------ src/core/colorspace.h | 16 ++++++++-------- src/opengl/glshader.cpp | 2 +- src/wayland/frog_colormanagement_v1.cpp | 13 ++++++++----- src/wayland/frog_colormanagement_v1.h | 4 ++-- 7 files changed, 27 insertions(+), 24 deletions(-) diff --git a/src/backends/drm/drm_egl_layer_surface.cpp b/src/backends/drm/drm_egl_layer_surface.cpp index 18d513b1c1..98d8664bb4 100644 --- a/src/backends/drm/drm_egl_layer_surface.cpp +++ b/src/backends/drm/drm_egl_layer_surface.cpp @@ -202,7 +202,7 @@ bool EglGbmLayerSurface::endRendering(const QRegion &damagedRegion, OutputFrame binder.shader()->setUniform(GLShader::IntUniform::SourceNamedTransferFunction, int(m_surface->intermediaryColorDescription.transferFunction())); binder.shader()->setUniform(GLShader::IntUniform::DestinationNamedTransferFunction, int(m_surface->targetColorDescription.transferFunction())); binder.shader()->setUniform(GLShader::FloatUniform::SdrBrightness, m_surface->intermediaryColorDescription.sdrBrightness()); - binder.shader()->setUniform(GLShader::FloatUniform::MaxHdrBrightness, m_surface->intermediaryColorDescription.maxHdrHighlightBrightness()); + binder.shader()->setUniform(GLShader::FloatUniform::MaxHdrBrightness, m_surface->intermediaryColorDescription.maxHdrHighlightBrightness().value_or(800)); } QMatrix4x4 mat; mat.scale(1, -1); diff --git a/src/backends/drm/drm_output.cpp b/src/backends/drm/drm_output.cpp index bcce668e02..a86acfcddd 100644 --- a/src/backends/drm/drm_output.cpp +++ b/src/backends/drm/drm_output.cpp @@ -382,7 +382,7 @@ ColorDescription DrmOutput::createColorDescription(const std::shared_ptrminBrightnessOverride.value_or(m_state.minBrightnessOverride).value_or(m_connector->edid()->desiredMinLuminance()) : 0; const double maxAverageBrightness = effectiveHdr ? props->maxAverageBrightnessOverride.value_or(m_state.maxAverageBrightnessOverride).value_or(m_connector->edid()->desiredMaxFrameAverageLuminance().value_or(m_state.sdrBrightness)) : 200; - const double maxPeakBrightness = effectiveHdr ? props->maxPeakBrightnessOverride.value_or(m_state.maxPeakBrightnessOverride).value_or(m_connector->edid()->desiredMaxLuminance().value_or(1000)) : 200; + const double maxPeakBrightness = effectiveHdr ? props->maxPeakBrightnessOverride.value_or(m_state.maxPeakBrightnessOverride).value_or(m_connector->edid()->desiredMaxLuminance().value_or(800)) : 200; const double sdrBrightness = effectiveHdr ? props->sdrBrightness.value_or(m_state.sdrBrightness) : maxPeakBrightness; return ColorDescription(containerColorimetry, transferFunction, sdrBrightness, minBrightness, maxAverageBrightness, maxPeakBrightness, masteringColorimetry, sdrColorimetry); } diff --git a/src/core/colorspace.cpp b/src/core/colorspace.cpp index bd04376af5..444909ddb6 100644 --- a/src/core/colorspace.cpp +++ b/src/core/colorspace.cpp @@ -200,17 +200,17 @@ const Colorimetry &Colorimetry::fromName(NamedColorimetry name) const ColorDescription ColorDescription::sRGB = ColorDescription(NamedColorimetry::BT709, NamedTransferFunction::gamma22, 100, 0, 100, 100); -ColorDescription::ColorDescription(const Colorimetry &containerColorimetry, NamedTransferFunction tf, double sdrBrightness, double minHdrBrightness, double maxFrameAverageBrightness, double maxHdrHighlightBrightness) +ColorDescription::ColorDescription(const Colorimetry &containerColorimetry, NamedTransferFunction tf, double sdrBrightness, double minHdrBrightness, std::optional maxFrameAverageBrightness, std::optional maxHdrHighlightBrightness) : ColorDescription(containerColorimetry, tf, sdrBrightness, minHdrBrightness, maxFrameAverageBrightness, maxHdrHighlightBrightness, std::nullopt, Colorimetry::fromName(NamedColorimetry::BT709)) { } -ColorDescription::ColorDescription(NamedColorimetry containerColorimetry, NamedTransferFunction tf, double sdrBrightness, double minHdrBrightness, double maxFrameAverageBrightness, double maxHdrHighlightBrightness) +ColorDescription::ColorDescription(NamedColorimetry containerColorimetry, NamedTransferFunction tf, double sdrBrightness, double minHdrBrightness, std::optional maxFrameAverageBrightness, std::optional maxHdrHighlightBrightness) : ColorDescription(Colorimetry::fromName(containerColorimetry), tf, sdrBrightness, minHdrBrightness, maxFrameAverageBrightness, maxHdrHighlightBrightness, std::nullopt, Colorimetry::fromName(NamedColorimetry::BT709)) { } -ColorDescription::ColorDescription(const Colorimetry &containerColorimetry, NamedTransferFunction tf, double sdrBrightness, double minHdrBrightness, double maxFrameAverageBrightness, double maxHdrHighlightBrightness, std::optional masteringColorimetry, const Colorimetry &sdrColorimetry) +ColorDescription::ColorDescription(const Colorimetry &containerColorimetry, NamedTransferFunction tf, double sdrBrightness, double minHdrBrightness, std::optional maxFrameAverageBrightness, std::optional maxHdrHighlightBrightness, std::optional masteringColorimetry, const Colorimetry &sdrColorimetry) : m_containerColorimetry(containerColorimetry) , m_masteringColorimetry(masteringColorimetry) , m_transferFunction(tf) @@ -222,7 +222,7 @@ ColorDescription::ColorDescription(const Colorimetry &containerColorimetry, Name { } -ColorDescription::ColorDescription(NamedColorimetry containerColorimetry, NamedTransferFunction tf, double sdrBrightness, double minHdrBrightness, double maxFrameAverageBrightness, double maxHdrHighlightBrightness, std::optional masteringColorimetry, const Colorimetry &sdrColorimetry) +ColorDescription::ColorDescription(NamedColorimetry containerColorimetry, NamedTransferFunction tf, double sdrBrightness, double minHdrBrightness, std::optional maxFrameAverageBrightness, std::optional maxHdrHighlightBrightness, std::optional masteringColorimetry, const Colorimetry &sdrColorimetry) : ColorDescription(Colorimetry::fromName(containerColorimetry), tf, sdrBrightness, minHdrBrightness, maxFrameAverageBrightness, maxHdrHighlightBrightness, masteringColorimetry, sdrColorimetry) { } @@ -257,12 +257,12 @@ double ColorDescription::minHdrBrightness() const return m_minHdrBrightness; } -double ColorDescription::maxFrameAverageBrightness() const +std::optional ColorDescription::maxFrameAverageBrightness() const { return m_maxFrameAverageBrightness; } -double ColorDescription::maxHdrHighlightBrightness() const +std::optional ColorDescription::maxHdrHighlightBrightness() const { return m_maxHdrHighlightBrightness; } diff --git a/src/core/colorspace.h b/src/core/colorspace.h index 5d7a6c0ee4..e228b313f3 100644 --- a/src/core/colorspace.h +++ b/src/core/colorspace.h @@ -111,10 +111,10 @@ public: * @param maxHdrHighlightBrightness the maximum brightness of HDR content, for a small part of the screen only * @param sdrColorimetry */ - explicit ColorDescription(const Colorimetry &containerColorimetry, NamedTransferFunction tf, double sdrBrightness, double minHdrBrightness, double maxFrameAverageBrightness, double maxHdrHighlightBrightness); - explicit ColorDescription(NamedColorimetry containerColorimetry, NamedTransferFunction tf, double sdrBrightness, double minHdrBrightness, double maxFrameAverageBrightness, double maxHdrHighlightBrightness); - explicit ColorDescription(const Colorimetry &containerColorimetry, NamedTransferFunction tf, double sdrBrightness, double minHdrBrightness, double maxFrameAverageBrightness, double maxHdrHighlightBrightness, std::optional masteringColorimetry, const Colorimetry &sdrColorimetry); - explicit ColorDescription(NamedColorimetry containerColorimetry, NamedTransferFunction tf, double sdrBrightness, double minHdrBrightness, double maxFrameAverageBrightness, double maxHdrHighlightBrightness, std::optional masteringColorimetry, const Colorimetry &sdrColorimetry); + explicit ColorDescription(const Colorimetry &containerColorimetry, NamedTransferFunction tf, double sdrBrightness, double minHdrBrightness, std::optional maxFrameAverageBrightness, std::optional maxHdrHighlightBrightness); + explicit ColorDescription(NamedColorimetry containerColorimetry, NamedTransferFunction tf, double sdrBrightness, double minHdrBrightness, std::optional maxFrameAverageBrightness, std::optional maxHdrHighlightBrightness); + explicit ColorDescription(const Colorimetry &containerColorimetry, NamedTransferFunction tf, double sdrBrightness, double minHdrBrightness, std::optional maxFrameAverageBrightness, std::optional maxHdrHighlightBrightness, std::optional masteringColorimetry, const Colorimetry &sdrColorimetry); + explicit ColorDescription(NamedColorimetry containerColorimetry, NamedTransferFunction tf, double sdrBrightness, double minHdrBrightness, std::optional maxFrameAverageBrightness, std::optional maxHdrHighlightBrightness, std::optional masteringColorimetry, const Colorimetry &sdrColorimetry); /** * The primaries and whitepoint that colors are encoded for. This is used to convert between different colorspaces. @@ -131,8 +131,8 @@ public: NamedTransferFunction transferFunction() const; double sdrBrightness() const; double minHdrBrightness() const; - double maxFrameAverageBrightness() const; - double maxHdrHighlightBrightness() const; + std::optional maxFrameAverageBrightness() const; + std::optional maxHdrHighlightBrightness() const; bool operator==(const ColorDescription &other) const = default; @@ -152,7 +152,7 @@ private: Colorimetry m_sdrColorimetry; double m_sdrBrightness; double m_minHdrBrightness; - double m_maxFrameAverageBrightness; - double m_maxHdrHighlightBrightness; + std::optional m_maxFrameAverageBrightness; + std::optional m_maxHdrHighlightBrightness; }; } diff --git a/src/opengl/glshader.cpp b/src/opengl/glshader.cpp index 878d07f7a4..d214798c1c 100644 --- a/src/opengl/glshader.cpp +++ b/src/opengl/glshader.cpp @@ -453,7 +453,7 @@ bool GLShader::setColorspaceUniforms(const ColorDescription &src, const ColorDes && setUniform(GLShader::IntUniform::SourceNamedTransferFunction, int(src.transferFunction())) && setUniform(GLShader::IntUniform::DestinationNamedTransferFunction, int(dst.transferFunction())) && setUniform(FloatUniform::SdrBrightness, dst.sdrBrightness()) - && setUniform(FloatUniform::MaxHdrBrightness, dst.maxHdrHighlightBrightness()); + && setUniform(FloatUniform::MaxHdrBrightness, dst.maxHdrHighlightBrightness().value_or(10'000)); } bool GLShader::setColorspaceUniformsFromSRGB(const ColorDescription &dst) diff --git a/src/wayland/frog_colormanagement_v1.cpp b/src/wayland/frog_colormanagement_v1.cpp index 4089ae7e96..a02427aae0 100644 --- a/src/wayland/frog_colormanagement_v1.cpp +++ b/src/wayland/frog_colormanagement_v1.cpp @@ -80,9 +80,9 @@ void FrogColorManagementSurfaceV1::setPreferredColorDescription(const ColorDescr encodePrimary(color.green().x()), encodePrimary(color.green().y()), encodePrimary(color.blue().x()), encodePrimary(color.blue().y()), encodePrimary(color.white().x()), encodePrimary(color.white().y()), - std::round(colorDescription.maxHdrHighlightBrightness()), + std::round(colorDescription.maxHdrHighlightBrightness().value_or(0)), std::round(colorDescription.minHdrBrightness() / 0.0001), - std::round(colorDescription.maxFrameAverageBrightness())); + std::round(colorDescription.maxFrameAverageBrightness().value_or(0))); } void FrogColorManagementSurfaceV1::frog_color_managed_surface_set_known_transfer_function(Resource *resource, uint32_t transfer_function) @@ -130,8 +130,12 @@ void FrogColorManagementSurfaceV1::frog_color_managed_surface_set_hdr_metadata(R uint32_t max_display_mastering_luminance, uint32_t min_display_mastering_luminance, uint32_t max_cll, uint32_t max_fall) { - m_maxPeakBrightness = max_cll; - m_maxFrameAverageBrightness = max_fall; + if (max_fall > 0) { + m_maxFrameAverageBrightness = max_fall; + } + if (max_cll > 0) { + m_maxPeakBrightness = max_cll; + } if (mastering_display_primary_red_x > 0 && mastering_display_primary_red_y > 0 && mastering_display_primary_green_x > 0 && mastering_display_primary_green_y > 0 && mastering_display_primary_blue_x > 0 && mastering_display_primary_blue_y > 0 && mastering_white_point_x > 0 && mastering_white_point_y > 0) { m_masteringColorimetry = Colorimetry{ QVector2D(mastering_display_primary_red_x / 10'000.0, mastering_display_primary_red_y / 10'000.0), @@ -156,7 +160,6 @@ void FrogColorManagementSurfaceV1::frog_color_managed_surface_destroy_resource(R void FrogColorManagementSurfaceV1::updateColorDescription() { if (m_surface) { - // TODO make brightness values optional in ColorDescription SurfaceInterfacePrivate *priv = SurfaceInterfacePrivate::get(m_surface); priv->pending->colorDescription = ColorDescription(m_containerColorimetry, m_transferFunction, 0, 0, m_maxFrameAverageBrightness, m_maxPeakBrightness, m_masteringColorimetry, Colorimetry::fromName(NamedColorimetry::BT709)); priv->pending->colorDescriptionIsSet = true; diff --git a/src/wayland/frog_colormanagement_v1.h b/src/wayland/frog_colormanagement_v1.h index 80ee1c77c6..c5fa1a87ba 100644 --- a/src/wayland/frog_colormanagement_v1.h +++ b/src/wayland/frog_colormanagement_v1.h @@ -56,8 +56,8 @@ private: NamedTransferFunction m_transferFunction = NamedTransferFunction::sRGB; NamedColorimetry m_containerColorimetry = NamedColorimetry::BT709; std::optional m_masteringColorimetry; - float m_maxFrameAverageBrightness = 0; - float m_maxPeakBrightness = 0; + std::optional m_maxFrameAverageBrightness; + std::optional m_maxPeakBrightness; }; }