more Surface lifetime management
Surfaces are now destroyed once all references from the clients are gone, but they go through a partial destruction as soon as the window manager requests it.
This last part is still buggy. see comments in SurfaceFlinger::destroySurface()
diff --git a/libs/surfaceflinger/BootAnimation.cpp b/libs/surfaceflinger/BootAnimation.cpp
index 519b112..ee36b67 100644
--- a/libs/surfaceflinger/BootAnimation.cpp
+++ b/libs/surfaceflinger/BootAnimation.cpp
@@ -179,6 +179,7 @@
eglMakeCurrent(mDisplay, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT);
eglDestroyContext(mDisplay, mContext);
eglDestroySurface(mDisplay, mSurface);
+ mFlingerSurface.clear();
mFlingerSurfaceControl.clear();
return r;
}
diff --git a/libs/surfaceflinger/Layer.cpp b/libs/surfaceflinger/Layer.cpp
index 980b78b..5fdec3f 100644
--- a/libs/surfaceflinger/Layer.cpp
+++ b/libs/surfaceflinger/Layer.cpp
@@ -87,6 +87,12 @@
return mSurface;
}
+status_t Layer::ditch()
+{
+ mSurface.clear();
+ return NO_ERROR;
+}
+
status_t Layer::setBuffers( Client* client,
uint32_t w, uint32_t h,
PixelFormat format, uint32_t flags)
@@ -119,7 +125,7 @@
return err;
}
}
- mSurface = new SurfaceLayer(clientIndex(), this);
+ mSurface = new SurfaceLayer(mFlinger, clientIndex(), this);
return NO_ERROR;
}
@@ -626,8 +632,13 @@
// ---------------------------------------------------------------------------
-Layer::SurfaceLayer::SurfaceLayer(SurfaceID id, const sp<Layer>& owner)
- : Surface(id, owner->getIdentity(), owner)
+Layer::SurfaceLayer::SurfaceLayer(const sp<SurfaceFlinger>& flinger,
+ SurfaceID id, const sp<Layer>& owner)
+ : Surface(flinger, id, owner->getIdentity(), owner)
+{
+}
+
+Layer::SurfaceLayer::~SurfaceLayer()
{
}
diff --git a/libs/surfaceflinger/Layer.h b/libs/surfaceflinger/Layer.h
index dc03d35..3f3953f 100644
--- a/libs/surfaceflinger/Layer.h
+++ b/libs/surfaceflinger/Layer.h
@@ -81,6 +81,7 @@
virtual bool needsBlending() const { return mNeedsBlending; }
virtual bool isSecure() const { return mSecure; }
virtual sp<Surface> createSurface() const;
+ virtual status_t ditch();
const LayerBitmap& getBuffer(int i) const { return mBuffers[i]; }
LayerBitmap& getBuffer(int i) { return mBuffers[i]; }
@@ -108,7 +109,9 @@
class SurfaceLayer : public LayerBaseClient::Surface
{
public:
- SurfaceLayer(SurfaceID id, const sp<Layer>& owner);
+ SurfaceLayer(const sp<SurfaceFlinger>& flinger,
+ SurfaceID id, const sp<Layer>& owner);
+ ~SurfaceLayer();
private:
virtual sp<SurfaceBuffer> getBuffer();
diff --git a/libs/surfaceflinger/LayerBase.cpp b/libs/surfaceflinger/LayerBase.cpp
index 6da0bf7..8609225 100644
--- a/libs/surfaceflinger/LayerBase.cpp
+++ b/libs/surfaceflinger/LayerBase.cpp
@@ -681,25 +681,32 @@
sp<LayerBaseClient::Surface> LayerBaseClient::createSurface() const
{
- return new Surface(clientIndex(), mIdentity,
+ return new Surface(mFlinger, clientIndex(), mIdentity,
const_cast<LayerBaseClient *>(this));
}
// ---------------------------------------------------------------------------
-LayerBaseClient::Surface::Surface(SurfaceID id, int identity,
+LayerBaseClient::Surface::Surface(
+ const sp<SurfaceFlinger>& flinger,
+ SurfaceID id, int identity,
const sp<LayerBaseClient>& owner)
- : mToken(id), mIdentity(identity), mOwner(owner)
-{
+ : mFlinger(flinger), mToken(id), mIdentity(identity), mOwner(owner)
+{
}
-LayerBaseClient::Surface::~Surface() {
- // TODO: We now have a point here were we can clean-up the
- // client's mess.
- // This is also where surface id should be recycled.
- //LOGD("Surface %d, heaps={%p, %p} destroyed",
- // mId, mHeap[0].get(), mHeap[1].get());
+LayerBaseClient::Surface::~Surface()
+{
+ /*
+ * This is a good place to clean-up all client resources
+ */
+
+ // destroy client resources
+ sp<LayerBaseClient> layer = getOwner();
+ if (layer != 0) {
+ mFlinger->destroySurface(layer);
+ }
}
sp<LayerBaseClient> LayerBaseClient::Surface::getOwner() const {
diff --git a/libs/surfaceflinger/LayerBase.h b/libs/surfaceflinger/LayerBase.h
index 2a4e133..ccff36d 100644
--- a/libs/surfaceflinger/LayerBase.h
+++ b/libs/surfaceflinger/LayerBase.h
@@ -203,14 +203,19 @@
/**
* isSecure - true if this surface is secure, that is if it prevents
- * screenshots or vns servers.
+ * screenshots or VNC servers.
*/
virtual bool isSecure() const { return false; }
- enum { // flags for doTransaction()
- eVisibleRegion = 0x00000002,
- eRestartTransaction = 0x00000008
- };
+ /** signal this layer that it's not needed any longer */
+ virtual status_t ditch() { return NO_ERROR; }
+
+
+
+ enum { // flags for doTransaction()
+ eVisibleRegion = 0x00000002,
+ eRestartTransaction = 0x00000008
+ };
inline const State& drawingState() const { return mDrawingState; }
@@ -244,7 +249,7 @@
GLint textureName, const GGLSurface& t,
GLuint& textureWidth, GLuint& textureHeight) const;
- SurfaceFlinger* mFlinger;
+ sp<SurfaceFlinger> mFlinger;
uint32_t mFlags;
// cached during validateVisibility()
@@ -316,7 +321,9 @@
ISurfaceFlingerClient::surface_data_t* params) const;
protected:
- Surface(SurfaceID id, int identity, const sp<LayerBaseClient>& owner);
+ Surface(const sp<SurfaceFlinger>& flinger,
+ SurfaceID id, int identity,
+ const sp<LayerBaseClient>& owner);
virtual ~Surface();
virtual status_t onTransact(uint32_t code, const Parcel& data,
Parcel* reply, uint32_t flags);
@@ -330,8 +337,9 @@
virtual sp<OverlayRef> createOverlay(uint32_t w, uint32_t h,
int32_t format);
- private:
+ protected:
friend class LayerBaseClient;
+ sp<SurfaceFlinger> mFlinger;
int32_t mToken;
int32_t mIdentity;
wp<LayerBaseClient> mOwner;
diff --git a/libs/surfaceflinger/LayerBuffer.cpp b/libs/surfaceflinger/LayerBuffer.cpp
index f22ff59..97d6f97 100644
--- a/libs/surfaceflinger/LayerBuffer.cpp
+++ b/libs/surfaceflinger/LayerBuffer.cpp
@@ -50,10 +50,21 @@
{
}
+void LayerBuffer::onFirstRef()
+{
+ mSurface = new SurfaceBuffer(mFlinger, clientIndex(),
+ const_cast<LayerBuffer *>(this));
+}
+
sp<LayerBaseClient::Surface> LayerBuffer::createSurface() const
{
- return new SurfaceBuffer(clientIndex(),
- const_cast<LayerBuffer *>(this));
+ return mSurface;
+}
+
+status_t LayerBuffer::ditch()
+{
+ mSurface.clear();
+ return NO_ERROR;
}
bool LayerBuffer::needsBlending() const {
@@ -167,9 +178,9 @@
// LayerBuffer::SurfaceBuffer
// ============================================================================
-LayerBuffer::SurfaceBuffer::SurfaceBuffer(SurfaceID id,
- const sp<LayerBuffer>& owner)
- : LayerBaseClient::Surface(id, owner->getIdentity(), owner)
+LayerBuffer::SurfaceBuffer::SurfaceBuffer(const sp<SurfaceFlinger>& flinger,
+ SurfaceID id, const sp<LayerBuffer>& owner)
+ : LayerBaseClient::Surface(flinger, id, owner->getIdentity(), owner)
{
}
diff --git a/libs/surfaceflinger/LayerBuffer.h b/libs/surfaceflinger/LayerBuffer.h
index 2d44121..5284409 100644
--- a/libs/surfaceflinger/LayerBuffer.h
+++ b/libs/surfaceflinger/LayerBuffer.h
@@ -50,7 +50,6 @@
LayerBuffer& mLayer;
};
-
public:
static const uint32_t typeInfo;
static const char* const typeID;
@@ -61,9 +60,11 @@
Client* client, int32_t i);
virtual ~LayerBuffer();
+ virtual void onFirstRef();
virtual bool needsBlending() const;
virtual sp<LayerBaseClient::Surface> createSurface() const;
+ virtual status_t ditch();
virtual void onDraw(const Region& clip) const;
virtual uint32_t doTransaction(uint32_t flags);
virtual void unlockPageFlip(const Transform& planeTransform, Region& outDirtyRegion);
@@ -177,7 +178,8 @@
class SurfaceBuffer : public LayerBaseClient::Surface
{
public:
- SurfaceBuffer(SurfaceID id, const sp<LayerBuffer>& owner);
+ SurfaceBuffer(const sp<SurfaceFlinger>& flinger,
+ SurfaceID id, const sp<LayerBuffer>& owner);
virtual ~SurfaceBuffer();
virtual status_t registerBuffers(const ISurface::BufferHeap& buffers);
@@ -191,12 +193,10 @@
return static_cast<LayerBuffer*>(Surface::getOwner().get());
}
};
-
- friend class SurfaceFlinger;
- sp<SurfaceBuffer> getClientSurface() const;
-
+
mutable Mutex mLock;
sp<Source> mSource;
+ sp<Surface> mSurface;
bool mInvalidate;
bool mNeedsBlending;
};
diff --git a/libs/surfaceflinger/SurfaceFlinger.cpp b/libs/surfaceflinger/SurfaceFlinger.cpp
index a63a282..6b2a103 100644
--- a/libs/surfaceflinger/SurfaceFlinger.cpp
+++ b/libs/surfaceflinger/SurfaceFlinger.cpp
@@ -1033,7 +1033,8 @@
ssize_t index = mCurrentState.layersSortedByZ.remove(layerBase);
if (index >= 0) {
mLayersRemoved = true;
- sp<LayerBaseClient> layer = LayerBase::dynamicCast< LayerBaseClient* >(layerBase.get());
+ sp<LayerBaseClient> layer =
+ LayerBase::dynamicCast< LayerBaseClient* >(layerBase.get());
if (layer != 0) {
mLayerMap.removeItem(layer->serverIndex());
}
@@ -1041,11 +1042,23 @@
}
// it's possible that we don't find a layer, because it might
// have been destroyed already -- this is not technically an error
- // from the user because there is a race between destroySurface,
- // destroyclient and destroySurface-from-a-transaction.
+ // from the user because there is a race between BClient::destroySurface(),
+ // ~BClient() and destroySurface-from-a-transaction.
return (index == NAME_NOT_FOUND) ? status_t(NO_ERROR) : index;
}
+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).
+ ssize_t err = removeLayer_l(layerBase);
+ if (err >= 0) {
+ mLayerPurgatory.add(layerBase);
+ }
+ return (err == NAME_NOT_FOUND) ? status_t(NO_ERROR) : err;
+}
+
+
void SurfaceFlinger::free_resources_l()
{
// Destroy layers that were removed
@@ -1252,14 +1265,53 @@
return layer;
}
-status_t SurfaceFlinger::destroySurface(SurfaceID index)
+status_t SurfaceFlinger::removeSurface(SurfaceID index)
{
+ /*
+ * called by the window manager, when a surface should be marked for
+ * destruction.
+ */
+
+ // TODO: here we should make the surface disappear from the screen
+ // and mark it for removal. however, we can't free anything until all
+ // client are done. All operations on this surface should return errors.
+
+ status_t err = NAME_NOT_FOUND;
+ sp<LayerBaseClient> layer;
+
+ { // scope for the lock
+ Mutex::Autolock _l(mStateLock);
+ layer = getLayerUser_l(index);
+ err = purgatorizeLayer_l(layer);
+ if (err == NO_ERROR) {
+ setTransactionFlags(eTransactionNeeded);
+ }
+ }
+
+ if (layer != 0) {
+ // do this outside of mStateLock
+ layer->ditch();
+ }
+ return err;
+}
+
+status_t SurfaceFlinger::destroySurface(const sp<LayerBaseClient>& layer)
+{
+ /*
+ * called by ~ISurface() when all references are gone
+ */
+
+ /* FIXME:
+ * - this can calls ~Layer(), which is wrong because we're not in the
+ * GL thread, and ~Layer() currently calls OpenGL.
+ * - ideally we want to release as much GL state as possible after
+ * purgatorizeLayer_l() has been called and the surface is not in any
+ * active list.
+ * - ideally we'd call ~Layer() without mStateLock held
+ */
+
Mutex::Autolock _l(mStateLock);
- const sp<LayerBaseClient>& layer = getLayerUser_l(index);
- status_t err = removeLayer_l(layer);
- if (err < 0)
- return err;
- setTransactionFlags(eTransactionNeeded);
+ mLayerPurgatory.remove(layer);
return NO_ERROR;
}
@@ -1626,7 +1678,7 @@
status_t BClient::destroySurface(SurfaceID sid)
{
sid |= (mId << 16); // add the client-part to id
- return mFlinger->destroySurface(sid);
+ return mFlinger->removeSurface(sid);
}
status_t BClient::setState(int32_t count, const layer_state_t* states)
diff --git a/libs/surfaceflinger/SurfaceFlinger.h b/libs/surfaceflinger/SurfaceFlinger.h
index cb84542f..b488ab5 100644
--- a/libs/surfaceflinger/SurfaceFlinger.h
+++ b/libs/surfaceflinger/SurfaceFlinger.h
@@ -203,7 +203,8 @@
Client* client, DisplayID display,
int32_t id, uint32_t w, uint32_t h, uint32_t flags);
- status_t destroySurface(SurfaceID surface_id);
+ status_t removeSurface(SurfaceID surface_id);
+ status_t destroySurface(const sp<LayerBaseClient>& layer);
status_t setClientState(ClientID cid, int32_t count, const layer_state_t* states);
@@ -287,6 +288,7 @@
sp<LayerBaseClient> getLayerUser_l(SurfaceID index) const;
status_t addLayer_l(const sp<LayerBase>& layer);
status_t removeLayer_l(const sp<LayerBase>& layer);
+ status_t purgatorizeLayer_l(const sp<LayerBase>& layer);
void free_resources_l();
uint32_t getTransactionFlags(uint32_t flags);
@@ -315,7 +317,9 @@
volatile int32_t mTransactionFlags;
volatile int32_t mTransactionCount;
Condition mTransactionCV;
+ SortedVector< sp<LayerBase> > mLayerPurgatory;
+
// protected by mStateLock (but we could use another lock)
Tokenizer mTokens;
DefaultKeyedVector<ClientID, Client*> mClientsMap;
