From 167bdde88b9159c62c5c4d839e7615c94e7851dc Mon Sep 17 00:00:00 2001 From: Robert Carr Date: Wed, 28 Jul 2021 11:26:51 -0700 Subject: Prevent HDRLayerInfoListener traversal from running on every frame We should only have to update the HDRLayerInfoListener in a few scenarios. 1. A new listener appeared 2. A buffer changed colorspace 3. Surface geometry (visibility, parenting, display, etc) changed We protect the traversal behind these flags to avoid the runtime in the hot path of continuous buffer updates. A follow up fix could consider directly recursively traversing and early returning if !Layer->isVisible. Bug: 186200583 Test: Existing tests pass, simpleperf Change-Id: I549fdf6ea228344f79f6989b86b8e73a6065158a --- services/surfaceflinger/BufferStateLayer.cpp | 4 ++ services/surfaceflinger/SurfaceFlinger.cpp | 60 +++++++++++++++++----------- services/surfaceflinger/SurfaceFlinger.h | 8 ++++ 3 files changed, 49 insertions(+), 23 deletions(-) (limited to 'services/surfaceflinger') diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp index 032ff9a572..6253036c41 100644 --- a/services/surfaceflinger/BufferStateLayer.cpp +++ b/services/surfaceflinger/BufferStateLayer.cpp @@ -890,7 +890,11 @@ void BufferStateLayer::gatherBufferInfo() { mBufferInfo.mFenceTime = std::make_shared(s.acquireFence); mBufferInfo.mFence = s.acquireFence; mBufferInfo.mTransform = s.bufferTransform; + auto lastDataspace = mBufferInfo.mDataspace; mBufferInfo.mDataspace = translateDataspace(s.dataspace); + if (lastDataspace != mBufferInfo.mDataspace) { + mFlinger->mSomeDataspaceChanged = true; + } mBufferInfo.mCrop = computeBufferCrop(s); mBufferInfo.mScaleMode = NATIVE_WINDOW_SCALING_MODE_SCALE_TO_WINDOW; mBufferInfo.mSurfaceDamage = s.surfaceDamageRegion; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 72700162ba..76221bc05d 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -1643,6 +1643,9 @@ status_t SurfaceFlinger::addHdrLayerInfoListener(const sp& displayToken hdrInfoReporter = sp::make(); } hdrInfoReporter->addListener(listener); + + + mAddingHDRLayerInfoListener = true; return OK; } @@ -2143,6 +2146,8 @@ void SurfaceFlinger::onMessageRefresh() { mTracing.notify("bufferLatched"); } } + + mVisibleRegionsWereDirtyThisFrame = mVisibleRegionsDirty; // Cache value for use in post-comp mVisibleRegionsDirty = false; if (mCompositionEngine->needsAnotherUpdate()) { @@ -2287,6 +2292,7 @@ void SurfaceFlinger::postComposition() { std::vector, sp>> hdrInfoListeners; + bool haveNewListeners = false; { Mutex::Autolock lock(mStateLock); if (mFpsReporter) { @@ -2304,37 +2310,45 @@ void SurfaceFlinger::postComposition() { } } } + haveNewListeners = mAddingHDRLayerInfoListener; // grab this with state lock + mAddingHDRLayerInfoListener = false; } - for (auto& [compositionDisplay, listener] : hdrInfoListeners) { - HdrLayerInfoReporter::HdrLayerInfo info; - int32_t maxArea = 0; - mDrawingState.traverse([&, compositionDisplay = compositionDisplay](Layer* layer) { - const auto layerFe = layer->getCompositionEngineLayerFE(); - if (layer->isVisible() && compositionDisplay->belongsInOutput(layerFe)) { - const Dataspace transfer = + if (haveNewListeners || mSomeDataspaceChanged || mVisibleRegionsWereDirtyThisFrame) { + for (auto& [compositionDisplay, listener] : hdrInfoListeners) { + HdrLayerInfoReporter::HdrLayerInfo info; + int32_t maxArea = 0; + mDrawingState.traverse([&, compositionDisplay = compositionDisplay](Layer* layer) { + const auto layerFe = layer->getCompositionEngineLayerFE(); + if (layer->isVisible() && compositionDisplay->belongsInOutput(layerFe)) { + const Dataspace transfer = static_cast(layer->getDataSpace() & Dataspace::TRANSFER_MASK); - const bool isHdr = (transfer == Dataspace::TRANSFER_ST2084 || - transfer == Dataspace::TRANSFER_HLG); - - if (isHdr) { - const auto* outputLayer = compositionDisplay->getOutputLayerForLayer(layerFe); - if (outputLayer) { - info.numberOfHdrLayers++; - const auto displayFrame = outputLayer->getState().displayFrame; - const int32_t area = displayFrame.width() * displayFrame.height(); - if (area > maxArea) { - maxArea = area; - info.maxW = displayFrame.width(); - info.maxH = displayFrame.height(); + const bool isHdr = (transfer == Dataspace::TRANSFER_ST2084 || + transfer == Dataspace::TRANSFER_HLG); + + if (isHdr) { + const auto* outputLayer = + compositionDisplay->getOutputLayerForLayer(layerFe); + if (outputLayer) { + info.numberOfHdrLayers++; + const auto displayFrame = outputLayer->getState().displayFrame; + const int32_t area = displayFrame.width() * displayFrame.height(); + if (area > maxArea) { + maxArea = area; + info.maxW = displayFrame.width(); + info.maxH = displayFrame.height(); + } } } } - } - }); - listener->dispatchHdrLayerInfo(info); + }); + listener->dispatchHdrLayerInfo(info); + } } + mSomeDataspaceChanged = false; + mVisibleRegionsWereDirtyThisFrame = false; + mTransactionCallbackInvoker.addPresentFence(mPreviousPresentFences[0].fence); mTransactionCallbackInvoker.sendCallbacks(); diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index b9b26db260..59fd218bb4 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -1240,11 +1240,19 @@ private: State mDrawingState{LayerVector::StateSet::Drawing}; bool mVisibleRegionsDirty = false; + // VisibleRegions dirty is already cleared by postComp, but we need to track it to prevent + // extra work in the HDR layer info listener. + bool mVisibleRegionsWereDirtyThisFrame = false; + // Used to ensure we omit a callback when HDR layer info listener is newly added but the + // scene hasn't changed + bool mAddingHDRLayerInfoListener = false; + // Set during transaction application stage to track if the input info or children // for a layer has changed. // TODO: Also move visibleRegions over to a boolean system. bool mInputInfoChanged = false; bool mSomeChildrenChanged; + bool mSomeDataspaceChanged = false; bool mForceTransactionDisplayChange = false; bool mGeometryInvalid = false; -- cgit v1.2.3-59-g8ed1b