From a722b8165fc48c44cfebd2f3fb0ca038eb53baeb Mon Sep 17 00:00:00 2001 From: Chavi Weingarten Date: Wed, 17 Aug 2022 21:57:04 +0000 Subject: Don't queue buffer if nothing new to draw in Display The current logic only checks if dirty is not empty, but doesn't account for no content changing. Instead use the same logic that's in beginFrame to compute mustRecompute and use that when determining if the display should queue a buffer. This fixes an issue where a Displays first few frames could be black until the content is loaded. Bug: 239888440 Test: First frame from screenrecord is no longer black Change-Id: Ibb568302bf2d50db167a1f9ca76a0f37e2c65bf5 (cherry picked from commit 09fa1d6665aa6022d5c6c8f61fc81f950b476a2f) --- .../include/compositionengine/impl/Output.h | 5 ++++ .../CompositionEngine/src/Display.cpp | 2 +- .../CompositionEngine/src/Output.cpp | 16 +++++++----- .../CompositionEngine/tests/DisplayTest.cpp | 30 +++++++++++++++++++++- 4 files changed, 45 insertions(+), 8 deletions(-) diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h index df721cdc89..428c19fe02 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h @@ -152,6 +152,8 @@ protected: virtual const compositionengine::CompositionEngine& getCompositionEngine() const = 0; virtual void dumpState(std::string& out) const = 0; + bool mustRecompose() const; + private: void dirtyEntireOutput(); compositionengine::OutputLayer* findLayerRequestingBackgroundComposition() const; @@ -170,6 +172,9 @@ private: std::unique_ptr mClientCompositionRequestCache; std::unique_ptr mPlanner; std::unique_ptr mHwComposerAsyncWorker; + + // Whether the content must be recomposed this frame. + bool mMustRecompose = false; }; // This template factory function standardizes the implementation details of the diff --git a/services/surfaceflinger/CompositionEngine/src/Display.cpp b/services/surfaceflinger/CompositionEngine/src/Display.cpp index 1ec6449ed0..163d9a3748 100644 --- a/services/surfaceflinger/CompositionEngine/src/Display.cpp +++ b/services/surfaceflinger/CompositionEngine/src/Display.cpp @@ -426,7 +426,7 @@ void Display::finishFrame(const compositionengine::CompositionRefreshArgs& refre // 1) It is being handled by hardware composer, which may need this to // keep its virtual display state machine in sync, or // 2) There is work to be done (the dirty region isn't empty) - if (GpuVirtualDisplayId::tryCast(mId) && getDirtyRegion().isEmpty()) { + if (GpuVirtualDisplayId::tryCast(mId) && !mustRecompose()) { ALOGV("Skipping display composition"); return; } diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp index b724daa8ce..d0c5803d42 100644 --- a/services/surfaceflinger/CompositionEngine/src/Output.cpp +++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp @@ -977,17 +977,17 @@ void Output::beginFrame() { // frame, then nothing more until we get new layers. // - When a display is created with a private layer stack, we won't // emit any black frames until a layer is added to the layer stack. - const bool mustRecompose = dirty && !(empty && wasEmpty); + mMustRecompose = dirty && !(empty && wasEmpty); const char flagPrefix[] = {'-', '+'}; static_cast(flagPrefix); - ALOGV_IF("%s: %s composition for %s (%cdirty %cempty %cwasEmpty)", __FUNCTION__, - mustRecompose ? "doing" : "skipping", getName().c_str(), flagPrefix[dirty], - flagPrefix[empty], flagPrefix[wasEmpty]); + ALOGV("%s: %s composition for %s (%cdirty %cempty %cwasEmpty)", __func__, + mMustRecompose ? "doing" : "skipping", getName().c_str(), flagPrefix[dirty], + flagPrefix[empty], flagPrefix[wasEmpty]); - mRenderSurface->beginFrame(mustRecompose); + mRenderSurface->beginFrame(mMustRecompose); - if (mustRecompose) { + if (mMustRecompose) { outputState.lastCompositionHadVisibleLayers = !empty; } } @@ -1590,5 +1590,9 @@ void Output::finishPrepareFrame() { mRenderSurface->prepareFrame(state.usesClientComposition, state.usesDeviceComposition); } +bool Output::mustRecompose() const { + return mMustRecompose; +} + } // namespace impl } // namespace android::compositionengine diff --git a/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp b/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp index 344fea3331..5369642a1b 100644 --- a/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp @@ -971,16 +971,40 @@ TEST_F(DisplayFinishFrameTest, skipsCompositionIfNotDirty) { // We expect no calls to queueBuffer if composition was skipped. EXPECT_CALL(*renderSurface, queueBuffer(_)).Times(0); + EXPECT_CALL(*renderSurface, beginFrame(false)); gpuDisplay->editState().isEnabled = true; gpuDisplay->editState().usesClientComposition = false; gpuDisplay->editState().layerStackSpace.setContent(Rect(0, 0, 1, 1)); gpuDisplay->editState().dirtyRegion = Region::INVALID_REGION; + gpuDisplay->editState().lastCompositionHadVisibleLayers = true; + gpuDisplay->beginFrame(); gpuDisplay->finishFrame({}, std::move(mResultWithoutBuffer)); } -TEST_F(DisplayFinishFrameTest, performsCompositionIfDirty) { +TEST_F(DisplayFinishFrameTest, skipsCompositionIfEmpty) { + auto args = getDisplayCreationArgsForGpuVirtualDisplay(); + std::shared_ptr gpuDisplay = impl::createDisplay(mCompositionEngine, args); + + mock::RenderSurface* renderSurface = new StrictMock(); + gpuDisplay->setRenderSurfaceForTest(std::unique_ptr(renderSurface)); + + // We expect no calls to queueBuffer if composition was skipped. + EXPECT_CALL(*renderSurface, queueBuffer(_)).Times(0); + EXPECT_CALL(*renderSurface, beginFrame(false)); + + gpuDisplay->editState().isEnabled = true; + gpuDisplay->editState().usesClientComposition = false; + gpuDisplay->editState().layerStackSpace.setContent(Rect(0, 0, 1, 1)); + gpuDisplay->editState().dirtyRegion = Region(Rect(0, 0, 1, 1)); + gpuDisplay->editState().lastCompositionHadVisibleLayers = false; + + gpuDisplay->beginFrame(); + gpuDisplay->finishFrame({}, std::move(mResultWithoutBuffer)); +} + +TEST_F(DisplayFinishFrameTest, performsCompositionIfDirtyAndNotEmpty) { auto args = getDisplayCreationArgsForGpuVirtualDisplay(); std::shared_ptr gpuDisplay = impl::createDisplay(mCompositionEngine, args); @@ -989,11 +1013,15 @@ TEST_F(DisplayFinishFrameTest, performsCompositionIfDirty) { // We expect a single call to queueBuffer when composition is not skipped. EXPECT_CALL(*renderSurface, queueBuffer(_)).Times(1); + EXPECT_CALL(*renderSurface, beginFrame(true)); gpuDisplay->editState().isEnabled = true; gpuDisplay->editState().usesClientComposition = false; gpuDisplay->editState().layerStackSpace.setContent(Rect(0, 0, 1, 1)); gpuDisplay->editState().dirtyRegion = Region(Rect(0, 0, 1, 1)); + gpuDisplay->editState().lastCompositionHadVisibleLayers = true; + + gpuDisplay->beginFrame(); gpuDisplay->finishFrame({}, std::move(mResultWithBuffer)); } -- cgit v1.2.3-59-g8ed1b