From f7662afb76dfafebdd449c5e3e168f050da850a0 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Wed, 9 Mar 2011 16:13:31 -0800 Subject: [PATCH] revert the surface purgatory list and dependent changes. 6d0f6cb Revert "Fix [3513017] in lockscreen but showing empty launcher (live wallpaper) only" 6154412 Revert "partially fix [3306150] HTML5 video with H/W acceleration blackout (DO NOT MERGE)" 37c2a37 fix [3408713] Dialog window invisible sometimes It looks like there is a surface leak, it's unclear where it is. Without those reverts, this would cause a leak of the associated buffers which is far more problematic. this change might hide the surface leak. Bug: 4078032 Change-Id: Iedcda3ffcdd2f69d41047b5c3134c1e867ff90d7 --- libs/ui/GraphicBufferAllocator.cpp | 2 +- services/surfaceflinger/Layer.cpp | 8 +++- services/surfaceflinger/Layer.h | 1 + services/surfaceflinger/LayerBase.cpp | 32 +++------------ services/surfaceflinger/LayerBase.h | 6 +-- services/surfaceflinger/LayerDim.cpp | 10 +---- services/surfaceflinger/SurfaceFlinger.cpp | 47 +--------------------- services/surfaceflinger/SurfaceFlinger.h | 1 - 8 files changed, 19 insertions(+), 88 deletions(-) diff --git a/libs/ui/GraphicBufferAllocator.cpp b/libs/ui/GraphicBufferAllocator.cpp index 9847a5fab..fa46ab7b3 100644 --- a/libs/ui/GraphicBufferAllocator.cpp +++ b/libs/ui/GraphicBufferAllocator.cpp @@ -63,7 +63,7 @@ void GraphicBufferAllocator::dump(String8& result) const const size_t c = list.size(); for (size_t i=0 ; i Layer::createSurface() const { - sp sur(new SurfaceLayer(mFlinger, const_cast(this))); - return sur; + return mSurface; } status_t Layer::ditch() @@ -131,6 +130,10 @@ status_t Layer::ditch() // the layer is not on screen anymore. free as much resources as possible mFreezeLock.clear(); + EGLDisplay dpy(mFlinger->graphicPlane(0).getEGLDisplay()); + mBufferManager.destroy(dpy); + mSurface.clear(); + Mutex::Autolock _l(mLock); mWidth = mHeight = 0; return NO_ERROR; @@ -175,6 +178,7 @@ status_t Layer::setBuffers( uint32_t w, uint32_t h, int layerRedsize = info.getSize(PixelFormatInfo::INDEX_RED); mNeedsDithering = layerRedsize > displayRedSize; + mSurface = new SurfaceLayer(mFlinger, this); return NO_ERROR; } diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index 4a10e7394..7bb207a1f 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -211,6 +211,7 @@ private: ClientRef mUserClientRef; // constants + sp mSurface; PixelFormat mFormat; const GLExtensions& mGLExtensions; bool mNeedsBlending; diff --git a/services/surfaceflinger/LayerBase.cpp b/services/surfaceflinger/LayerBase.cpp index 0f02c6d3d..21c36e184 100644 --- a/services/surfaceflinger/LayerBase.cpp +++ b/services/surfaceflinger/LayerBase.cpp @@ -515,21 +515,13 @@ void LayerBase::dump(String8& result, char* buffer, size_t SIZE) const result.append(buffer); } -void LayerBase::shortDump(String8& result, char* scratch, size_t size) const -{ - LayerBase::dump(result, scratch, size); -} - - // --------------------------------------------------------------------------- int32_t LayerBaseClient::sIdentity = 1; LayerBaseClient::LayerBaseClient(SurfaceFlinger* flinger, DisplayID display, const sp& client) - : LayerBase(flinger, display), - mHasSurface(false), - mClientRef(client), + : LayerBase(flinger, display), mClientRef(client), mIdentity(uint32_t(android_atomic_inc(&sIdentity))) { } @@ -546,20 +538,14 @@ sp LayerBaseClient::getSurface() { sp s; Mutex::Autolock _l(mLock); - - LOG_ALWAYS_FATAL_IF(mHasSurface, - "LayerBaseClient::getSurface() has already been called"); - - mHasSurface = true; - s = createSurface(); - mClientSurfaceBinder = s->asBinder(); + s = mClientSurface.promote(); + if (s == 0) { + s = createSurface(); + mClientSurface = s; + } return s; } -wp LayerBaseClient::getSurfaceBinder() const { - return mClientSurfaceBinder; -} - sp LayerBaseClient::createSurface() const { return new Surface(mFlinger, mIdentity, @@ -580,12 +566,6 @@ void LayerBaseClient::dump(String8& result, char* buffer, size_t SIZE) const result.append(buffer); } - -void LayerBaseClient::shortDump(String8& result, char* scratch, size_t size) const -{ - LayerBaseClient::dump(result, scratch, size); -} - // --------------------------------------------------------------------------- LayerBaseClient::Surface::Surface( diff --git a/services/surfaceflinger/LayerBase.h b/services/surfaceflinger/LayerBase.h index 332e5dd96..afc5ec8d5 100644 --- a/services/surfaceflinger/LayerBase.h +++ b/services/surfaceflinger/LayerBase.h @@ -206,7 +206,6 @@ public: /** always call base class first */ virtual void dump(String8& result, char* scratch, size_t size) const; - virtual void shortDump(String8& result, char* scratch, size_t size) const; enum { // flags for doTransaction() @@ -285,7 +284,6 @@ public: virtual ~LayerBaseClient(); sp getSurface(); - wp getSurfaceBinder() const; virtual sp createSurface() const; virtual sp getLayerBaseClient() const { return const_cast(this); } @@ -327,12 +325,10 @@ public: protected: virtual void dump(String8& result, char* scratch, size_t size) const; - virtual void shortDump(String8& result, char* scratch, size_t size) const; private: mutable Mutex mLock; - mutable bool mHasSurface; - wp mClientSurfaceBinder; + mutable wp mClientSurface; const wp mClientRef; // only read const uint32_t mIdentity; diff --git a/services/surfaceflinger/LayerDim.cpp b/services/surfaceflinger/LayerDim.cpp index 11f8feb1f..80cc52c42 100644 --- a/services/surfaceflinger/LayerDim.cpp +++ b/services/surfaceflinger/LayerDim.cpp @@ -67,14 +67,8 @@ void LayerDim::onDraw(const Region& clip) const const GLfloat alpha = s.alpha/255.0f; const uint32_t fbHeight = hw.getHeight(); glDisable(GL_DITHER); - - if (s.alpha == 0xFF) { - glDisable(GL_BLEND); - } else { - glEnable(GL_BLEND); - glBlendFunc(GL_ONE, GL_ONE_MINUS_SRC_ALPHA); - } - + glEnable(GL_BLEND); + glBlendFunc(GL_ONE, GL_ONE_MINUS_SRC_ALPHA); glColor4f(0, 0, 0, alpha); #if defined(GL_OES_EGL_image_external) diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index e2e686fa4..c08e2c941 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -1054,7 +1054,7 @@ status_t SurfaceFlinger::removeLayer_l(const sp& layerBase) { sp lbc(layerBase->getLayerBaseClient()); if (lbc != 0) { - mLayerMap.removeItem( lbc->getSurfaceBinder() ); + mLayerMap.removeItem( lbc->getSurface()->asBinder() ); } ssize_t index = mCurrentState.layersSortedByZ.remove(layerBase); if (index >= 0) { @@ -1066,12 +1066,8 @@ status_t SurfaceFlinger::removeLayer_l(const sp& layerBase) status_t SurfaceFlinger::purgatorizeLayer_l(const sp& layerBase) { - // First add the layer to the purgatory list, which makes sure it won't - // go away, then remove it from the main list (through a transaction). + // remove the layer from the main list (through a transaction). ssize_t err = removeLayer_l(layerBase); - if (err >= 0) { - mLayerPurgatory.add(layerBase); - } layerBase->onRemoved(); @@ -1342,19 +1338,6 @@ status_t SurfaceFlinger::destroySurface(const sp& layer) * 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; @@ -1470,13 +1453,8 @@ status_t SurfaceFlinger::dump(int fd, const Vector& args) result.append(buffer); } - /* - * Dump the visible layer list - */ const LayerVector& currentLayers = mCurrentState.layersSortedByZ; const size_t count = currentLayers.size(); - snprintf(buffer, SIZE, "Visible layers (count = %d)\n", count); - result.append(buffer); for (size_t i=0 ; i& layer(currentLayers[i]); layer->dump(result, buffer, SIZE); @@ -1486,24 +1464,6 @@ status_t SurfaceFlinger::dump(int fd, const Vector& args) layer->visibleRegionScreen.dump(result, "visibleRegionScreen"); } - /* - * Dump the layers in the purgatory - */ - - const size_t purgatorySize = mLayerPurgatory.size(); - snprintf(buffer, SIZE, "Purgatory state (%d entries)\n", purgatorySize); - result.append(buffer); - for (size_t i=0 ; i& layer(mLayerPurgatory.itemAt(i)); - layer->shortDump(result, buffer, SIZE); - } - - /* - * Dump SurfaceFlinger global state - */ - - snprintf(buffer, SIZE, "SurfaceFlinger global state\n"); - result.append(buffer); mWormholeRegion.dump(result, "WormholeRegion"); const DisplayHardware& hw(graphicPlane(0).displayHardware()); snprintf(buffer, SIZE, @@ -1529,9 +1489,6 @@ status_t SurfaceFlinger::dump(int fd, const Vector& args) result.append(buffer); } - /* - * Dump gralloc state - */ const GraphicBufferAllocator& alloc(GraphicBufferAllocator::get()); alloc.dump(result); diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index a023347f0..df1ca4892 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -370,7 +370,6 @@ private: volatile int32_t mTransactionFlags; volatile int32_t mTransactionCount; Condition mTransactionCV; - SortedVector< sp > mLayerPurgatory; bool mResizeTransationPending; // protected by mStateLock (but we could use another lock)