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
diff --git a/libs/ui/GraphicBufferAllocator.cpp b/libs/ui/GraphicBufferAllocator.cpp
index 9847a5f..fa46ab7 100644
--- a/libs/ui/GraphicBufferAllocator.cpp
+++ b/libs/ui/GraphicBufferAllocator.cpp
@@ -63,7 +63,7 @@
const size_t c = list.size();
for (size_t i=0 ; i<c ; i++) {
const alloc_rec_t& rec(list.valueAt(i));
- snprintf(buffer, SIZE, "%10p: %7.2f KiB | %4u (%4u) x %4u | %8X | 0x%08x\n",
+ snprintf(buffer, SIZE, "%10p: %7.2f KiB | %4u (%4u) x %4u | %2d | 0x%08x\n",
list.keyAt(i), rec.size/1024.0f,
rec.w, rec.s, rec.h, rec.format, rec.usage);
result.append(buffer);
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index e3cf27e..c9dcef3 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -120,8 +120,7 @@
sp<LayerBaseClient::Surface> Layer::createSurface() const
{
- sp<Surface> sur(new SurfaceLayer(mFlinger, const_cast<Layer *>(this)));
- return sur;
+ return mSurface;
}
status_t Layer::ditch()
@@ -131,6 +130,10 @@
// 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 @@
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 4a10e73..7bb207a 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -211,6 +211,7 @@
ClientRef mUserClientRef;
// constants
+ sp<Surface> mSurface;
PixelFormat mFormat;
const GLExtensions& mGLExtensions;
bool mNeedsBlending;
diff --git a/services/surfaceflinger/LayerBase.cpp b/services/surfaceflinger/LayerBase.cpp
index 0f02c6d..21c36e18 100644
--- a/services/surfaceflinger/LayerBase.cpp
+++ b/services/surfaceflinger/LayerBase.cpp
@@ -515,21 +515,13 @@
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>& 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<Surface> 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<IBinder> LayerBaseClient::getSurfaceBinder() const {
- return mClientSurfaceBinder;
-}
-
sp<LayerBaseClient::Surface> LayerBaseClient::createSurface() const
{
return new Surface(mFlinger, mIdentity,
@@ -580,12 +566,6 @@
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 332e5dd..afc5ec8 100644
--- a/services/surfaceflinger/LayerBase.h
+++ b/services/surfaceflinger/LayerBase.h
@@ -206,7 +206,6 @@
/** 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 @@
virtual ~LayerBaseClient();
sp<Surface> getSurface();
- wp<IBinder> getSurfaceBinder() const;
virtual sp<Surface> createSurface() const;
virtual sp<LayerBaseClient> getLayerBaseClient() const {
return const_cast<LayerBaseClient*>(this); }
@@ -327,12 +325,10 @@
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<IBinder> mClientSurfaceBinder;
+ mutable wp<Surface> mClientSurface;
const wp<Client> mClientRef;
// only read
const uint32_t mIdentity;
diff --git a/services/surfaceflinger/LayerDim.cpp b/services/surfaceflinger/LayerDim.cpp
index 11f8feb..80cc52c 100644
--- a/services/surfaceflinger/LayerDim.cpp
+++ b/services/surfaceflinger/LayerDim.cpp
@@ -67,14 +67,8 @@
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 e2e686f..c08e2c9 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1054,7 +1054,7 @@
{
sp<LayerBaseClient> 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::purgatorizeLayer_l(const sp<LayerBase>& 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 @@
* 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 @@
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<count ; i++) {
const sp<LayerBase>& layer(currentLayers[i]);
layer->dump(result, buffer, SIZE);
@@ -1486,24 +1464,6 @@
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<purgatorySize ; i++) {
- const sp<LayerBase>& 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 @@
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 a023347..df1ca48 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -370,7 +370,6 @@
volatile int32_t mTransactionFlags;
volatile int32_t mTransactionCount;
Condition mTransactionCV;
- SortedVector< sp<LayerBase> > mLayerPurgatory;
bool mResizeTransationPending;
// protected by mStateLock (but we could use another lock)