From 3c51bc5024b849c9ed5defbea058133399cdda3e Mon Sep 17 00:00:00 2001 From: Alec Mouri Date: Fri, 4 Nov 2022 01:28:55 +0000 Subject: Improve tonemapping utilities * Add util for getting buffer dataspace from metadata, since some ANativeWindow queries are unreliable when Surface endpoints are passed between processes, e.g., camera * Let libshaders generate SkSL for SkColorFilters Bug: 238395777 Test: Switching HDR cameras don't color shift Change-Id: I7c3b917eeafcf8d028f8f52f38aa1389025bc607 --- libs/shaders/shaders.cpp | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) (limited to 'libs/shaders/shaders.cpp') diff --git a/libs/shaders/shaders.cpp b/libs/shaders/shaders.cpp index f80e93f6f8..a3c403e551 100644 --- a/libs/shaders/shaders.cpp +++ b/libs/shaders/shaders.cpp @@ -386,12 +386,23 @@ void generateOETF(ui::Dataspace dataspace, std::string& shader) { } } -void generateEffectiveOOTF(bool undoPremultipliedAlpha, std::string& shader) { - shader.append(R"( - uniform shader child; - half4 main(float2 xy) { - float4 c = float4(child.eval(xy)); - )"); +void generateEffectiveOOTF(bool undoPremultipliedAlpha, LinearEffect::SkSLType type, + std::string& shader) { + switch (type) { + case LinearEffect::SkSLType::ColorFilter: + shader.append(R"( + half4 main(half4 inputColor) { + float4 c = float4(inputColor); + )"); + break; + case LinearEffect::SkSLType::Shader: + shader.append(R"( + uniform shader child; + half4 main(float2 xy) { + float4 c = float4(child.eval(xy)); + )"); + break; + } if (undoPremultipliedAlpha) { shader.append(R"( c.rgb = c.rgb / (c.a + 0.0019); @@ -459,7 +470,7 @@ std::string buildLinearEffectSkSL(const LinearEffect& linearEffect) { generateXYZTransforms(shaderString); generateOOTF(linearEffect.inputDataspace, linearEffect.outputDataspace, shaderString); generateOETF(linearEffect.outputDataspace, shaderString); - generateEffectiveOOTF(linearEffect.undoPremultipliedAlpha, shaderString); + generateEffectiveOOTF(linearEffect.undoPremultipliedAlpha, linearEffect.type, shaderString); return shaderString; } -- cgit v1.2.3-59-g8ed1b From 3e5965f3184a828a1b7ee2b5bd5d5c2177807a35 Mon Sep 17 00:00:00 2001 From: Alec Mouri Date: Fri, 7 Apr 2023 18:00:58 +0000 Subject: Support screenshots of HDR content Previously screenshots always rendered to either an SDR or a wide gamut colorspace. For screenshotting HDR content, this is only appropriate when the resulting screenshot (a) never leaves the device and (b) the relevant code has workarounds for the display to appropriately handle its luminance range. HDR screenshots will now have two paths: * A standard path for rendering to HLG. HLG was chosen because the OOTF shape is less hand-wavey than PQ's, does not require metadata, and bands less at 8-bits of color. * A special path for "display-native" screenshots. This is for use-cases like screen rotation where there are stricter color accuracy requirements for round-tripping. Skia already encodes the resulting screenshot by supplying an HLG CICP alongside a backwards-compatible transfer function, so it's only sufficient to change how SurfaceFlinger renders. Bug: 242324609 Bug: 276812775 Test: screencap binary Test: rotation animation Test: swiping in Recents Change-Id: Ic9edb92391d3beb38d076fba8f15e3fdcc2b8f50 --- libs/gui/LayerState.cpp | 4 +- libs/gui/aidl/android/gui/ISurfaceComposer.aidl | 4 + libs/gui/include/gui/DisplayCaptureArgs.h | 11 +- libs/renderengine/skia/SkiaRenderEngine.cpp | 6 +- libs/shaders/shaders.cpp | 58 ++++++---- services/surfaceflinger/DisplayRenderArea.cpp | 11 +- services/surfaceflinger/DisplayRenderArea.h | 4 +- services/surfaceflinger/LayerRenderArea.cpp | 5 +- services/surfaceflinger/LayerRenderArea.h | 3 +- services/surfaceflinger/RegionSamplingThread.cpp | 4 +- services/surfaceflinger/RenderArea.h | 12 ++- services/surfaceflinger/ScreenCaptureOutput.cpp | 2 +- services/surfaceflinger/ScreenCaptureOutput.h | 2 + services/surfaceflinger/SurfaceFlinger.cpp | 118 ++++++++++++++------- .../fuzzer/surfaceflinger_layer_fuzzer.cpp | 3 +- .../tests/unittests/CompositionTest.cpp | 2 +- 16 files changed, 173 insertions(+), 76 deletions(-) (limited to 'libs/shaders/shaders.cpp') diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp index fee91a40c2..2322b70d1c 100644 --- a/libs/gui/LayerState.cpp +++ b/libs/gui/LayerState.cpp @@ -897,11 +897,11 @@ status_t CaptureArgs::writeToParcel(Parcel* output) const { SAFE_PARCEL(output->writeInt32, static_cast(dataspace)); SAFE_PARCEL(output->writeBool, allowProtected); SAFE_PARCEL(output->writeBool, grayscale); - SAFE_PARCEL(output->writeInt32, excludeHandles.size()); for (auto& excludeHandle : excludeHandles) { SAFE_PARCEL(output->writeStrongBinder, excludeHandle); } + SAFE_PARCEL(output->writeBool, hintForSeamlessTransition); return NO_ERROR; } @@ -918,7 +918,6 @@ status_t CaptureArgs::readFromParcel(const Parcel* input) { dataspace = static_cast(value); SAFE_PARCEL(input->readBool, &allowProtected); SAFE_PARCEL(input->readBool, &grayscale); - int32_t numExcludeHandles = 0; SAFE_PARCEL_READ_SIZE(input->readInt32, &numExcludeHandles, input->dataSize()); excludeHandles.reserve(numExcludeHandles); @@ -927,6 +926,7 @@ status_t CaptureArgs::readFromParcel(const Parcel* input) { SAFE_PARCEL(input->readStrongBinder, &binder); excludeHandles.emplace(binder); } + SAFE_PARCEL(input->readBool, &hintForSeamlessTransition); return NO_ERROR; } diff --git a/libs/gui/aidl/android/gui/ISurfaceComposer.aidl b/libs/gui/aidl/android/gui/ISurfaceComposer.aidl index aa58e2e580..ec3266ca83 100644 --- a/libs/gui/aidl/android/gui/ISurfaceComposer.aidl +++ b/libs/gui/aidl/android/gui/ISurfaceComposer.aidl @@ -230,6 +230,10 @@ interface ISurfaceComposer { */ void captureDisplay(in DisplayCaptureArgs args, IScreenCaptureListener listener); + /** + * Capture the specified screen. This requires the READ_FRAME_BUFFER + * permission. + */ void captureDisplayById(long displayId, IScreenCaptureListener listener); /** diff --git a/libs/gui/include/gui/DisplayCaptureArgs.h b/libs/gui/include/gui/DisplayCaptureArgs.h index 5c794aea37..2676e0a338 100644 --- a/libs/gui/include/gui/DisplayCaptureArgs.h +++ b/libs/gui/include/gui/DisplayCaptureArgs.h @@ -41,7 +41,7 @@ struct CaptureArgs : public Parcelable { bool captureSecureLayers{false}; int32_t uid{UNSET_UID}; // Force capture to be in a color space. If the value is ui::Dataspace::UNKNOWN, the captured - // result will be in the display's colorspace. + // result will be in a colorspace appropriate for capturing the display contents // The display may use non-RGB dataspace (ex. displayP3) that could cause pixel data could be // different from SRGB (byte per color), and failed when checking colors in tests. // NOTE: In normal cases, we want the screen to be captured in display's colorspace. @@ -59,6 +59,15 @@ struct CaptureArgs : public Parcelable { std::unordered_set, SpHash> excludeHandles; + // Hint that the caller will use the screenshot animation as part of a transition animation. + // The canonical example would be screen rotation - in such a case any color shift in the + // screenshot is a detractor so composition in the display's colorspace is required. + // Otherwise, the system may choose a colorspace that is more appropriate for use-cases + // such as file encoding or for blending HDR content into an ap's UI, where the display's + // exact colorspace is not an appropriate intermediate result. + // Note that if the caller is requesting a specific dataspace, this hint does nothing. + bool hintForSeamlessTransition = false; + virtual status_t writeToParcel(Parcel* output) const; virtual status_t readFromParcel(const Parcel* input); }; diff --git a/libs/renderengine/skia/SkiaRenderEngine.cpp b/libs/renderengine/skia/SkiaRenderEngine.cpp index 8256dd8e69..2a51493cea 100644 --- a/libs/renderengine/skia/SkiaRenderEngine.cpp +++ b/libs/renderengine/skia/SkiaRenderEngine.cpp @@ -517,16 +517,18 @@ sk_sp SkiaRenderEngine::createRuntimeEffectShader( } else { runtimeEffect = effectIter->second; } + mat4 colorTransform = parameters.layer.colorTransform; colorTransform *= mat4::scale(vec4(parameters.layerDimmingRatio, parameters.layerDimmingRatio, parameters.layerDimmingRatio, 1.f)); + const auto targetBuffer = parameters.layer.source.buffer.buffer; const auto graphicBuffer = targetBuffer ? targetBuffer->getBuffer() : nullptr; const auto hardwareBuffer = graphicBuffer ? graphicBuffer->toAHardwareBuffer() : nullptr; - return createLinearEffectShader(parameters.shader, effect, runtimeEffect, colorTransform, - parameters.display.maxLuminance, + return createLinearEffectShader(parameters.shader, effect, runtimeEffect, + std::move(colorTransform), parameters.display.maxLuminance, parameters.display.currentLuminanceNits, parameters.layer.source.buffer.maxLuminanceNits, hardwareBuffer, parameters.display.renderIntent); diff --git a/libs/shaders/shaders.cpp b/libs/shaders/shaders.cpp index a3c403e551..19518eaecc 100644 --- a/libs/shaders/shaders.cpp +++ b/libs/shaders/shaders.cpp @@ -191,31 +191,35 @@ void generateLuminanceScalesForOOTF(ui::Dataspace inputDataspace, ui::Dataspace )"); break; default: + // Input is SDR so map to its white point luminance switch (outputDataspace & HAL_DATASPACE_TRANSFER_MASK) { - case HAL_DATASPACE_TRANSFER_ST2084: + // Max HLG output is nominally 1000 nits, but BT. 2100-2 allows + // for gamma correcting the HLG OOTF for displays with a different + // dynamic range. Scale to 1000 nits to apply an inverse OOTF against + // a reference display correctly. + // TODO: Use knowledge of the dimming ratio here to prevent + // unintended gamma shaft. case HAL_DATASPACE_TRANSFER_HLG: - // SDR -> HDR tonemap shader.append(R"( float3 ScaleLuminance(float3 xyz) { - return xyz * in_libtonemap_inputMaxLuminance; + return xyz * 1000.0; } )"); break; default: - // Input and output are both SDR, so no tone-mapping is expected so - // no-op the luminance normalization. shader.append(R"( - float3 ScaleLuminance(float3 xyz) { - return xyz * in_libtonemap_displayMaxLuminance; - } - )"); + float3 ScaleLuminance(float3 xyz) { + return xyz * in_libtonemap_displayMaxLuminance; + } + )"); break; } } } // Normalizes from absolute light back to relative light (maps from [0, maxNits] back to [0, 1]) -static void generateLuminanceNormalizationForOOTF(ui::Dataspace outputDataspace, +static void generateLuminanceNormalizationForOOTF(ui::Dataspace inputDataspace, + ui::Dataspace outputDataspace, std::string& shader) { switch (outputDataspace & HAL_DATASPACE_TRANSFER_MASK) { case HAL_DATASPACE_TRANSFER_ST2084: @@ -226,11 +230,28 @@ static void generateLuminanceNormalizationForOOTF(ui::Dataspace outputDataspace, )"); break; case HAL_DATASPACE_TRANSFER_HLG: - shader.append(R"( - float3 NormalizeLuminance(float3 xyz) { - return xyz / 1000.0; - } - )"); + switch (inputDataspace & HAL_DATASPACE_TRANSFER_MASK) { + case HAL_DATASPACE_TRANSFER_HLG: + shader.append(R"( + float3 NormalizeLuminance(float3 xyz) { + return xyz / 1000.0; + } + )"); + break; + default: + // Transcoding to HLG requires applying the inverse OOTF + // with the expectation that the OOTF is then applied during + // tonemapping downstream. + shader.append(R"( + float3 NormalizeLuminance(float3 xyz) { + // BT. 2100-2 operates on normalized luminances, + // so renormalize to the input + float ootfGain = pow(xyz.y / 1000.0, -0.2 / 1.2) / 1000.0; + return xyz * ootfGain; + } + )"); + break; + } break; default: shader.append(R"( @@ -250,7 +271,7 @@ void generateOOTF(ui::Dataspace inputDataspace, ui::Dataspace outputDataspace, .c_str()); generateLuminanceScalesForOOTF(inputDataspace, outputDataspace, shader); - generateLuminanceNormalizationForOOTF(outputDataspace, shader); + generateLuminanceNormalizationForOOTF(inputDataspace, outputDataspace, shader); shader.append(R"( float3 OOTF(float3 linearRGB, float3 xyz) { @@ -501,9 +522,8 @@ std::vector buildLinearEffectUniforms( tonemap::Metadata metadata{.displayMaxLuminance = maxDisplayLuminance, // If the input luminance is unknown, use display luminance (aka, - // no-op any luminance changes) - // This will be the case for eg screenshots in addition to - // uncalibrated displays + // no-op any luminance changes). + // This is expected to only be meaningful for PQ content .contentMaxLuminance = maxLuminance > 0 ? maxLuminance : maxDisplayLuminance, .currentDisplayLuminance = currentDisplayLuminanceNits > 0 diff --git a/services/surfaceflinger/DisplayRenderArea.cpp b/services/surfaceflinger/DisplayRenderArea.cpp index 8f39e26e0f..e55cd3ea42 100644 --- a/services/surfaceflinger/DisplayRenderArea.cpp +++ b/services/surfaceflinger/DisplayRenderArea.cpp @@ -35,21 +35,24 @@ std::unique_ptr DisplayRenderArea::create(wp di const Rect& sourceCrop, ui::Size reqSize, ui::Dataspace reqDataSpace, bool useIdentityTransform, + bool hintForSeamlessTransition, bool allowSecureLayers) { if (auto display = displayWeak.promote()) { // Using new to access a private constructor. return std::unique_ptr( new DisplayRenderArea(std::move(display), sourceCrop, reqSize, reqDataSpace, - useIdentityTransform, allowSecureLayers)); + useIdentityTransform, hintForSeamlessTransition, + allowSecureLayers)); } return nullptr; } DisplayRenderArea::DisplayRenderArea(sp display, const Rect& sourceCrop, ui::Size reqSize, ui::Dataspace reqDataSpace, - bool useIdentityTransform, bool allowSecureLayers) - : RenderArea(reqSize, CaptureFill::OPAQUE, reqDataSpace, allowSecureLayers, - applyDeviceOrientation(useIdentityTransform, *display)), + bool useIdentityTransform, bool hintForSeamlessTransition, + bool allowSecureLayers) + : RenderArea(reqSize, CaptureFill::OPAQUE, reqDataSpace, hintForSeamlessTransition, + allowSecureLayers, applyDeviceOrientation(useIdentityTransform, *display)), mDisplay(std::move(display)), mSourceCrop(sourceCrop) {} diff --git a/services/surfaceflinger/DisplayRenderArea.h b/services/surfaceflinger/DisplayRenderArea.h index ce5410a90d..9a4981c881 100644 --- a/services/surfaceflinger/DisplayRenderArea.h +++ b/services/surfaceflinger/DisplayRenderArea.h @@ -30,6 +30,7 @@ public: static std::unique_ptr create(wp, const Rect& sourceCrop, ui::Size reqSize, ui::Dataspace, bool useIdentityTransform, + bool hintForSeamlessTransition, bool allowSecureLayers = true); const ui::Transform& getTransform() const override; @@ -39,7 +40,8 @@ public: private: DisplayRenderArea(sp, const Rect& sourceCrop, ui::Size reqSize, - ui::Dataspace, bool useIdentityTransform, bool allowSecureLayers = true); + ui::Dataspace, bool useIdentityTransform, bool hintForSeamlessTransition, + bool allowSecureLayers = true); const sp mDisplay; const Rect mSourceCrop; diff --git a/services/surfaceflinger/LayerRenderArea.cpp b/services/surfaceflinger/LayerRenderArea.cpp index 1b8ff28a76..f6b5afc6a8 100644 --- a/services/surfaceflinger/LayerRenderArea.cpp +++ b/services/surfaceflinger/LayerRenderArea.cpp @@ -40,8 +40,9 @@ void reparentForDrawing(const sp& oldParent, const sp& newParent, LayerRenderArea::LayerRenderArea(SurfaceFlinger& flinger, sp layer, const Rect& crop, ui::Size reqSize, ui::Dataspace reqDataSpace, bool childrenOnly, bool allowSecureLayers, const ui::Transform& layerTransform, - const Rect& layerBufferSize) - : RenderArea(reqSize, CaptureFill::CLEAR, reqDataSpace, allowSecureLayers), + const Rect& layerBufferSize, bool hintForSeamlessTransition) + : RenderArea(reqSize, CaptureFill::CLEAR, reqDataSpace, hintForSeamlessTransition, + allowSecureLayers), mLayer(std::move(layer)), mLayerTransform(layerTransform), mLayerBufferSize(layerBufferSize), diff --git a/services/surfaceflinger/LayerRenderArea.h b/services/surfaceflinger/LayerRenderArea.h index 9bb13b3d6a..aa609eea38 100644 --- a/services/surfaceflinger/LayerRenderArea.h +++ b/services/surfaceflinger/LayerRenderArea.h @@ -34,7 +34,8 @@ class LayerRenderArea : public RenderArea { public: LayerRenderArea(SurfaceFlinger& flinger, sp layer, const Rect& crop, ui::Size reqSize, ui::Dataspace reqDataSpace, bool childrenOnly, bool allowSecureLayers, - const ui::Transform& layerTransform, const Rect& layerBufferSize); + const ui::Transform& layerTransform, const Rect& layerBufferSize, + bool hintForSeamlessTransition); const ui::Transform& getTransform() const override; bool isSecure() const override; diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp index 531d277ffc..8f658d5a09 100644 --- a/services/surfaceflinger/RegionSamplingThread.cpp +++ b/services/surfaceflinger/RegionSamplingThread.cpp @@ -277,10 +277,12 @@ void RegionSamplingThread::captureSample() { const Rect sampledBounds = sampleRegion.bounds(); constexpr bool kUseIdentityTransform = false; + constexpr bool kHintForSeamlessTransition = false; SurfaceFlinger::RenderAreaFuture renderAreaFuture = ftl::defer([=] { return DisplayRenderArea::create(displayWeak, sampledBounds, sampledBounds.getSize(), - ui::Dataspace::V0_SRGB, kUseIdentityTransform); + ui::Dataspace::V0_SRGB, kUseIdentityTransform, + kHintForSeamlessTransition); }); std::unordered_set, SpHash> listeners; diff --git a/services/surfaceflinger/RenderArea.h b/services/surfaceflinger/RenderArea.h index 910fce043c..71b85bd3b2 100644 --- a/services/surfaceflinger/RenderArea.h +++ b/services/surfaceflinger/RenderArea.h @@ -25,12 +25,14 @@ public: static float getCaptureFillValue(CaptureFill captureFill); RenderArea(ui::Size reqSize, CaptureFill captureFill, ui::Dataspace reqDataSpace, - bool allowSecureLayers = false, RotationFlags rotation = ui::Transform::ROT_0) + bool hintForSeamlessTransition, bool allowSecureLayers = false, + RotationFlags rotation = ui::Transform::ROT_0) : mAllowSecureLayers(allowSecureLayers), mReqSize(reqSize), mReqDataSpace(reqDataSpace), mCaptureFill(captureFill), - mRotationFlags(rotation) {} + mRotationFlags(rotation), + mHintForSeamlessTransition(hintForSeamlessTransition) {} static std::function>>()> fromTraverseLayersLambda( std::function traverseLayers) { @@ -90,6 +92,10 @@ public: // capture operation. virtual sp getParentLayer() const { return nullptr; } + // Returns whether the render result may be used for system animations that + // must preserve the exact colors of the display. + bool getHintForSeamlessTransition() const { return mHintForSeamlessTransition; } + protected: const bool mAllowSecureLayers; @@ -98,7 +104,7 @@ private: const ui::Dataspace mReqDataSpace; const CaptureFill mCaptureFill; const RotationFlags mRotationFlags; - const Rect mLayerStackSpaceRect; + const bool mHintForSeamlessTransition; }; } // namespace android diff --git a/services/surfaceflinger/ScreenCaptureOutput.cpp b/services/surfaceflinger/ScreenCaptureOutput.cpp index a1d5cd7379..b70b53d05a 100644 --- a/services/surfaceflinger/ScreenCaptureOutput.cpp +++ b/services/surfaceflinger/ScreenCaptureOutput.cpp @@ -36,6 +36,7 @@ std::shared_ptr createScreenCaptureOutput(ScreenCaptureOutp output->setLayerFilter({args.layerStack}); output->setRenderSurface(std::make_unique(std::move(args.buffer))); output->setDisplayBrightness(args.sdrWhitePointNits, args.displayBrightnessNits); + output->editState().clientTargetBrightness = args.targetBrightness; output->setDisplayColorProfile(std::make_unique( compositionengine::DisplayColorProfileCreationArgsBuilder() @@ -75,7 +76,6 @@ renderengine::DisplaySettings ScreenCaptureOutput::generateClientCompositionDisp auto clientCompositionDisplay = compositionengine::impl::Output::generateClientCompositionDisplaySettings(); clientCompositionDisplay.clip = mRenderArea.getSourceCrop(); - clientCompositionDisplay.targetLuminanceNits = -1; return clientCompositionDisplay; } diff --git a/services/surfaceflinger/ScreenCaptureOutput.h b/services/surfaceflinger/ScreenCaptureOutput.h index 4e5a0ccfd2..3c307b0733 100644 --- a/services/surfaceflinger/ScreenCaptureOutput.h +++ b/services/surfaceflinger/ScreenCaptureOutput.h @@ -33,6 +33,8 @@ struct ScreenCaptureOutputArgs { std::shared_ptr buffer; float sdrWhitePointNits; float displayBrightnessNits; + // Counterintuitively, when targetBrightness > 1.0 then dim the scene. + float targetBrightness; bool regionSampling; }; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index c88bff5ed8..d406afff1f 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -6915,6 +6915,32 @@ status_t SurfaceFlinger::setSchedAttr(bool enabled) { return NO_ERROR; } +namespace { + +ui::Dataspace pickBestDataspace(ui::Dataspace requestedDataspace, const DisplayDevice* display, + bool capturingHdrLayers, bool hintForSeamlessTransition) { + if (requestedDataspace != ui::Dataspace::UNKNOWN || display == nullptr) { + return requestedDataspace; + } + + const auto& state = display->getCompositionDisplay()->getState(); + + const auto dataspaceForColorMode = ui::pickDataspaceFor(state.colorMode); + + if (capturingHdrLayers && !hintForSeamlessTransition) { + // For now since we only support 8-bit screenshots, just use HLG and + // assume that 1.0 >= display max luminance. This isn't quite as future + // proof as PQ is, but is good enough. + // Consider using PQ once we support 16-bit screenshots and we're able + // to consistently supply metadata to image encoders. + return ui::Dataspace::BT2020_HLG; + } + + return dataspaceForColorMode; +} + +} // namespace + status_t SurfaceFlinger::captureDisplay(const DisplayCaptureArgs& args, const sp& captureListener) { ATRACE_CALL(); @@ -6930,7 +6956,6 @@ status_t SurfaceFlinger::captureDisplay(const DisplayCaptureArgs& args, ui::LayerStack layerStack; ui::Size reqSize(args.width, args.height); std::unordered_set excludeLayerIds; - ui::Dataspace dataspace; { Mutex::Autolock lock(mStateLock); sp display = getDisplayDeviceLocked(args.displayToken); @@ -6952,17 +6977,12 @@ status_t SurfaceFlinger::captureDisplay(const DisplayCaptureArgs& args, return NAME_NOT_FOUND; } } - - // Allow the caller to specify a dataspace regardless of the display's color mode, e.g. if - // it wants sRGB regardless of the display's wide color mode. - dataspace = args.dataspace == ui::Dataspace::UNKNOWN - ? ui::pickDataspaceFor(display->getCompositionDisplay()->getState().colorMode) - : args.dataspace; } RenderAreaFuture renderAreaFuture = ftl::defer([=] { - return DisplayRenderArea::create(displayWeak, args.sourceCrop, reqSize, dataspace, - args.useIdentityTransform, args.captureSecureLayers); + return DisplayRenderArea::create(displayWeak, args.sourceCrop, reqSize, + ui::Dataspace::UNKNOWN, args.useIdentityTransform, + args.hintForSeamlessTransition, args.captureSecureLayers); }); GetLayerSnapshotsFunction getLayerSnapshots; @@ -6988,7 +7008,6 @@ status_t SurfaceFlinger::captureDisplay(DisplayId displayId, ui::LayerStack layerStack; wp displayWeak; ui::Size size; - ui::Dataspace dataspace; { Mutex::Autolock lock(mStateLock); @@ -7000,12 +7019,12 @@ status_t SurfaceFlinger::captureDisplay(DisplayId displayId, displayWeak = display; layerStack = display->getLayerStack(); size = display->getLayerStackSpaceRect().getSize(); - dataspace = ui::pickDataspaceFor(display->getCompositionDisplay()->getState().colorMode); } RenderAreaFuture renderAreaFuture = ftl::defer([=] { - return DisplayRenderArea::create(displayWeak, Rect(), size, dataspace, + return DisplayRenderArea::create(displayWeak, Rect(), size, ui::Dataspace::UNKNOWN, false /* useIdentityTransform */, + false /* hintForSeamlessTransition */, false /* captureSecureLayers */); }); @@ -7047,7 +7066,7 @@ status_t SurfaceFlinger::captureLayers(const LayerCaptureArgs& args, sp parent; Rect crop(args.sourceCrop); std::unordered_set excludeLayerIds; - ui::Dataspace dataspace; + ui::Dataspace dataspace = args.dataspace; // Call this before holding mStateLock to avoid any deadlocking. bool canCaptureBlackoutContent = hasCaptureBlackoutContentPermission(); @@ -7094,12 +7113,6 @@ status_t SurfaceFlinger::captureLayers(const LayerCaptureArgs& args, return NAME_NOT_FOUND; } } - - // The dataspace is depended on the color mode of display, that could use non-native mode - // (ex. displayP3) to enhance the content, but some cases are checking native RGB in bytes, - // and failed if display is not in native mode. This provide a way to force using native - // colors when capture. - dataspace = args.dataspace; } // mStateLock // really small crop or frameScale @@ -7128,7 +7141,8 @@ status_t SurfaceFlinger::captureLayers(const LayerCaptureArgs& args, return std::make_unique(*this, parent, crop, reqSize, dataspace, childrenOnly, args.captureSecureLayers, - layerTransform, layerBufferSize); + layerTransform, layerBufferSize, + args.hintForSeamlessTransition); }); GetLayerSnapshotsFunction getLayerSnapshots; if (mLayerLifecycleManagerEnabled) { @@ -7314,29 +7328,48 @@ ftl::SharedFuture SurfaceFlinger::renderScreenImpl( return ftl::yield(base::unexpected(PERMISSION_DENIED)).share(); } - captureResults.buffer = buffer->getBuffer(); - auto dataspace = renderArea->getReqDataSpace(); + auto capturedBuffer = buffer; + + auto requestedDataspace = renderArea->getReqDataSpace(); auto parent = renderArea->getParentLayer(); auto renderIntent = RenderIntent::TONE_MAP_COLORIMETRIC; auto sdrWhitePointNits = DisplayDevice::sDefaultMaxLumiance; auto displayBrightnessNits = DisplayDevice::sDefaultMaxLumiance; - if (dataspace == ui::Dataspace::UNKNOWN && parent) { + captureResults.capturedDataspace = requestedDataspace; + + { Mutex::Autolock lock(mStateLock); - auto display = findDisplay([layerStack = parent->getLayerStack()](const auto& display) { - return display.getLayerStack() == layerStack; - }); - if (!display) { - // If the layer is not on a display, use the dataspace for the default display. - display = getDefaultDisplayDeviceLocked(); + const DisplayDevice* display = nullptr; + if (parent) { + display = findDisplay([layerStack = parent->getLayerStack()](const auto& display) { + return display.getLayerStack() == layerStack; + }).get(); + } + + if (display == nullptr) { + display = renderArea->getDisplayDevice().get(); + } + + if (display == nullptr) { + display = getDefaultDisplayDeviceLocked().get(); } - dataspace = ui::pickDataspaceFor(display->getCompositionDisplay()->getState().colorMode); - renderIntent = display->getCompositionDisplay()->getState().renderIntent; - sdrWhitePointNits = display->getCompositionDisplay()->getState().sdrWhitePointNits; - displayBrightnessNits = display->getCompositionDisplay()->getState().displayBrightnessNits; + if (display != nullptr) { + const auto& state = display->getCompositionDisplay()->getState(); + captureResults.capturedDataspace = + pickBestDataspace(requestedDataspace, display, captureResults.capturedHdrLayers, + renderArea->getHintForSeamlessTransition()); + sdrWhitePointNits = state.sdrWhitePointNits; + displayBrightnessNits = state.displayBrightnessNits; + + if (requestedDataspace == ui::Dataspace::UNKNOWN) { + renderIntent = state.renderIntent; + } + } } - captureResults.capturedDataspace = dataspace; + + captureResults.buffer = capturedBuffer->getBuffer(); ui::LayerStack layerStack{ui::DEFAULT_LAYER_STACK}; if (!layers.empty()) { @@ -7353,9 +7386,9 @@ ftl::SharedFuture SurfaceFlinger::renderScreenImpl( return layerFEs; }; - auto present = [this, buffer = std::move(buffer), dataspace, sdrWhitePointNits, - displayBrightnessNits, grayscale, layerFEs = copyLayerFEs(), layerStack, - regionSampling, renderArea = std::move(renderArea), + auto present = [this, buffer = capturedBuffer, dataspace = captureResults.capturedDataspace, + sdrWhitePointNits, displayBrightnessNits, grayscale, layerFEs = copyLayerFEs(), + layerStack, regionSampling, renderArea = std::move(renderArea), renderIntent]() -> FenceResult { std::unique_ptr compositionEngine = mFactory.createCompositionEngine(); @@ -7364,6 +7397,16 @@ ftl::SharedFuture SurfaceFlinger::renderScreenImpl( compositionengine::Output::ColorProfile colorProfile{.dataspace = dataspace, .renderIntent = renderIntent}; + float targetBrightness = 1.0f; + if (dataspace == ui::Dataspace::BT2020_HLG) { + const float maxBrightnessNits = displayBrightnessNits / sdrWhitePointNits * 203; + // With a low dimming ratio, don't fit the entire curve. Otherwise mixed content + // will appear way too bright. + if (maxBrightnessNits < 1000.f) { + targetBrightness = 1000.f / maxBrightnessNits; + } + } + std::shared_ptr output = createScreenCaptureOutput( ScreenCaptureOutputArgs{.compositionEngine = *compositionEngine, .colorProfile = colorProfile, @@ -7372,6 +7415,7 @@ ftl::SharedFuture SurfaceFlinger::renderScreenImpl( .buffer = std::move(buffer), .sdrWhitePointNits = sdrWhitePointNits, .displayBrightnessNits = displayBrightnessNits, + .targetBrightness = targetBrightness, .regionSampling = regionSampling}); const float colorSaturation = grayscale ? 0 : 1; diff --git a/services/surfaceflinger/fuzzer/surfaceflinger_layer_fuzzer.cpp b/services/surfaceflinger/fuzzer/surfaceflinger_layer_fuzzer.cpp index c3dcb85686..921cae4e41 100644 --- a/services/surfaceflinger/fuzzer/surfaceflinger_layer_fuzzer.cpp +++ b/services/surfaceflinger/fuzzer/surfaceflinger_layer_fuzzer.cpp @@ -177,7 +177,8 @@ void LayerFuzzer::invokeBufferStateLayer() { {mFdp.ConsumeIntegral(), mFdp.ConsumeIntegral()} /*reqSize*/, mFdp.PickValueInArray(kDataspaces), mFdp.ConsumeBool(), - mFdp.ConsumeBool(), getFuzzedTransform(), getFuzzedRect()); + mFdp.ConsumeBool(), getFuzzedTransform(), getFuzzedRect(), + mFdp.ConsumeBool()); layerArea.render([]() {} /*drawLayers*/); if (!ownsHandle) { diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp index 156007b862..6ca21bd458 100644 --- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp +++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp @@ -200,7 +200,7 @@ void CompositionTest::captureScreenComposition() { constexpr bool regionSampling = false; auto renderArea = DisplayRenderArea::create(mDisplay, sourceCrop, sourceCrop.getSize(), - ui::Dataspace::V0_SRGB, ui::Transform::ROT_0); + ui::Dataspace::V0_SRGB, true, true); auto traverseLayers = [this](const LayerVector::Visitor& visitor) { return mFlinger.traverseLayersInLayerStack(mDisplay->getLayerStack(), -- cgit v1.2.3-59-g8ed1b From 8a186104276cdf57d552c54dbad488e14f0d3d16 Mon Sep 17 00:00:00 2001 From: Alec Mouri Date: Tue, 25 Apr 2023 00:34:30 +0000 Subject: Align HLG and PQ color management to 203 nits == SDR max by default The public HLG and PQ definitions map 203 nits to SDR white by scaling the respective transfer functions. Bug: 278121691 Bug: 278122024 Test: HwAccelerationTest test gradients Test: HDR test videos match DPU and GPU composition Test: SilkFX test HLG and PQ images Change-Id: Id830e2ac72f5bcf8566556053fcf3af6b945581b --- libs/renderengine/skia/ColorSpaces.cpp | 9 +- libs/renderengine/skia/SkiaRenderEngine.cpp | 10 +- libs/shaders/include/shaders/shaders.h | 27 +- libs/shaders/shaders.cpp | 481 ++++++++-------------------- libs/shaders/tests/shaders_test.cpp | 42 ++- 5 files changed, 167 insertions(+), 402 deletions(-) (limited to 'libs/shaders/shaders.cpp') diff --git a/libs/renderengine/skia/ColorSpaces.cpp b/libs/renderengine/skia/ColorSpaces.cpp index 37ff5dfede..92b01e07e6 100644 --- a/libs/renderengine/skia/ColorSpaces.cpp +++ b/libs/renderengine/skia/ColorSpaces.cpp @@ -21,6 +21,8 @@ namespace renderengine { namespace skia { // please keep in sync with hwui/utils/Color.cpp +// TODO: Scale by the dimming ratio here instead of in a generic 3x3 transform +// Otherwise there may be luminance shift for e.g., HLG. sk_sp toSkColorSpace(ui::Dataspace dataspace) { skcms_Matrix3x3 gamut; switch (dataspace & HAL_DATASPACE_STANDARD_MASK) { @@ -61,13 +63,14 @@ sk_sp toSkColorSpace(ui::Dataspace dataspace) { case HAL_DATASPACE_TRANSFER_GAMMA2_8: return SkColorSpace::MakeRGB({2.8f, 1.0f, 0.0f, 0.0f, 0.0f, 0.0f, 0.0f}, gamut); case HAL_DATASPACE_TRANSFER_ST2084: - return SkColorSpace::MakeRGB(SkNamedTransferFn::kPQ, gamut); + return SkColorSpace::MakeRGB({-2.f, -1.55522297832f, 1.86045365631f, 32 / 2523.0f, + 2413 / 128.0f, -2392 / 128.0f, 8192 / 1305.0f}, + gamut); case HAL_DATASPACE_TRANSFER_SMPTE_170M: return SkColorSpace::MakeRGB(SkNamedTransferFn::kRec2020, gamut); case HAL_DATASPACE_TRANSFER_HLG: - // return HLG transfer but scale by 1/12 skcms_TransferFunction hlgFn; - if (skcms_TransferFunction_makeScaledHLGish(&hlgFn, 1.f / 12.f, 2.f, 2.f, + if (skcms_TransferFunction_makeScaledHLGish(&hlgFn, 0.314509843, 2.f, 2.f, 1.f / 0.17883277f, 0.28466892f, 0.55991073f)) { return SkColorSpace::MakeRGB(hlgFn, gamut); diff --git a/libs/renderengine/skia/SkiaRenderEngine.cpp b/libs/renderengine/skia/SkiaRenderEngine.cpp index 2a51493cea..cfea85f98b 100644 --- a/libs/renderengine/skia/SkiaRenderEngine.cpp +++ b/libs/renderengine/skia/SkiaRenderEngine.cpp @@ -913,12 +913,10 @@ void SkiaRenderEngine::drawLayersInternal( continue; } - // If we need to map to linear space or color management is disabled, then mark the source - // image with the same colorspace as the destination surface so that Skia's color - // management is a no-op. - const ui::Dataspace layerDataspace = (!mUseColorManagement || requiresLinearEffect) - ? display.outputDataspace - : layer.sourceDataspace; + // If color management is disabled, then mark the source image with the same colorspace as + // the destination surface so that Skia's color management is a no-op. + const ui::Dataspace layerDataspace = + !mUseColorManagement ? display.outputDataspace : layer.sourceDataspace; SkPaint paint; if (layer.source.buffer.buffer) { diff --git a/libs/shaders/include/shaders/shaders.h b/libs/shaders/include/shaders/shaders.h index 42b0cc131c..5a4aaab851 100644 --- a/libs/shaders/include/shaders/shaders.h +++ b/libs/shaders/include/shaders/shaders.h @@ -51,23 +51,20 @@ struct LinearEffect { // Input dataspace of the source colors. const ui::Dataspace inputDataspace = ui::Dataspace::SRGB; - // Working dataspace for the output surface, for conversion from linear space. + // Working dataspace for the output surface. const ui::Dataspace outputDataspace = ui::Dataspace::SRGB; // Sets whether alpha premultiplication must be undone. // This is required if the source colors use premultiplied alpha and is not opaque. const bool undoPremultipliedAlpha = false; - // "Fake" dataspace of the source colors. This is used for applying an EOTF to compute linear - // RGB. This is used when Skia is expected to color manage the input image based on the - // dataspace of the provided source image and destination surface. SkRuntimeEffects use the - // destination color space as the working color space. RenderEngine deliberately sets the color - // space for input images and destination surfaces to be the same whenever LinearEffects are - // expected to be used so that color-management is controlled by RenderEngine, but other users - // of a LinearEffect may not be able to control the color space of the images and surfaces. So - // fakeInputDataspace is used to essentially masquerade the input dataspace to be the output - // dataspace for correct conversion to linear colors. - ui::Dataspace fakeInputDataspace = ui::Dataspace::UNKNOWN; + // "Fake" dataspace of the destination colors. This is used for applying an OETF to compute + // non-linear RGB. This is used when Skia is expected to color manage the input image based on + // the dataspace of the provided source image and destination surface. Some use-cases in + // RenderEngine expect to apply a different OETF than what is expected by Skia. As in, + // RenderEngine will color manage to a custom destination and "cast" the result to Skia's + // working space. + ui::Dataspace fakeOutputDataspace = ui::Dataspace::UNKNOWN; enum SkSLType { Shader, ColorFilter }; SkSLType type = Shader; @@ -76,7 +73,7 @@ struct LinearEffect { static inline bool operator==(const LinearEffect& lhs, const LinearEffect& rhs) { return lhs.inputDataspace == rhs.inputDataspace && lhs.outputDataspace == rhs.outputDataspace && lhs.undoPremultipliedAlpha == rhs.undoPremultipliedAlpha && - lhs.fakeInputDataspace == rhs.fakeInputDataspace; + lhs.fakeOutputDataspace == rhs.fakeOutputDataspace; } struct LinearEffectHasher { @@ -89,7 +86,7 @@ struct LinearEffectHasher { size_t result = std::hash{}(le.inputDataspace); result = HashCombine(result, std::hash{}(le.outputDataspace)); result = HashCombine(result, std::hash{}(le.undoPremultipliedAlpha)); - return HashCombine(result, std::hash{}(le.fakeInputDataspace)); + return HashCombine(result, std::hash{}(le.fakeOutputDataspace)); } }; @@ -99,10 +96,6 @@ struct LinearEffectHasher { // 2. Apply color transform matrices in linear space std::string buildLinearEffectSkSL(const LinearEffect& linearEffect); -// Generates a shader string that applies color transforms in linear space. -// This is intended to be plugged into an SkColorFilter -std::string buildLinearEffectSkSLForColorFilter(const LinearEffect& linearEffect); - // Generates a list of uniforms to set on the LinearEffect shader above. std::vector buildLinearEffectUniforms( const LinearEffect& linearEffect, const mat4& colorTransform, float maxDisplayLuminance, diff --git a/libs/shaders/shaders.cpp b/libs/shaders/shaders.cpp index 19518eaecc..b8f2be111e 100644 --- a/libs/shaders/shaders.cpp +++ b/libs/shaders/shaders.cpp @@ -33,187 +33,46 @@ aidl::android::hardware::graphics::common::Dataspace toAidlDataspace(ui::Dataspa return static_cast(dataspace); } -void generateEOTF(ui::Dataspace dataspace, std::string& shader) { - switch (dataspace & HAL_DATASPACE_TRANSFER_MASK) { - case HAL_DATASPACE_TRANSFER_ST2084: - shader.append(R"( - - float3 EOTF(float3 color) { - float m1 = (2610.0 / 4096.0) / 4.0; - float m2 = (2523.0 / 4096.0) * 128.0; - float c1 = (3424.0 / 4096.0); - float c2 = (2413.0 / 4096.0) * 32.0; - float c3 = (2392.0 / 4096.0) * 32.0; - - float3 tmp = pow(clamp(color, 0.0, 1.0), 1.0 / float3(m2)); - tmp = max(tmp - c1, 0.0) / (c2 - c3 * tmp); - return pow(tmp, 1.0 / float3(m1)); - } - )"); - break; - case HAL_DATASPACE_TRANSFER_HLG: - shader.append(R"( - float EOTF_channel(float channel) { - const float a = 0.17883277; - const float b = 0.28466892; - const float c = 0.55991073; - return channel <= 0.5 ? channel * channel / 3.0 : - (exp((channel - c) / a) + b) / 12.0; - } - - float3 EOTF(float3 color) { - return float3(EOTF_channel(color.r), EOTF_channel(color.g), - EOTF_channel(color.b)); - } - )"); - break; - case HAL_DATASPACE_TRANSFER_LINEAR: - shader.append(R"( - float3 EOTF(float3 color) { - return color; - } - )"); - break; - case HAL_DATASPACE_TRANSFER_SMPTE_170M: - shader.append(R"( - - float EOTF_sRGB(float srgb) { - return srgb <= 0.08125 ? srgb / 4.50 : pow((srgb + 0.099) / 1.099, 1 / 0.45); - } - - float3 EOTF_sRGB(float3 srgb) { - return float3(EOTF_sRGB(srgb.r), EOTF_sRGB(srgb.g), EOTF_sRGB(srgb.b)); - } - - float3 EOTF(float3 srgb) { - return sign(srgb.rgb) * EOTF_sRGB(abs(srgb.rgb)); - } - )"); - break; - case HAL_DATASPACE_TRANSFER_GAMMA2_2: - shader.append(R"( - - float EOTF_sRGB(float srgb) { - return pow(srgb, 2.2); - } - - float3 EOTF_sRGB(float3 srgb) { - return float3(EOTF_sRGB(srgb.r), EOTF_sRGB(srgb.g), EOTF_sRGB(srgb.b)); - } - - float3 EOTF(float3 srgb) { - return sign(srgb.rgb) * EOTF_sRGB(abs(srgb.rgb)); - } - )"); - break; - case HAL_DATASPACE_TRANSFER_GAMMA2_6: - shader.append(R"( - - float EOTF_sRGB(float srgb) { - return pow(srgb, 2.6); - } - - float3 EOTF_sRGB(float3 srgb) { - return float3(EOTF_sRGB(srgb.r), EOTF_sRGB(srgb.g), EOTF_sRGB(srgb.b)); - } - - float3 EOTF(float3 srgb) { - return sign(srgb.rgb) * EOTF_sRGB(abs(srgb.rgb)); - } - )"); - break; - case HAL_DATASPACE_TRANSFER_GAMMA2_8: - shader.append(R"( - - float EOTF_sRGB(float srgb) { - return pow(srgb, 2.8); - } - - float3 EOTF_sRGB(float3 srgb) { - return float3(EOTF_sRGB(srgb.r), EOTF_sRGB(srgb.g), EOTF_sRGB(srgb.b)); - } - - float3 EOTF(float3 srgb) { - return sign(srgb.rgb) * EOTF_sRGB(abs(srgb.rgb)); - } - )"); - break; - case HAL_DATASPACE_TRANSFER_SRGB: - default: - shader.append(R"( - - float EOTF_sRGB(float srgb) { - return srgb <= 0.04045 ? srgb / 12.92 : pow((srgb + 0.055) / 1.055, 2.4); - } - - float3 EOTF_sRGB(float3 srgb) { - return float3(EOTF_sRGB(srgb.r), EOTF_sRGB(srgb.g), EOTF_sRGB(srgb.b)); - } - - float3 EOTF(float3 srgb) { - return sign(srgb.rgb) * EOTF_sRGB(abs(srgb.rgb)); - } - )"); - break; - } -} - void generateXYZTransforms(std::string& shader) { shader.append(R"( - uniform float4x4 in_rgbToXyz; - uniform float4x4 in_xyzToRgb; + uniform float3x3 in_rgbToXyz; + uniform float3x3 in_xyzToSrcRgb; + uniform float4x4 in_colorTransform; float3 ToXYZ(float3 rgb) { - return (in_rgbToXyz * float4(rgb, 1.0)).rgb; + return in_rgbToXyz * rgb; + } + + float3 ToSrcRGB(float3 xyz) { + return in_xyzToSrcRgb * xyz; } - float3 ToRGB(float3 xyz) { - return clamp((in_xyzToRgb * float4(xyz, 1.0)).rgb, 0.0, 1.0); + float3 ApplyColorTransform(float3 rgb) { + return (in_colorTransform * float4(rgb, 1.0)).rgb; } )"); } -// Conversion from relative light to absolute light (maps from [0, 1] to [0, maxNits]) -void generateLuminanceScalesForOOTF(ui::Dataspace inputDataspace, ui::Dataspace outputDataspace, - std::string& shader) { +// Conversion from relative light to absolute light +// Note that 1.0 == 203 nits. +void generateLuminanceScalesForOOTF(ui::Dataspace inputDataspace, std::string& shader) { switch (inputDataspace & HAL_DATASPACE_TRANSFER_MASK) { - case HAL_DATASPACE_TRANSFER_ST2084: - shader.append(R"( - float3 ScaleLuminance(float3 xyz) { - return xyz * 10000.0; - } - )"); - break; case HAL_DATASPACE_TRANSFER_HLG: + // BT. 2408 says that a signal level of 0.75 == 203 nits for HLG, but that's after + // applying OOTF. But we haven't applied OOTF yet, so we need to scale by a different + // constant instead. shader.append(R"( - float3 ScaleLuminance(float3 xyz) { - return xyz * 1000.0; - } - )"); + float3 ScaleLuminance(float3 xyz) { + return xyz * 264.96; + } + )"); break; default: - // Input is SDR so map to its white point luminance - switch (outputDataspace & HAL_DATASPACE_TRANSFER_MASK) { - // Max HLG output is nominally 1000 nits, but BT. 2100-2 allows - // for gamma correcting the HLG OOTF for displays with a different - // dynamic range. Scale to 1000 nits to apply an inverse OOTF against - // a reference display correctly. - // TODO: Use knowledge of the dimming ratio here to prevent - // unintended gamma shaft. - case HAL_DATASPACE_TRANSFER_HLG: - shader.append(R"( - float3 ScaleLuminance(float3 xyz) { - return xyz * 1000.0; - } - )"); - break; - default: - shader.append(R"( - float3 ScaleLuminance(float3 xyz) { - return xyz * in_libtonemap_displayMaxLuminance; - } - )"); - break; - } + shader.append(R"( + float3 ScaleLuminance(float3 xyz) { + return xyz * 203.0; + } + )"); + break; } } @@ -224,17 +83,17 @@ static void generateLuminanceNormalizationForOOTF(ui::Dataspace inputDataspace, switch (outputDataspace & HAL_DATASPACE_TRANSFER_MASK) { case HAL_DATASPACE_TRANSFER_ST2084: shader.append(R"( - float3 NormalizeLuminance(float3 xyz) { - return xyz / 10000.0; - } - )"); + float3 NormalizeLuminance(float3 xyz) { + return xyz / 203.0; + } + )"); break; case HAL_DATASPACE_TRANSFER_HLG: switch (inputDataspace & HAL_DATASPACE_TRANSFER_MASK) { case HAL_DATASPACE_TRANSFER_HLG: shader.append(R"( float3 NormalizeLuminance(float3 xyz) { - return xyz / 1000.0; + return xyz / 264.96; } )"); break; @@ -242,24 +101,39 @@ static void generateLuminanceNormalizationForOOTF(ui::Dataspace inputDataspace, // Transcoding to HLG requires applying the inverse OOTF // with the expectation that the OOTF is then applied during // tonemapping downstream. + // BT. 2100-2 operates on normalized luminances, so renormalize to the input to + // correctly adjust gamma. shader.append(R"( float3 NormalizeLuminance(float3 xyz) { - // BT. 2100-2 operates on normalized luminances, - // so renormalize to the input - float ootfGain = pow(xyz.y / 1000.0, -0.2 / 1.2) / 1000.0; - return xyz * ootfGain; + float ootfGain = pow(xyz.y / 1000.0, -0.2 / 1.2); + return xyz * ootfGain / 203.0; } )"); break; } break; default: - shader.append(R"( - float3 NormalizeLuminance(float3 xyz) { - return xyz / in_libtonemap_displayMaxLuminance; - } - )"); - break; + switch (inputDataspace & HAL_DATASPACE_TRANSFER_MASK) { + case HAL_DATASPACE_TRANSFER_HLG: + case HAL_DATASPACE_TRANSFER_ST2084: + // libtonemap outputs a range [0, in_libtonemap_displayMaxLuminance], so + // normalize back to [0, 1] when the output is SDR. + shader.append(R"( + float3 NormalizeLuminance(float3 xyz) { + return xyz / in_libtonemap_displayMaxLuminance; + } + )"); + break; + default: + // Otherwise normalize back down to the range [0, 1] + // TODO: get this working for extended range outputs + shader.append(R"( + float3 NormalizeLuminance(float3 xyz) { + return xyz / 203.0; + } + )"); + break; + } } } @@ -270,145 +144,34 @@ void generateOOTF(ui::Dataspace inputDataspace, ui::Dataspace outputDataspace, toAidlDataspace(outputDataspace)) .c_str()); - generateLuminanceScalesForOOTF(inputDataspace, outputDataspace, shader); + generateLuminanceScalesForOOTF(inputDataspace, shader); generateLuminanceNormalizationForOOTF(inputDataspace, outputDataspace, shader); + // Some tonemappers operate on CIE luminance, other tonemappers operate on linear rgb + // luminance in the source gamut. shader.append(R"( - float3 OOTF(float3 linearRGB, float3 xyz) { + float3 OOTF(float3 linearRGB) { float3 scaledLinearRGB = ScaleLuminance(linearRGB); - float3 scaledXYZ = ScaleLuminance(xyz); + float3 scaledXYZ = ToXYZ(scaledLinearRGB); - float gain = libtonemap_LookupTonemapGain(scaledLinearRGB, scaledXYZ); + float gain = libtonemap_LookupTonemapGain(ToSrcRGB(scaledXYZ), scaledXYZ); return NormalizeLuminance(scaledXYZ * gain); } )"); } -void generateOETF(ui::Dataspace dataspace, std::string& shader) { - switch (dataspace & HAL_DATASPACE_TRANSFER_MASK) { - case HAL_DATASPACE_TRANSFER_ST2084: - shader.append(R"( - - float3 OETF(float3 xyz) { - float m1 = (2610.0 / 4096.0) / 4.0; - float m2 = (2523.0 / 4096.0) * 128.0; - float c1 = (3424.0 / 4096.0); - float c2 = (2413.0 / 4096.0) * 32.0; - float c3 = (2392.0 / 4096.0) * 32.0; - - float3 tmp = pow(xyz, float3(m1)); - tmp = (c1 + c2 * tmp) / (1.0 + c3 * tmp); - return pow(tmp, float3(m2)); - } - )"); - break; - case HAL_DATASPACE_TRANSFER_HLG: - shader.append(R"( - float OETF_channel(float channel) { - const float a = 0.17883277; - const float b = 0.28466892; - const float c = 0.55991073; - return channel <= 1.0 / 12.0 ? sqrt(3.0 * channel) : - a * log(12.0 * channel - b) + c; - } - - float3 OETF(float3 linear) { - return float3(OETF_channel(linear.r), OETF_channel(linear.g), - OETF_channel(linear.b)); - } - )"); - break; - case HAL_DATASPACE_TRANSFER_LINEAR: - shader.append(R"( - float3 OETF(float3 linear) { - return linear; - } - )"); - break; - case HAL_DATASPACE_TRANSFER_SMPTE_170M: - shader.append(R"( - float OETF_sRGB(float linear) { - return linear <= 0.018 ? - linear * 4.50 : (pow(linear, 0.45) * 1.099) - 0.099; - } - - float3 OETF_sRGB(float3 linear) { - return float3(OETF_sRGB(linear.r), OETF_sRGB(linear.g), OETF_sRGB(linear.b)); - } - - float3 OETF(float3 linear) { - return sign(linear.rgb) * OETF_sRGB(abs(linear.rgb)); - } - )"); - break; - case HAL_DATASPACE_TRANSFER_GAMMA2_2: - shader.append(R"( - float OETF_sRGB(float linear) { - return pow(linear, (1.0 / 2.2)); - } - - float3 OETF_sRGB(float3 linear) { - return float3(OETF_sRGB(linear.r), OETF_sRGB(linear.g), OETF_sRGB(linear.b)); - } - - float3 OETF(float3 linear) { - return sign(linear.rgb) * OETF_sRGB(abs(linear.rgb)); - } - )"); - break; - case HAL_DATASPACE_TRANSFER_GAMMA2_6: - shader.append(R"( - float OETF_sRGB(float linear) { - return pow(linear, (1.0 / 2.6)); - } - - float3 OETF_sRGB(float3 linear) { - return float3(OETF_sRGB(linear.r), OETF_sRGB(linear.g), OETF_sRGB(linear.b)); - } - - float3 OETF(float3 linear) { - return sign(linear.rgb) * OETF_sRGB(abs(linear.rgb)); - } - )"); - break; - case HAL_DATASPACE_TRANSFER_GAMMA2_8: - shader.append(R"( - float OETF_sRGB(float linear) { - return pow(linear, (1.0 / 2.8)); - } - - float3 OETF_sRGB(float3 linear) { - return float3(OETF_sRGB(linear.r), OETF_sRGB(linear.g), OETF_sRGB(linear.b)); - } - - float3 OETF(float3 linear) { - return sign(linear.rgb) * OETF_sRGB(abs(linear.rgb)); - } - )"); - break; - case HAL_DATASPACE_TRANSFER_SRGB: - default: - shader.append(R"( - float OETF_sRGB(float linear) { - return linear <= 0.0031308 ? - linear * 12.92 : (pow(linear, 1.0 / 2.4) * 1.055) - 0.055; - } - - float3 OETF_sRGB(float3 linear) { - return float3(OETF_sRGB(linear.r), OETF_sRGB(linear.g), OETF_sRGB(linear.b)); - } - - float3 OETF(float3 linear) { - return sign(linear.rgb) * OETF_sRGB(abs(linear.rgb)); - } - )"); - break; - } +void generateOETF(std::string& shader) { + // Only support gamma 2.2 for now + shader.append(R"( + float OETF(float3 linear) { + return sign(linear) * pow(abs(linear), (1.0 / 2.2)); + } + )"); } void generateEffectiveOOTF(bool undoPremultipliedAlpha, LinearEffect::SkSLType type, - std::string& shader) { + bool needsCustomOETF, std::string& shader) { switch (type) { case LinearEffect::SkSLType::ColorFilter: shader.append(R"( @@ -429,11 +192,19 @@ void generateEffectiveOOTF(bool undoPremultipliedAlpha, LinearEffect::SkSLType t c.rgb = c.rgb / (c.a + 0.0019); )"); } + // We are using linear sRGB as a working space, with 1.0 == 203 nits shader.append(R"( - float3 linearRGB = EOTF(c.rgb); - float3 xyz = ToXYZ(linearRGB); - c.rgb = OETF(ToRGB(OOTF(linearRGB, xyz))); + c.rgb = ApplyColorTransform(OOTF(toLinearSrgb(c.rgb))); )"); + if (needsCustomOETF) { + shader.append(R"( + c.rgb = OETF(c.rgb); + )"); + } else { + shader.append(R"( + c.rgb = fromLinearSrgb(c.rgb); + )"); + } if (undoPremultipliedAlpha) { shader.append(R"( c.rgb = c.rgb * (c.a + 0.0019); @@ -445,7 +216,31 @@ void generateEffectiveOOTF(bool undoPremultipliedAlpha, LinearEffect::SkSLType t )"); } -// please keep in sync with toSkColorSpace function in renderengine/skia/ColorSpaces.cpp +template ::value, bool> = true> +std::vector buildUniformValue(T value) { + std::vector result; + result.resize(sizeof(value)); + std::memcpy(result.data(), &value, sizeof(value)); + return result; +} + +} // namespace + +std::string buildLinearEffectSkSL(const LinearEffect& linearEffect) { + std::string shaderString; + generateXYZTransforms(shaderString); + generateOOTF(linearEffect.inputDataspace, linearEffect.outputDataspace, shaderString); + + const bool needsCustomOETF = (linearEffect.fakeOutputDataspace & HAL_DATASPACE_TRANSFER_MASK) == + HAL_DATASPACE_TRANSFER_GAMMA2_2; + if (needsCustomOETF) { + generateOETF(shaderString); + } + generateEffectiveOOTF(linearEffect.undoPremultipliedAlpha, linearEffect.type, needsCustomOETF, + shaderString); + return shaderString; +} + ColorSpace toColorSpace(ui::Dataspace dataspace) { switch (dataspace & HAL_DATASPACE_STANDARD_MASK) { case HAL_DATASPACE_STANDARD_BT709: @@ -457,14 +252,14 @@ ColorSpace toColorSpace(ui::Dataspace dataspace) { return ColorSpace::BT2020(); case HAL_DATASPACE_STANDARD_ADOBE_RGB: return ColorSpace::AdobeRGB(); - // TODO(b/208290320): BT601 format and variants return different primaries + // TODO(b/208290320): BT601 format and variants return different primaries case HAL_DATASPACE_STANDARD_BT601_625: case HAL_DATASPACE_STANDARD_BT601_625_UNADJUSTED: case HAL_DATASPACE_STANDARD_BT601_525: case HAL_DATASPACE_STANDARD_BT601_525_UNADJUSTED: - // TODO(b/208290329): BT407M format returns different primaries + // TODO(b/208290329): BT407M format returns different primaries case HAL_DATASPACE_STANDARD_BT470M: - // TODO(b/208290904): FILM format returns different primaries + // TODO(b/208290904): FILM format returns different primaries case HAL_DATASPACE_STANDARD_FILM: case HAL_DATASPACE_STANDARD_UNSPECIFIED: default: @@ -472,29 +267,6 @@ ColorSpace toColorSpace(ui::Dataspace dataspace) { } } -template ::value, bool> = true> -std::vector buildUniformValue(T value) { - std::vector result; - result.resize(sizeof(value)); - std::memcpy(result.data(), &value, sizeof(value)); - return result; -} - -} // namespace - -std::string buildLinearEffectSkSL(const LinearEffect& linearEffect) { - std::string shaderString; - generateEOTF(linearEffect.fakeInputDataspace == ui::Dataspace::UNKNOWN - ? linearEffect.inputDataspace - : linearEffect.fakeInputDataspace, - shaderString); - generateXYZTransforms(shaderString); - generateOOTF(linearEffect.inputDataspace, linearEffect.outputDataspace, shaderString); - generateOETF(linearEffect.outputDataspace, shaderString); - generateEffectiveOOTF(linearEffect.undoPremultipliedAlpha, linearEffect.type, shaderString); - return shaderString; -} - // Generates a list of uniforms to set on the LinearEffect shader above. std::vector buildLinearEffectUniforms( const LinearEffect& linearEffect, const mat4& colorTransform, float maxDisplayLuminance, @@ -502,23 +274,24 @@ std::vector buildLinearEffectUniforms( aidl::android::hardware::graphics::composer3::RenderIntent renderIntent) { std::vector uniforms; - const ui::Dataspace inputDataspace = linearEffect.fakeInputDataspace == ui::Dataspace::UNKNOWN - ? linearEffect.inputDataspace - : linearEffect.fakeInputDataspace; - - if (inputDataspace == linearEffect.outputDataspace) { - uniforms.push_back({.name = "in_rgbToXyz", .value = buildUniformValue(mat4())}); - uniforms.push_back( - {.name = "in_xyzToRgb", .value = buildUniformValue(colorTransform)}); - } else { - ColorSpace inputColorSpace = toColorSpace(inputDataspace); - ColorSpace outputColorSpace = toColorSpace(linearEffect.outputDataspace); - uniforms.push_back({.name = "in_rgbToXyz", - .value = buildUniformValue(mat4(inputColorSpace.getRGBtoXYZ()))}); - uniforms.push_back({.name = "in_xyzToRgb", - .value = buildUniformValue( - colorTransform * mat4(outputColorSpace.getXYZtoRGB()))}); - } + auto inputColorSpace = toColorSpace(linearEffect.inputDataspace); + auto outputColorSpace = toColorSpace(linearEffect.outputDataspace); + + uniforms.push_back( + {.name = "in_rgbToXyz", + .value = buildUniformValue(ColorSpace::linearExtendedSRGB().getRGBtoXYZ())}); + uniforms.push_back({.name = "in_xyzToSrcRgb", + .value = buildUniformValue(inputColorSpace.getXYZtoRGB())}); + // Transforms xyz colors to linear source colors, then applies the color transform, then + // transforms to linear extended RGB for skia to color manage. + uniforms.push_back({.name = "in_colorTransform", + .value = buildUniformValue( + mat4(ColorSpace::linearExtendedSRGB().getXYZtoRGB()) * + // TODO: the color transform ideally should be applied + // in the source colorspace, but doing that breaks + // renderengine tests + mat4(outputColorSpace.getRGBtoXYZ()) * colorTransform * + mat4(outputColorSpace.getXYZtoRGB()))}); tonemap::Metadata metadata{.displayMaxLuminance = maxDisplayLuminance, // If the input luminance is unknown, use display luminance (aka, diff --git a/libs/shaders/tests/shaders_test.cpp b/libs/shaders/tests/shaders_test.cpp index d45fb246c7..ba8bed23e3 100644 --- a/libs/shaders/tests/shaders_test.cpp +++ b/libs/shaders/tests/shaders_test.cpp @@ -35,6 +35,10 @@ MATCHER_P2(UniformEq, name, value, "") { return arg.name == name && arg.value == value; } +MATCHER_P(UniformNameEq, name, "") { + return arg.name == name; +} + template ::value, bool> = true> std::vector buildUniformValue(T value) { std::vector result; @@ -49,50 +53,44 @@ TEST_F(ShadersTest, buildLinearEffectUniforms_selectsNoOpGamutMatrices) { shaders::LinearEffect effect = shaders::LinearEffect{.inputDataspace = ui::Dataspace::V0_SRGB_LINEAR, .outputDataspace = ui::Dataspace::V0_SRGB_LINEAR, - .fakeInputDataspace = ui::Dataspace::UNKNOWN}; + .fakeOutputDataspace = ui::Dataspace::UNKNOWN}; mat4 colorTransform = mat4::scale(vec4(.9, .9, .9, 1.)); auto uniforms = shaders::buildLinearEffectUniforms(effect, colorTransform, 1.f, 1.f, 1.f, nullptr, aidl::android::hardware::graphics::composer3:: RenderIntent::COLORIMETRIC); - EXPECT_THAT(uniforms, Contains(UniformEq("in_rgbToXyz", buildUniformValue(mat4())))); EXPECT_THAT(uniforms, - Contains(UniformEq("in_xyzToRgb", buildUniformValue(colorTransform)))); + Contains(UniformEq("in_rgbToXyz", + buildUniformValue( + ColorSpace::linearExtendedSRGB().getRGBtoXYZ())))); + EXPECT_THAT(uniforms, + Contains(UniformEq("in_xyzToSrcRgb", + buildUniformValue( + ColorSpace::linearSRGB().getXYZtoRGB())))); + // color transforms are already tested in renderengine's tests + EXPECT_THAT(uniforms, Contains(UniformNameEq("in_colorTransform"))); } TEST_F(ShadersTest, buildLinearEffectUniforms_selectsGamutTransformMatrices) { shaders::LinearEffect effect = shaders::LinearEffect{.inputDataspace = ui::Dataspace::V0_SRGB, .outputDataspace = ui::Dataspace::DISPLAY_P3, - .fakeInputDataspace = ui::Dataspace::UNKNOWN}; + .fakeOutputDataspace = ui::Dataspace::UNKNOWN}; ColorSpace inputColorSpace = ColorSpace::sRGB(); - ColorSpace outputColorSpace = ColorSpace::DisplayP3(); auto uniforms = shaders::buildLinearEffectUniforms(effect, mat4(), 1.f, 1.f, 1.f, nullptr, aidl::android::hardware::graphics::composer3:: RenderIntent::COLORIMETRIC); EXPECT_THAT(uniforms, Contains(UniformEq("in_rgbToXyz", - buildUniformValue(mat4(inputColorSpace.getRGBtoXYZ()))))); + buildUniformValue( + ColorSpace::linearExtendedSRGB().getRGBtoXYZ())))); EXPECT_THAT(uniforms, - Contains(UniformEq("in_xyzToRgb", - buildUniformValue(mat4(outputColorSpace.getXYZtoRGB()))))); -} - -TEST_F(ShadersTest, buildLinearEffectUniforms_respectsFakeInputDataspace) { - shaders::LinearEffect effect = - shaders::LinearEffect{.inputDataspace = ui::Dataspace::V0_SRGB, - .outputDataspace = ui::Dataspace::DISPLAY_P3, - .fakeInputDataspace = ui::Dataspace::DISPLAY_P3}; - - auto uniforms = - shaders::buildLinearEffectUniforms(effect, mat4(), 1.f, 1.f, 1.f, nullptr, - aidl::android::hardware::graphics::composer3:: - RenderIntent::COLORIMETRIC); - EXPECT_THAT(uniforms, Contains(UniformEq("in_rgbToXyz", buildUniformValue(mat4())))); - EXPECT_THAT(uniforms, Contains(UniformEq("in_xyzToRgb", buildUniformValue(mat4())))); + Contains(UniformEq("in_xyzToSrcRgb", + buildUniformValue(inputColorSpace.getXYZtoRGB())))); + EXPECT_THAT(uniforms, Contains(UniformNameEq("in_colorTransform"))); } } // namespace android -- cgit v1.2.3-59-g8ed1b From 792b150a708a61faedaaa5983fbc5c5185984f50 Mon Sep 17 00:00:00 2001 From: Alec Mouri Date: Tue, 2 May 2023 00:03:08 +0000 Subject: Don't overdim SDR content in an HLG screenshot. Aligning HLG and PQ to 1.0 == 203 nits made SDR assets in screenshots too dim, since both the colorspace and the color transform applied dimming. Removing dimming application from the color transform is a larger change, so just compensate when configuring the screenshot in SurfaceFlinger instead. Bug: 280347733 Test: HwAccelerationTest Test: Navigate in and out of recents Change-Id: Idfdb74c0c3b977717b870b2bb9a469be37d27dc9 --- libs/shaders/shaders.cpp | 6 +++++- services/surfaceflinger/ScreenCaptureOutput.cpp | 10 ++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) (limited to 'libs/shaders/shaders.cpp') diff --git a/libs/shaders/shaders.cpp b/libs/shaders/shaders.cpp index b8f2be111e..c85517a976 100644 --- a/libs/shaders/shaders.cpp +++ b/libs/shaders/shaders.cpp @@ -103,10 +103,14 @@ static void generateLuminanceNormalizationForOOTF(ui::Dataspace inputDataspace, // tonemapping downstream. // BT. 2100-2 operates on normalized luminances, so renormalize to the input to // correctly adjust gamma. + // Note that following BT. 2408 for HLG OETF actually maps 0.75 == ~264.96 nits, + // rather than 203 nits, because 203 nits == OOTF(invOETF(0.75)), so even though + // we originally scaled by 203 nits we need to re-normalize to 264.96 nits when + // converting to the correct brightness range. shader.append(R"( float3 NormalizeLuminance(float3 xyz) { float ootfGain = pow(xyz.y / 1000.0, -0.2 / 1.2); - return xyz * ootfGain / 203.0; + return xyz * ootfGain / 264.96; } )"); break; diff --git a/services/surfaceflinger/ScreenCaptureOutput.cpp b/services/surfaceflinger/ScreenCaptureOutput.cpp index b70b53d05a..09dac23410 100644 --- a/services/surfaceflinger/ScreenCaptureOutput.cpp +++ b/services/surfaceflinger/ScreenCaptureOutput.cpp @@ -94,6 +94,16 @@ ScreenCaptureOutput::generateClientCompositionRequests( } } + if (outputDataspace == ui::Dataspace::BT2020_HLG) { + for (auto& layer : clientCompositionLayers) { + auto transfer = layer.sourceDataspace & ui::Dataspace::TRANSFER_MASK; + if (transfer != static_cast(ui::Dataspace::TRANSFER_HLG) && + transfer != static_cast(ui::Dataspace::TRANSFER_ST2084)) { + layer.whitePointNits *= (1000.0f / 203.0f); + } + } + } + Rect sourceCrop = mRenderArea.getSourceCrop(); compositionengine::LayerFE::LayerSettings fillLayer; fillLayer.source.buffer.buffer = nullptr; -- cgit v1.2.3-59-g8ed1b