diff options
13 files changed, 79 insertions, 0 deletions
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h index 5846e674f5..db2fd1b500 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h @@ -267,6 +267,9 @@ public: // Enables predicting composition strategy to run client composition earlier virtual void setPredictCompositionStrategy(bool) = 0; + // Enables overriding the 170M trasnfer function as sRGB + virtual void setTreat170mAsSrgb(bool) = 0; + protected: virtual void setDisplayColorProfile(std::unique_ptr<DisplayColorProfile>) = 0; virtual void setRenderSurface(std::unique_ptr<RenderSurface>) = 0; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h index 0feb9f7fb7..31c51e6a8d 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h @@ -108,6 +108,7 @@ public: void cacheClientCompositionRequests(uint32_t) override; bool canPredictCompositionStrategy(const CompositionRefreshArgs&) override; void setPredictCompositionStrategy(bool) override; + void setTreat170mAsSrgb(bool) override; // Testing const ReleasedLayers& getReleasedLayersForTest() const; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h index 61be983480..c65d467b0d 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h @@ -162,6 +162,8 @@ struct OutputCompositionState { CompositionStrategyPredictionState strategyPrediction = CompositionStrategyPredictionState::DISABLED; + bool treat170mAsSrgb = false; + // Debugging void dump(std::string& result) const; }; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h index fa86076b44..cb9fbad8dd 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h @@ -132,6 +132,7 @@ public: MOCK_METHOD1(cacheClientCompositionRequests, void(uint32_t)); MOCK_METHOD1(canPredictCompositionStrategy, bool(const CompositionRefreshArgs&)); MOCK_METHOD1(setPredictCompositionStrategy, void(bool)); + MOCK_METHOD1(setTreat170mAsSrgb, void(bool)); }; } // namespace android::compositionengine::mock diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp index ec8673177f..4c30f99610 100644 --- a/services/surfaceflinger/CompositionEngine/src/Output.cpp +++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp @@ -1488,6 +1488,10 @@ void Output::setPredictCompositionStrategy(bool predict) { } } +void Output::setTreat170mAsSrgb(bool enable) { + editState().treat170mAsSrgb = enable; +} + bool Output::canPredictCompositionStrategy(const CompositionRefreshArgs& refreshArgs) { if (!getState().isEnabled || !mHwComposerAsyncWorker) { ALOGV("canPredictCompositionStrategy disabled"); diff --git a/services/surfaceflinger/CompositionEngine/src/OutputCompositionState.cpp b/services/surfaceflinger/CompositionEngine/src/OutputCompositionState.cpp index 3b85e3b5b0..948c0c90bf 100644 --- a/services/surfaceflinger/CompositionEngine/src/OutputCompositionState.cpp +++ b/services/surfaceflinger/CompositionEngine/src/OutputCompositionState.cpp @@ -66,6 +66,9 @@ void OutputCompositionState::dump(std::string& out) const { out.append("\n "); dumpVal(out, "compositionStrategyPredictionState", ftl::enum_string(strategyPrediction)); + out.append("\n "); + dumpVal(out, "treate170mAsSrgb", treat170mAsSrgb); + out += '\n'; } diff --git a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp index 3289d55870..61c53df38d 100644 --- a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp +++ b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp @@ -322,6 +322,17 @@ void OutputLayer::updateCompositionState( ? outputState.targetDataspace : layerFEState->dataspace; + // Override the dataspace transfer from 170M to sRGB if the device configuration requests this. + // We do this here instead of in buffer info so that dumpsys can still report layers that are + // using the 170M transfer. Also we only do this if the colorspace is not agnostic for the + // layer, in case the color profile uses a 170M transfer function. + if (outputState.treat170mAsSrgb && !layerFEState->isColorspaceAgnostic && + (state.dataspace & HAL_DATASPACE_TRANSFER_MASK) == HAL_DATASPACE_TRANSFER_SMPTE_170M) { + state.dataspace = static_cast<ui::Dataspace>( + (state.dataspace & HAL_DATASPACE_STANDARD_MASK) | + (state.dataspace & HAL_DATASPACE_RANGE_MASK) | HAL_DATASPACE_TRANSFER_SRGB); + } + // For hdr content, treat the white point as the display brightness - HDR content should not be // boosted or dimmed. // If the layer explicitly requests to disable dimming, then don't dim either. diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp index ceee48c1ef..ca4f39729a 100644 --- a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp @@ -657,6 +657,23 @@ TEST_F(OutputLayerUpdateCompositionStateTest, setsOutputLayerColorspaceCorrectly EXPECT_EQ(ui::Dataspace::V0_SCRGB, mOutputLayer.getState().dataspace); } +TEST_F(OutputLayerUpdateCompositionStateTest, setsOutputLayerColorspaceWith170mReplacement) { + mLayerFEState.dataspace = ui::Dataspace::TRANSFER_SMPTE_170M; + mOutputState.targetDataspace = ui::Dataspace::V0_SCRGB; + mOutputState.treat170mAsSrgb = false; + mLayerFEState.isColorspaceAgnostic = false; + + mOutputLayer.updateCompositionState(false, false, ui::Transform::RotationFlags::ROT_0); + + EXPECT_EQ(ui::Dataspace::TRANSFER_SMPTE_170M, mOutputLayer.getState().dataspace); + + // Rewrite SMPTE 170M as sRGB + mOutputState.treat170mAsSrgb = true; + mOutputLayer.updateCompositionState(false, false, ui::Transform::RotationFlags::ROT_0); + + EXPECT_EQ(ui::Dataspace::TRANSFER_SRGB, mOutputLayer.getState().dataspace); +} + TEST_F(OutputLayerUpdateCompositionStateTest, setsWhitePointNitsAndDimmingRatioCorrectly) { mOutputState.sdrWhitePointNits = 200.f; mOutputState.displayBrightnessNits = 800.f; diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp index 42c8b37710..3a3c91e518 100644 --- a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp @@ -248,6 +248,20 @@ TEST_F(OutputTest, setCompositionEnabledSetsDisabledAndDirtiesEntireOutput) { } /* + * Output::setTreat170mAsSrgb() + */ + +TEST_F(OutputTest, setTreat170mAsSrgb) { + EXPECT_FALSE(mOutput->getState().treat170mAsSrgb); + + mOutput->setTreat170mAsSrgb(true); + EXPECT_TRUE(mOutput->getState().treat170mAsSrgb); + + mOutput->setTreat170mAsSrgb(false); + EXPECT_FALSE(mOutput->getState().treat170mAsSrgb); +} + +/* * Output::setLayerCachingEnabled() */ diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp index 52529d6b04..a915b615d9 100644 --- a/services/surfaceflinger/DisplayDevice.cpp +++ b/services/surfaceflinger/DisplayDevice.cpp @@ -89,6 +89,7 @@ DisplayDevice::DisplayDevice(DisplayDeviceCreationArgs& args) } mCompositionDisplay->setPredictCompositionStrategy(mFlinger->mPredictCompositionStrategy); + mCompositionDisplay->setTreat170mAsSrgb(mFlinger->mTreat170mAsSrgb); mCompositionDisplay->createDisplayColorProfile( compositionengine::DisplayColorProfileCreationArgsBuilder() .setHasWideColorGamut(args.hasWideColorGamut) diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 3a92ca49a2..e1eec8b97e 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -47,6 +47,7 @@ #include <stdint.h> #include <stdlib.h> #include <sys/types.h> +#include <system/graphics-base-v1.0.h> #include <ui/DataspaceUtils.h> #include <ui/DebugUtils.h> #include <ui/GraphicBuffer.h> @@ -600,6 +601,18 @@ std::optional<compositionengine::LayerFE::LayerSettings> Layer::prepareClientCom layerSettings.alpha = alpha; layerSettings.sourceDataspace = getDataSpace(); + // Override the dataspace transfer from 170M to sRGB if the device configuration requests this. + // We do this here instead of in buffer info so that dumpsys can still report layers that are + // using the 170M transfer. + if (mFlinger->mTreat170mAsSrgb && + (layerSettings.sourceDataspace & HAL_DATASPACE_TRANSFER_MASK) == + HAL_DATASPACE_TRANSFER_SMPTE_170M) { + layerSettings.sourceDataspace = static_cast<ui::Dataspace>( + (layerSettings.sourceDataspace & HAL_DATASPACE_STANDARD_MASK) | + (layerSettings.sourceDataspace & HAL_DATASPACE_RANGE_MASK) | + HAL_DATASPACE_TRANSFER_SRGB); + } + layerSettings.whitePointNits = targetSettings.whitePointNits; switch (targetSettings.blurSetting) { case LayerFE::ClientCompositionTargetSettings::BlurSetting::Enabled: diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index d39176be14..65c1c97efb 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -419,6 +419,9 @@ SurfaceFlinger::SurfaceFlinger(Factory& factory) : SurfaceFlinger(factory, SkipI property_get("debug.sf.predict_hwc_composition_strategy", value, "1"); mPredictCompositionStrategy = atoi(value); + property_get("debug.sf.treat_170m_as_sRGB", value, "0"); + mTreat170mAsSrgb = atoi(value); + // We should be reading 'persist.sys.sf.color_saturation' here // but since /data may be encrypted, we need to wait until after vold // comes online to attempt to read the property. The property is diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 74e0407215..c70e1749ff 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -349,6 +349,12 @@ public: // run parallel to the hwc validateDisplay call and re-run if the predition is incorrect. bool mPredictCompositionStrategy = false; + // If true, then any layer with a SMPTE 170M transfer function is decoded using the sRGB + // transfer instead. This is mainly to preserve legacy behavior, where implementations treated + // SMPTE 170M as sRGB prior to color management being implemented, and now implementations rely + // on this behavior to increase contrast for some media sources. + bool mTreat170mAsSrgb = false; + protected: // We're reference counted, never destroy SurfaceFlinger directly virtual ~SurfaceFlinger(); |