diff options
| author | 2022-02-11 14:18:57 -0800 | |
|---|---|---|
| committer | 2022-02-24 02:14:33 +0000 | |
| commit | e8dd3562a802fa84f25bb9b2196119a125f8ee01 (patch) | |
| tree | d8025a182d7c40f8602e18eb0d9e54630eef669e | |
| parent | c6f3101a182aebed1446c17fb6807b251f4dfeca (diff) | |
Fix dimming flicker when entering layer caching.
Caching does not perform any dimming operations, which means forcing
cached layers to not dim in composer is not correct. So...dim the cached
layers as if they were SDR layers
Note that because caching does not perform any dimming, then this does
imply that some cached sets may permanently be in GPU composition if the
dimming ratio is large enough that the DPU cannot dim + dither.
If we wanted to resolve the power cost of steady-state dimming by a
large ratio, then a future patch would have to teach caching how to dim.
That approach would have a few potential difficulties:
1. Modulating the dimming ratio of cached SDR layers may cause cache
evictions when an HDR video comes on screen, since caching will have
to recomposite those layers due to the change in dimming ratio, which
can be a power regression.
2. Reusing caching results that take into account dimming may be
difficult, since dimming can cause crush at lower grey levels, so
brightening a dimmed cached set may cause degradation in image
quality.
(1) and (2) aren't impossible to solve, but would either require changes
to the composer interface to communicate a dimming ratio capability,
and/or would introduce complexity into caching to selecttively dim
cached results based on composer capabilities as well as what is
expected to be more efficient. Whereas this patch is pretty
straightforward.
Bug: 217794675
Test: HDR test video
Change-Id: I618a0616f49c6ae3feac5bedbb4f5b0e283f5da7
| -rw-r--r-- | services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp | 19 | ||||
| -rw-r--r-- | services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp | 17 |
2 files changed, 26 insertions, 10 deletions
diff --git a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp index 3e983f3b20..723593d7ac 100644 --- a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp +++ b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp @@ -325,7 +325,8 @@ void OutputLayer::updateCompositionState( // For hdr content, treat the white point as the display brightness - HDR content should not be // boosted or dimmed. if (isHdrDataspace(state.dataspace) || - getOutput().getState().displayBrightnessNits == getOutput().getState().sdrWhitePointNits) { + getOutput().getState().displayBrightnessNits == getOutput().getState().sdrWhitePointNits || + getOutput().getState().displayBrightnessNits == 0.f) { state.dimmingRatio = 1.f; state.whitePointNits = getOutput().getState().displayBrightnessNits; } else { @@ -506,9 +507,18 @@ void OutputLayer::writeOutputDependentPerFrameStateToHWC(HWC2::Layer* hwcLayer) to_string(error).c_str(), static_cast<int32_t>(error)); } - // Don't dim cached layers - const auto dimmingRatio = - outputDependentState.overrideInfo.buffer ? 1.f : outputDependentState.dimmingRatio; + // Cached layers are not dimmed, which means that composer should attempt to dim. + // Note that if the dimming ratio is large, then this may cause the cached layer + // to kick back into GPU composition :( + // Also note that this assumes that there are no HDR layers that are able to be cached. + // Otherwise, this could cause HDR layers to be dimmed twice. + const auto dimmingRatio = outputDependentState.overrideInfo.buffer + ? (getOutput().getState().displayBrightnessNits != 0.f + ? std::clamp(getOutput().getState().sdrWhitePointNits / + getOutput().getState().displayBrightnessNits, + 0.f, 1.f) + : 1.f) + : outputDependentState.dimmingRatio; if (auto error = hwcLayer->setBrightness(dimmingRatio); error != hal::Error::NONE) { ALOGE("[%s] Failed to set brightness %f: %s (%d)", getLayerFE().getDebugName(), @@ -788,6 +798,7 @@ std::vector<LayerFE::LayerSettings> OutputLayer::getOverrideCompositionList() co }}; settings.sourceDataspace = getState().overrideInfo.dataspace; settings.alpha = 1.0f; + settings.whitePointNits = getOutput().getState().sdrWhitePointNits; return {static_cast<LayerFE::LayerSettings>(settings)}; } diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp index dda0822b1f..8eb1946b67 100644 --- a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp @@ -657,7 +657,7 @@ TEST_F(OutputLayerUpdateCompositionStateTest, setsOutputLayerColorspaceCorrectly EXPECT_EQ(ui::Dataspace::V0_SCRGB, mOutputLayer.getState().dataspace); } -TEST_F(OutputLayerUpdateCompositionStateTest, setsWhitePointNitsCorrectly) { +TEST_F(OutputLayerUpdateCompositionStateTest, setsWhitePointNitsAndDimmingRatioCorrectly) { mOutputState.sdrWhitePointNits = 200.f; mOutputState.displayBrightnessNits = 800.f; @@ -665,12 +665,15 @@ TEST_F(OutputLayerUpdateCompositionStateTest, setsWhitePointNitsCorrectly) { mLayerFEState.isColorspaceAgnostic = false; mOutputLayer.updateCompositionState(false, false, ui::Transform::RotationFlags::ROT_0); EXPECT_EQ(mOutputState.sdrWhitePointNits, mOutputLayer.getState().whitePointNits); + EXPECT_EQ(mOutputState.sdrWhitePointNits / mOutputState.displayBrightnessNits, + mOutputLayer.getState().dimmingRatio); mLayerFEState.dataspace = ui::Dataspace::BT2020_ITU_PQ; mLayerFEState.isColorspaceAgnostic = false; mOutputLayer.updateCompositionState(false, false, ui::Transform::RotationFlags::ROT_0); EXPECT_EQ(mOutputState.displayBrightnessNits, mOutputLayer.getState().whitePointNits); + EXPECT_EQ(1.f, mOutputLayer.getState().dimmingRatio); } TEST_F(OutputLayerUpdateCompositionStateTest, doesNotRecomputeGeometryIfNotRequested) { @@ -750,9 +753,10 @@ struct OutputLayerWriteStateToHWCTest : public OutputLayerTest { static constexpr bool kLayerGenericMetadata1Mandatory = true; static constexpr bool kLayerGenericMetadata2Mandatory = true; static constexpr float kWhitePointNits = 200.f; + static constexpr float kSdrWhitePointNits = 100.f; static constexpr float kDisplayBrightnessNits = 400.f; static constexpr float kLayerBrightness = kWhitePointNits / kDisplayBrightnessNits; - static constexpr float kFullLayerBrightness = 1.f; + static constexpr float kOverrideLayerBrightness = kSdrWhitePointNits / kDisplayBrightnessNits; static const half4 kColor; static const Rect kDisplayFrame; @@ -798,6 +802,7 @@ struct OutputLayerWriteStateToHWCTest : public OutputLayerTest { mLayerFEState.acquireFence = kFence; mOutputState.displayBrightnessNits = kDisplayBrightnessNits; + mOutputState.sdrWhitePointNits = kSdrWhitePointNits; EXPECT_CALL(mOutput, getDisplayColorProfile()) .WillRepeatedly(Return(&mDisplayColorProfile)); @@ -1117,7 +1122,7 @@ TEST_F(OutputLayerWriteStateToHWCTest, overriddenSkipLayerDoesNotSendBuffer) { expectGeometryCommonCalls(kOverrideDisplayFrame, kOverrideSourceCrop, kOverrideBufferTransform, kOverrideBlendMode, kSkipAlpha); expectPerFrameCommonCalls(SimulateUnsupported::None, kOverrideDataspace, kOverrideVisibleRegion, - kOverrideSurfaceDamage, kFullLayerBrightness); + kOverrideSurfaceDamage, kOverrideLayerBrightness); expectSetHdrMetadataAndBufferCalls(); expectSetCompositionTypeCall(Composition::DEVICE); EXPECT_CALL(*mLayerFE, hasRoundedCorners()).WillRepeatedly(Return(false)); @@ -1133,7 +1138,7 @@ TEST_F(OutputLayerWriteStateToHWCTest, overriddenSkipLayerForSolidColorDoesNotSe expectGeometryCommonCalls(kOverrideDisplayFrame, kOverrideSourceCrop, kOverrideBufferTransform, kOverrideBlendMode, kSkipAlpha); expectPerFrameCommonCalls(SimulateUnsupported::None, kOverrideDataspace, kOverrideVisibleRegion, - kOverrideSurfaceDamage, kFullLayerBrightness); + kOverrideSurfaceDamage, kOverrideLayerBrightness); expectSetHdrMetadataAndBufferCalls(); expectSetCompositionTypeCall(Composition::DEVICE); EXPECT_CALL(*mLayerFE, hasRoundedCorners()).WillRepeatedly(Return(false)); @@ -1149,7 +1154,7 @@ TEST_F(OutputLayerWriteStateToHWCTest, includesOverrideInfoIfPresent) { expectGeometryCommonCalls(kOverrideDisplayFrame, kOverrideSourceCrop, kOverrideBufferTransform, kOverrideBlendMode, kOverrideAlpha); expectPerFrameCommonCalls(SimulateUnsupported::None, kOverrideDataspace, kOverrideVisibleRegion, - kOverrideSurfaceDamage, kFullLayerBrightness); + kOverrideSurfaceDamage, kOverrideLayerBrightness); expectSetHdrMetadataAndBufferCalls(kOverrideHwcSlot, kOverrideBuffer, kOverrideFence); expectSetCompositionTypeCall(Composition::DEVICE); EXPECT_CALL(*mLayerFE, hasRoundedCorners()).WillRepeatedly(Return(false)); @@ -1165,7 +1170,7 @@ TEST_F(OutputLayerWriteStateToHWCTest, includesOverrideInfoForSolidColorIfPresen expectGeometryCommonCalls(kOverrideDisplayFrame, kOverrideSourceCrop, kOverrideBufferTransform, kOverrideBlendMode, kOverrideAlpha); expectPerFrameCommonCalls(SimulateUnsupported::None, kOverrideDataspace, kOverrideVisibleRegion, - kOverrideSurfaceDamage, kFullLayerBrightness); + kOverrideSurfaceDamage, kOverrideLayerBrightness); expectSetHdrMetadataAndBufferCalls(kOverrideHwcSlot, kOverrideBuffer, kOverrideFence); expectSetCompositionTypeCall(Composition::DEVICE); EXPECT_CALL(*mLayerFE, hasRoundedCorners()).WillRepeatedly(Return(false)); |