From 7143316af216fa92c31a60d4407b707637382da1 Mon Sep 17 00:00:00 2001 From: Dan Stoza Date: Tue, 4 Feb 2014 16:22:36 -0800 Subject: [PATCH] Fix virtual display nesting This fixes the cycling rendering loop caused by nesting virtual displays by preventing them from recomposing if their contents haven't changed. Bug: 12101046 Change-Id: I600365c0fd5d3ad93e04295d26cf9de177ffc79b --- services/surfaceflinger/DisplayDevice.cpp | 4 +-- services/surfaceflinger/DisplayDevice.h | 4 ++- .../DisplayHardware/DisplaySurface.h | 4 ++- .../DisplayHardware/FramebufferSurface.cpp | 2 +- .../DisplayHardware/FramebufferSurface.h | 2 +- .../DisplayHardware/VirtualDisplaySurface.cpp | 35 ++++++++++++------- .../DisplayHardware/VirtualDisplaySurface.h | 4 ++- services/surfaceflinger/SurfaceFlinger.cpp | 13 ++++++- 8 files changed, 48 insertions(+), 20 deletions(-) diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp index 0d0f98d89..e5ecf0778 100644 --- a/services/surfaceflinger/DisplayDevice.cpp +++ b/services/surfaceflinger/DisplayDevice.cpp @@ -199,8 +199,8 @@ void DisplayDevice::flip(const Region& dirty) const mPageFlipCount++; } -status_t DisplayDevice::beginFrame() const { - return mDisplaySurface->beginFrame(); +status_t DisplayDevice::beginFrame(bool mustRecompose) const { + return mDisplaySurface->beginFrame(mustRecompose); } status_t DisplayDevice::prepareFrame(const HWComposer& hwc) const { diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h index 01a9d2efc..f750c6c82 100644 --- a/services/surfaceflinger/DisplayDevice.h +++ b/services/surfaceflinger/DisplayDevice.h @@ -124,7 +124,9 @@ public: int32_t getHwcDisplayId() const { return mHwcDisplayId; } const wp& getDisplayToken() const { return mDisplayToken; } - status_t beginFrame() const; + // We pass in mustRecompose so we can keep VirtualDisplaySurface's state + // machine happy without actually queueing a buffer if nothing has changed + status_t beginFrame(bool mustRecompose) const; status_t prepareFrame(const HWComposer& hwc) const; void swapBuffers(HWComposer& hwc) const; diff --git a/services/surfaceflinger/DisplayHardware/DisplaySurface.h b/services/surfaceflinger/DisplayHardware/DisplaySurface.h index 48bf3f286..1db3eb8a8 100644 --- a/services/surfaceflinger/DisplayHardware/DisplaySurface.h +++ b/services/surfaceflinger/DisplayHardware/DisplaySurface.h @@ -33,7 +33,9 @@ public: // beginFrame is called at the beginning of the composition loop, before // the configuration is known. The DisplaySurface should do anything it // needs to do to enable HWComposer to decide how to compose the frame. - virtual status_t beginFrame() = 0; + // We pass in mustRecompose so we can keep VirtualDisplaySurface's state + // machine happy without actually queueing a buffer if nothing has changed. + virtual status_t beginFrame(bool mustRecompose) = 0; // prepareFrame is called after the composition configuration is known but // before composition takes place. The DisplaySurface can use the diff --git a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp index 8c634ed64..0f34764bd 100644 --- a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp +++ b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp @@ -68,7 +68,7 @@ FramebufferSurface::FramebufferSurface(HWComposer& hwc, int disp, mConsumer->setDefaultMaxBufferCount(NUM_FRAMEBUFFER_SURFACE_BUFFERS); } -status_t FramebufferSurface::beginFrame() { +status_t FramebufferSurface::beginFrame(bool mustRecompose) { return NO_ERROR; } diff --git a/services/surfaceflinger/DisplayHardware/FramebufferSurface.h b/services/surfaceflinger/DisplayHardware/FramebufferSurface.h index 1d67446b9..ba72ce38b 100644 --- a/services/surfaceflinger/DisplayHardware/FramebufferSurface.h +++ b/services/surfaceflinger/DisplayHardware/FramebufferSurface.h @@ -39,7 +39,7 @@ class FramebufferSurface : public ConsumerBase, public: FramebufferSurface(HWComposer& hwc, int disp, const sp& consumer); - virtual status_t beginFrame(); + virtual status_t beginFrame(bool mustRecompose); virtual status_t prepareFrame(CompositionType compositionType); virtual status_t compositionComplete(); virtual status_t advanceFrame(); diff --git a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp index d7fef8c1e..a1820abc6 100644 --- a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp +++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp @@ -56,7 +56,8 @@ VirtualDisplaySurface::VirtualDisplaySurface(HWComposer& hwc, int32_t dispId, mOutputUsage(GRALLOC_USAGE_HW_COMPOSER), mProducerSlotSource(0), mDbgState(DBG_STATE_IDLE), - mDbgLastCompositionType(COMPOSITION_UNKNOWN) + mDbgLastCompositionType(COMPOSITION_UNKNOWN), + mMustRecompose(false) { mSource[SOURCE_SINK] = sink; mSource[SOURCE_SCRATCH] = bq; @@ -92,10 +93,12 @@ VirtualDisplaySurface::VirtualDisplaySurface(HWComposer& hwc, int32_t dispId, VirtualDisplaySurface::~VirtualDisplaySurface() { } -status_t VirtualDisplaySurface::beginFrame() { +status_t VirtualDisplaySurface::beginFrame(bool mustRecompose) { if (mDisplayId < 0) return NO_ERROR; + mMustRecompose = mustRecompose; + VDS_LOGW_IF(mDbgState != DBG_STATE_IDLE, "Unexpected beginFrame() in %s state", dbgStateStr()); mDbgState = DBG_STATE_BEGUN; @@ -228,16 +231,24 @@ void VirtualDisplaySurface::onFrameCommitted() { QueueBufferOutput qbo; sp outFence = mHwc.getLastRetireFence(mDisplayId); VDS_LOGV("onFrameCommitted: queue sink sslot=%d", sslot); - status_t result = mSource[SOURCE_SINK]->queueBuffer(sslot, - QueueBufferInput( - systemTime(), false /* isAutoTimestamp */, - Rect(mSinkBufferWidth, mSinkBufferHeight), - NATIVE_WINDOW_SCALING_MODE_FREEZE, 0 /* transform */, - true /* async*/, - outFence), - &qbo); - if (result == NO_ERROR) { - updateQueueBufferOutput(qbo); + if (mMustRecompose) { + status_t result = mSource[SOURCE_SINK]->queueBuffer(sslot, + QueueBufferInput( + systemTime(), false /* isAutoTimestamp */, + Rect(mSinkBufferWidth, mSinkBufferHeight), + NATIVE_WINDOW_SCALING_MODE_FREEZE, 0 /* transform */, + true /* async*/, + outFence), + &qbo); + if (result == NO_ERROR) { + updateQueueBufferOutput(qbo); + } + } else { + // If the surface hadn't actually been updated, then we only went + // through the motions of updating the display to keep our state + // machine happy. We cancel the buffer to avoid triggering another + // re-composition and causing an infinite loop. + mSource[SOURCE_SINK]->cancelBuffer(sslot, outFence); } } diff --git a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h index 1e85ac4bd..68999049d 100644 --- a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h +++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h @@ -79,7 +79,7 @@ public: // // DisplaySurface interface // - virtual status_t beginFrame(); + virtual status_t beginFrame(bool mustRecompose); virtual status_t prepareFrame(CompositionType compositionType); virtual status_t compositionComplete(); virtual status_t advanceFrame(); @@ -222,6 +222,8 @@ private: const char* dbgStateStr() const; static const char* dbgSourceStr(Source s); + + bool mMustRecompose; }; // --------------------------------------------------------------------------- diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 40ca8ebf9..9f687e260 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -882,7 +882,9 @@ void SurfaceFlinger::rebuildLayerStacks() { void SurfaceFlinger::setUpHWComposer() { for (size_t dpy=0 ; dpybeginFrame(); + bool mustRecompose = + !(mDisplays[dpy]->getDirtyRegion(false).isEmpty()); + mDisplays[dpy]->beginFrame(mustRecompose); } HWComposer& hwc(getHwComposer()); @@ -1500,6 +1502,15 @@ void SurfaceFlinger::invalidateHwcGeometry() void SurfaceFlinger::doDisplayComposition(const sp& hw, const Region& inDirtyRegion) { + // We only need to actually compose the display if: + // 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) + bool isHwcDisplay = hw->getHwcDisplayId() >= 0; + if (!isHwcDisplay && inDirtyRegion.isEmpty()) { + return; + } + Region dirtyRegion(inDirtyRegion); // compute the invalid region