diff options
6 files changed, 129 insertions, 15 deletions
diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/CachedSet.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/CachedSet.h index e2d17ee502..86bcf20677 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/CachedSet.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/CachedSet.h @@ -143,7 +143,7 @@ public: compositionengine::OutputLayer* getBlurLayer() const; - bool hasUnsupportedDataspace() const; + bool hasKnownColorShift() const; bool hasProtectedLayers() const; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/LayerState.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/LayerState.h index dc3821ca43..5e3e3d8a31 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/LayerState.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/LayerState.h @@ -74,6 +74,7 @@ enum class LayerStateField : uint32_t { BlurRegions = 1u << 18, HasProtectedContent = 1u << 19, CachingHint = 1u << 20, + DimmingEnabled = 1u << 21, }; // clang-format on @@ -248,6 +249,10 @@ public: ui::Dataspace getDataspace() const { return mOutputDataspace.get(); } + hardware::graphics::composer::hal::PixelFormat getPixelFormat() const { + return mPixelFormat.get(); + } + float getHdrSdrRatio() const { return getOutputLayer()->getLayerFE().getCompositionState()->currentHdrSdrRatio; }; @@ -258,6 +263,8 @@ public: gui::CachingHint getCachingHint() const { return mCachingHint.get(); } + bool isDimmingEnabled() const { return mIsDimmingEnabled.get(); } + float getFps() const { return getOutputLayer()->getLayerFE().getCompositionState()->fps; } void dump(std::string& result) const; @@ -498,7 +505,10 @@ private: return std::vector<std::string>{toString(cachingHint)}; }}; - static const constexpr size_t kNumNonUniqueFields = 19; + OutputLayerState<bool, LayerStateField::DimmingEnabled> mIsDimmingEnabled{ + [](auto layer) { return layer->getLayerFE().getCompositionState()->dimmingEnabled; }}; + + static const constexpr size_t kNumNonUniqueFields = 20; std::array<StateInterface*, kNumNonUniqueFields> getNonUniqueFields() { std::array<const StateInterface*, kNumNonUniqueFields> constFields = @@ -516,7 +526,7 @@ private: &mAlpha, &mLayerMetadata, &mVisibleRegion, &mOutputDataspace, &mPixelFormat, &mColorTransform, &mCompositionType, &mSidebandStream, &mBuffer, &mSolidColor, &mBackgroundBlurRadius, &mBlurRegions, - &mFrameNumber, &mIsProtected, &mCachingHint}; + &mFrameNumber, &mIsProtected, &mCachingHint, &mIsDimmingEnabled}; } }; diff --git a/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp b/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp index 1f53588412..ea9442da06 100644 --- a/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp +++ b/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp @@ -27,8 +27,7 @@ #include <renderengine/DisplaySettings.h> #include <renderengine/RenderEngine.h> #include <ui/DebugUtils.h> -#include <utils/Trace.h> - +#include <ui/HdrRenderTypeUtils.h> #include <utils/Trace.h> namespace android::compositionengine::impl::planner { @@ -306,7 +305,7 @@ bool CachedSet::requiresHolePunch() const { return false; } - if (hasUnsupportedDataspace()) { + if (hasKnownColorShift()) { return false; } @@ -366,12 +365,21 @@ compositionengine::OutputLayer* CachedSet::getBlurLayer() const { return mBlurLayer ? mBlurLayer->getOutputLayer() : nullptr; } -bool CachedSet::hasUnsupportedDataspace() const { +bool CachedSet::hasKnownColorShift() const { return std::any_of(mLayers.cbegin(), mLayers.cend(), [](const Layer& layer) { auto dataspace = layer.getState()->getDataspace(); - const auto transfer = static_cast<ui::Dataspace>(dataspace & ui::Dataspace::TRANSFER_MASK); - if (transfer == ui::Dataspace::TRANSFER_ST2084 || transfer == ui::Dataspace::TRANSFER_HLG) { - // Skip HDR. + + // Layers are never dimmed when rendering a cached set, meaning that we may ask HWC to + // dim a cached set. But this means that we can never cache any HDR layers so that we + // don't accidentally dim those layers. + const auto hdrType = getHdrRenderType(dataspace, layer.getState()->getPixelFormat(), + layer.getState()->getHdrSdrRatio()); + if (hdrType != HdrRenderType::SDR) { + return true; + } + + // Layers that have dimming disabled pretend that they're HDR. + if (!layer.getState()->isDimmingEnabled()) { return true; } @@ -380,10 +388,6 @@ bool CachedSet::hasUnsupportedDataspace() const { // to avoid flickering/color differences. return true; } - // TODO(b/274804887): temp fix of overdimming issue, skip caching if hsdr/sdr ratio > 1.01f - if (layer.getState()->getHdrSdrRatio() > 1.01f) { - return true; - } return false; }); } diff --git a/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp b/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp index 0a5c43a99b..4bafed2c8e 100644 --- a/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp +++ b/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp @@ -439,7 +439,7 @@ std::vector<Flattener::Run> Flattener::findCandidateRuns(time_point now) const { if (!layerDeniedFromCaching && layerIsInactive && (firstLayer || runHasFirstLayer || !layerHasBlur) && - !currentSet->hasUnsupportedDataspace()) { + !currentSet->hasKnownColorShift()) { if (isPartOfRun) { builder.increment(); } else { diff --git a/services/surfaceflinger/CompositionEngine/tests/planner/FlattenerTest.cpp b/services/surfaceflinger/CompositionEngine/tests/planner/FlattenerTest.cpp index d2eff75fb6..54ee0efa11 100644 --- a/services/surfaceflinger/CompositionEngine/tests/planner/FlattenerTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/planner/FlattenerTest.cpp @@ -1339,6 +1339,55 @@ TEST_F(FlattenerTest, flattenLayers_skipsHDR2) { EXPECT_EQ(nullptr, overrideBuffer3); } +TEST_F(FlattenerTest, flattenLayers_skipsLayersDisablingDimming) { + auto& layerState1 = mTestLayers[0]->layerState; + const auto& overrideBuffer1 = layerState1->getOutputLayer()->getState().overrideInfo.buffer; + + auto& layerState2 = mTestLayers[1]->layerState; + const auto& overrideBuffer2 = layerState2->getOutputLayer()->getState().overrideInfo.buffer; + + // The third layer disables dimming, which means it should not be cached + auto& layerState3 = mTestLayers[2]->layerState; + const auto& overrideBuffer3 = layerState3->getOutputLayer()->getState().overrideInfo.buffer; + mTestLayers[2]->layerFECompositionState.dimmingEnabled = false; + mTestLayers[2]->layerState->update(&mTestLayers[2]->outputLayer); + + const std::vector<const LayerState*> layers = { + layerState1.get(), + layerState2.get(), + layerState3.get(), + }; + + initializeFlattener(layers); + + mTime += 200ms; + initializeOverrideBuffer(layers); + EXPECT_EQ(getNonBufferHash(layers), + mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime)); + + // This will render a CachedSet. + EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, _)) + .WillOnce(Return(ByMove(ftl::yield<FenceResult>(Fence::NO_FENCE)))); + mFlattener->renderCachedSets(mOutputState, std::nullopt, true); + + // We've rendered a CachedSet, but we haven't merged it in. + EXPECT_EQ(nullptr, overrideBuffer1); + EXPECT_EQ(nullptr, overrideBuffer2); + EXPECT_EQ(nullptr, overrideBuffer3); + + // This time we merge the CachedSet in, so we have a new hash, and we should + // only have two sets. + EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, _)).Times(0); + initializeOverrideBuffer(layers); + EXPECT_NE(getNonBufferHash(layers), + mFlattener->flattenLayers(layers, getNonBufferHash(layers), mTime)); + mFlattener->renderCachedSets(mOutputState, std::nullopt, true); + + EXPECT_NE(nullptr, overrideBuffer1); + EXPECT_EQ(overrideBuffer1, overrideBuffer2); + EXPECT_EQ(nullptr, overrideBuffer3); +} + TEST_F(FlattenerTest, flattenLayers_skipsColorLayers) { auto& layerState1 = mTestLayers[0]->layerState; const auto& overrideBuffer1 = layerState1->getOutputLayer()->getState().overrideInfo.buffer; diff --git a/services/surfaceflinger/CompositionEngine/tests/planner/LayerStateTest.cpp b/services/surfaceflinger/CompositionEngine/tests/planner/LayerStateTest.cpp index 954eb27181..03758b345a 100644 --- a/services/surfaceflinger/CompositionEngine/tests/planner/LayerStateTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/planner/LayerStateTest.cpp @@ -306,6 +306,16 @@ TEST_F(LayerStateTest, getCompositionType_forcedClient) { EXPECT_EQ(Composition::CLIENT, mLayerState->getCompositionType()); } +TEST_F(LayerStateTest, getHdrSdrRatio) { + OutputLayerCompositionState outputLayerCompositionState; + LayerFECompositionState layerFECompositionState; + layerFECompositionState.currentHdrSdrRatio = 2.f; + setupMocksForLayer(mOutputLayer, *mLayerFE, outputLayerCompositionState, + layerFECompositionState); + mLayerState = std::make_unique<LayerState>(&mOutputLayer); + EXPECT_EQ(2.f, mLayerState->getHdrSdrRatio()); +} + TEST_F(LayerStateTest, updateCompositionType) { OutputLayerCompositionState outputLayerCompositionState; LayerFECompositionState layerFECompositionState; @@ -1038,6 +1048,47 @@ TEST_F(LayerStateTest, compareCachingHint) { EXPECT_TRUE(otherLayerState->compare(*mLayerState)); } +TEST_F(LayerStateTest, updateDimmingEnabled) { + OutputLayerCompositionState outputLayerCompositionState; + LayerFECompositionState layerFECompositionState; + layerFECompositionState.dimmingEnabled = true; + setupMocksForLayer(mOutputLayer, *mLayerFE, outputLayerCompositionState, + layerFECompositionState); + mLayerState = std::make_unique<LayerState>(&mOutputLayer); + EXPECT_TRUE(mLayerState->isDimmingEnabled()); + + mock::OutputLayer newOutputLayer; + sp<mock::LayerFE> newLayerFE = sp<mock::LayerFE>::make(); + LayerFECompositionState layerFECompositionStateTwo; + layerFECompositionStateTwo.dimmingEnabled = false; + setupMocksForLayer(newOutputLayer, *newLayerFE, outputLayerCompositionState, + layerFECompositionStateTwo); + ftl::Flags<LayerStateField> updates = mLayerState->update(&newOutputLayer); + EXPECT_EQ(ftl::Flags<LayerStateField>(LayerStateField::DimmingEnabled), updates); + EXPECT_FALSE(mLayerState->isDimmingEnabled()); +} + +TEST_F(LayerStateTest, compareDimmingEnabled) { + OutputLayerCompositionState outputLayerCompositionState; + LayerFECompositionState layerFECompositionState; + layerFECompositionState.dimmingEnabled = true; + setupMocksForLayer(mOutputLayer, *mLayerFE, outputLayerCompositionState, + layerFECompositionState); + mLayerState = std::make_unique<LayerState>(&mOutputLayer); + mock::OutputLayer newOutputLayer; + sp<mock::LayerFE> newLayerFE = sp<mock::LayerFE>::make(); + LayerFECompositionState layerFECompositionStateTwo; + layerFECompositionStateTwo.dimmingEnabled = false; + setupMocksForLayer(newOutputLayer, *newLayerFE, outputLayerCompositionState, + layerFECompositionStateTwo); + auto otherLayerState = std::make_unique<LayerState>(&newOutputLayer); + + verifyNonUniqueDifferingFields(*mLayerState, *otherLayerState, LayerStateField::DimmingEnabled); + + EXPECT_TRUE(mLayerState->compare(*otherLayerState)); + EXPECT_TRUE(otherLayerState->compare(*mLayerState)); +} + TEST_F(LayerStateTest, dumpDoesNotCrash) { OutputLayerCompositionState outputLayerCompositionState; LayerFECompositionState layerFECompositionState; |