From e7d3ee9d81de13e992c7d063ca472d480956b0c6 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Thu, 16 Jun 2011 17:15:51 -0700 Subject: Revert "merge various SF fixes from gingerbread to honeycomb-mr2" (DO NOT MERGE) Also revert all dependent changes: This reverts commit 8e18668d14adf601cbe5973030c310ec23d88461. This reverts commit 69b4587bfbb3e98f793959d9123340360fa233a2. This reverts commit a9c9a4baf24700e8817d47d8ea8da1742caea0b5. This reverts commit 2c0042b666a969091c931614f2fc0dce2f1cfac8. This reverts commit f6c8206735e7e078461e5f2aef6e1a1446fdd075. This reverts commit 24855c09173a6caaec7dcedd0c2d7ce15121d39b. Change-Id: I33e699640f3f59e42fa03c99a9a1b7af0d27d4d8 --- include/utils/RefBase.h | 2 +- services/surfaceflinger/Layer.cpp | 12 +++ services/surfaceflinger/Layer.h | 1 + services/surfaceflinger/LayerBase.cpp | 5 +- services/surfaceflinger/LayerBase.h | 7 +- services/surfaceflinger/SurfaceFlinger.cpp | 132 ++++++++++++++++++----------- services/surfaceflinger/SurfaceFlinger.h | 20 ++--- 7 files changed, 111 insertions(+), 68 deletions(-) diff --git a/include/utils/RefBase.h b/include/utils/RefBase.h index b31e993..f355087 100644 --- a/include/utils/RefBase.h +++ b/include/utils/RefBase.h @@ -120,7 +120,7 @@ public: protected: RefBase(); virtual ~RefBase(); - + //! Flags for extendObjectLifetime() enum { OBJECT_LIFETIME_WEAK = 0x0001, diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 731d82b..517c335 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -145,6 +145,18 @@ sp Layer::createSurface() const return sur; } +status_t Layer::ditch() +{ + // NOTE: Called from the main UI thread + + // the layer is not on screen anymore. free as much resources as possible + mFreezeLock.clear(); + + Mutex::Autolock _l(mLock); + mWidth = mHeight = 0; + return NO_ERROR; +} + status_t Layer::setBuffers( uint32_t w, uint32_t h, PixelFormat format, uint32_t flags) { diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index 4c92278..128f93d 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -82,6 +82,7 @@ public: virtual bool isSecure() const { return mSecure; } virtual bool isProtected() const; virtual sp createSurface() const; + virtual status_t ditch(); virtual void onRemoved(); // only for debugging diff --git a/services/surfaceflinger/LayerBase.cpp b/services/surfaceflinger/LayerBase.cpp index 022f251..6025ed4 100644 --- a/services/surfaceflinger/LayerBase.cpp +++ b/services/surfaceflinger/LayerBase.cpp @@ -616,7 +616,10 @@ LayerBaseClient::Surface::~Surface() */ // destroy client resources - mFlinger->destroySurface(mOwner); + sp layer = getOwner(); + if (layer != 0) { + mFlinger->destroySurface(layer); + } } sp LayerBaseClient::Surface::getOwner() const { diff --git a/services/surfaceflinger/LayerBase.h b/services/surfaceflinger/LayerBase.h index 6c49a19..7162e47 100644 --- a/services/surfaceflinger/LayerBase.h +++ b/services/surfaceflinger/LayerBase.h @@ -202,6 +202,10 @@ public: */ virtual bool isProtected() const { return false; } + /** Called from the main thread, when the surface is removed from the + * draw list */ + virtual status_t ditch() { return NO_ERROR; } + /** called with the state lock when the surface is removed from the * current list */ virtual void onRemoved() { }; @@ -267,8 +271,7 @@ protected: volatile int32_t mInvalidate; -public: - // called from class SurfaceFlinger +protected: virtual ~LayerBase(); private: diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 33cb9a4..a9fa1ef 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -395,7 +395,7 @@ bool SurfaceFlinger::threadLoop() if (LIKELY(mTransactionCount == 0)) { // if we're in a global transaction, don't do anything. const uint32_t mask = eTransactionNeeded | eTraversalNeeded; - uint32_t transactionFlags = peekTransactionFlags(mask); + uint32_t transactionFlags = getTransactionFlags(mask); if (LIKELY(transactionFlags)) { handleTransaction(transactionFlags); } @@ -480,26 +480,39 @@ void SurfaceFlinger::handleConsoleEvents() void SurfaceFlinger::handleTransaction(uint32_t transactionFlags) { - Mutex::Autolock _l(mStateLock); - const nsecs_t now = systemTime(); - mDebugInTransaction = now; + Vector< sp > ditchedLayers; - // Here we're guaranteed that some transaction flags are set - // so we can call handleTransactionLocked() unconditionally. - // We call getTransactionFlags(), which will also clear the flags, - // with mStateLock held to guarantee that mCurrentState won't change - // until the transaction is committed. + /* + * Perform and commit the transaction + */ - const uint32_t mask = eTransactionNeeded | eTraversalNeeded; - transactionFlags = getTransactionFlags(mask); - handleTransactionLocked(transactionFlags); + { // scope for the lock + Mutex::Autolock _l(mStateLock); + const nsecs_t now = systemTime(); + mDebugInTransaction = now; + handleTransactionLocked(transactionFlags, ditchedLayers); + mLastTransactionTime = systemTime() - now; + mDebugInTransaction = 0; + invalidateHwcGeometry(); + // here the transaction has been committed + } - mLastTransactionTime = systemTime() - now; - mDebugInTransaction = 0; - // here the transaction has been committed + /* + * Clean-up all layers that went away + * (do this without the lock held) + */ + + const size_t count = ditchedLayers.size(); + for (size_t i=0 ; iditch(); + } + } } -void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags) +void SurfaceFlinger::handleTransactionLocked( + uint32_t transactionFlags, Vector< sp >& ditchedLayers) { const LayerVector& currentLayers(mCurrentState.layersSortedByZ); const size_t count = currentLayers.size(); @@ -571,6 +584,7 @@ void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags) const sp& layer(previousLayers[i]); if (currentLayers.indexOf( layer ) < 0) { // this layer is not visible anymore + ditchedLayers.add(layer); mDirtyRegionRemovedLayer.orSelf(layer->visibleRegionScreen); } } @@ -1082,15 +1096,15 @@ status_t SurfaceFlinger::addLayer_l(const sp& layer) ssize_t SurfaceFlinger::addClientLayer(const sp& client, const sp& lbc) { - // attach this layer to the client - size_t name = client->attachLayer(lbc); - Mutex::Autolock _l(mStateLock); + // attach this layer to the client + ssize_t name = client->attachLayer(lbc); + // add this layer to the current state list addLayer_l(lbc); - return ssize_t(name); + return name; } status_t SurfaceFlinger::removeLayer(const sp& layer) @@ -1141,11 +1155,6 @@ status_t SurfaceFlinger::invalidateLayerVisibility(const sp& layer) return NO_ERROR; } -uint32_t SurfaceFlinger::peekTransactionFlags(uint32_t flags) -{ - return android_atomic_release_load(&mTransactionFlags); -} - uint32_t SurfaceFlinger::getTransactionFlags(uint32_t flags) { return android_atomic_and(~flags, &mTransactionFlags) & flags; @@ -1353,26 +1362,51 @@ status_t SurfaceFlinger::removeSurface(const sp& client, SurfaceID sid) return err; } -status_t SurfaceFlinger::destroySurface(const wp& layer) +status_t SurfaceFlinger::destroySurface(const sp& layer) { // called by ~ISurface() when all references are gone - status_t err = NO_ERROR; - sp l(layer.promote()); - if (l != NULL) { - Mutex::Autolock _l(mStateLock); - err = removeLayer_l(l); - if (err == NAME_NOT_FOUND) { - // The surface wasn't in the current list, which means it was - // removed already, which means it is in the purgatory, - // and need to be removed from there. - ssize_t idx = mLayerPurgatory.remove(l); - LOGE_IF(idx < 0, - "layer=%p is not in the purgatory list", l.get()); - } - LOGE_IF(err<0 && err != NAME_NOT_FOUND, - "error removing layer=%p (%s)", l.get(), strerror(-err)); - } - return err; + + class MessageDestroySurface : public MessageBase { + SurfaceFlinger* flinger; + sp layer; + public: + MessageDestroySurface( + SurfaceFlinger* flinger, const sp& layer) + : flinger(flinger), layer(layer) { } + virtual bool handler() { + sp l(layer); + layer.clear(); // clear it outside of the lock; + Mutex::Autolock _l(flinger->mStateLock); + /* + * remove the layer from the current list -- chances are that it's + * not in the list anyway, because it should have been removed + * already upon request of the client (eg: window manager). + * However, a buggy client could have not done that. + * Since we know we don't have any more clients, we don't need + * to use the purgatory. + */ + status_t err = flinger->removeLayer_l(l); + if (err == NAME_NOT_FOUND) { + // The surface wasn't in the current list, which means it was + // removed already, which means it is in the purgatory, + // and need to be removed from there. + // This needs to happen from the main thread since its dtor + // must run from there (b/c of OpenGL ES). Additionally, we + // can't really acquire our internal lock from + // destroySurface() -- see postMessage() below. + ssize_t idx = flinger->mLayerPurgatory.remove(l); + LOGE_IF(idx < 0, + "layer=%p is not in the purgatory list", l.get()); + } + + LOGE_IF(err<0 && err != NAME_NOT_FOUND, + "error removing layer=%p (%s)", l.get(), strerror(-err)); + return true; + } + }; + + postMessageAsync( new MessageDestroySurface(this, layer) ); + return NO_ERROR; } status_t SurfaceFlinger::setClientState( @@ -2347,17 +2381,15 @@ status_t Client::initCheck() const { return NO_ERROR; } -size_t Client::attachLayer(const sp& layer) +ssize_t Client::attachLayer(const sp& layer) { - Mutex::Autolock _l(mLock); - size_t name = mNameGenerator++; + int32_t name = android_atomic_inc(&mNameGenerator); mLayers.add(name, layer); return name; } void Client::detachLayer(const LayerBaseClient* layer) { - Mutex::Autolock _l(mLock); // we do a linear search here, because this doesn't happen often const size_t count = mLayers.size(); for (size_t i=0 ; i Client::getLayerUser(int32_t i) const -{ - Mutex::Autolock _l(mLock); +sp Client::getLayerUser(int32_t i) const { sp lbc; - wp layer(mLayers.valueFor(i)); + const wp& layer(mLayers.valueFor(i)); if (layer != 0) { lbc = layer.promote(); LOGE_IF(lbc==0, "getLayerUser(name=%d) is dead", int(i)); diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 31c20d7..9566819 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -65,7 +65,7 @@ public: status_t initCheck() const; // protected by SurfaceFlinger::mStateLock - size_t attachLayer(const sp& layer); + ssize_t attachLayer(const sp& layer); void detachLayer(const LayerBaseClient* layer); sp getLayerUser(int32_t i) const; @@ -81,15 +81,9 @@ private: virtual status_t destroySurface(SurfaceID surfaceId); virtual status_t setState(int32_t count, const layer_state_t* states); - // constant - sp mFlinger; - - // protected by mLock DefaultKeyedVector< size_t, wp > mLayers; - size_t mNameGenerator; - - // thread-safe - mutable Mutex mLock; + sp mFlinger; + int32_t mNameGenerator; }; class UserClient : public BnSurfaceComposerClient @@ -260,7 +254,7 @@ private: uint32_t w, uint32_t h, uint32_t flags); status_t removeSurface(const sp& client, SurfaceID sid); - status_t destroySurface(const wp& layer); + status_t destroySurface(const sp& layer); status_t setClientState(const sp& client, int32_t count, const layer_state_t* states); @@ -305,7 +299,9 @@ public: // hack to work around gcc 4.0.3 bug private: void handleConsoleEvents(); void handleTransaction(uint32_t transactionFlags); - void handleTransactionLocked(uint32_t transactionFlags); + void handleTransactionLocked( + uint32_t transactionFlags, + Vector< sp >& ditchedLayers); void computeVisibleRegions( LayerVector& currentLayers, @@ -328,7 +324,6 @@ private: status_t purgatorizeLayer_l(const sp& layer); uint32_t getTransactionFlags(uint32_t flags); - uint32_t peekTransactionFlags(uint32_t flags); uint32_t setTransactionFlags(uint32_t flags); void commitTransaction(); @@ -427,7 +422,6 @@ private: // these are thread safe mutable Barrier mReadyToRunBarrier; - // atomic variables enum { eConsoleReleased = 1, -- cgit v1.1