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
diff --git a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
index 3e983f3..723593d 100644
--- a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp
@@ -325,7 +325,8 @@
// 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 @@
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 @@
}};
settings.sourceDataspace = getState().overrideInfo.dataspace;
settings.alpha = 1.0f;
+ settings.whitePointNits = getOutput().getState().sdrWhitePointNits;
return {static_cast<LayerFE::LayerSettings>(settings)};
}