diff --git a/core/java/android/view/Surface.java b/core/java/android/view/Surface.java index 0178d635fb3bd..f82cedce4c08e 100644 --- a/core/java/android/view/Surface.java +++ b/core/java/android/view/Surface.java @@ -283,7 +283,7 @@ public class Surface implements Parcelable { public native void readFromParcel(Parcel source); public native void writeToParcel(Parcel dest, int flags); - + public static final Parcelable.Creator CREATOR = new Parcelable.Creator() { @@ -304,7 +304,7 @@ public class Surface implements Parcelable { /* no user serviceable parts here ... */ @Override protected void finalize() throws Throwable { - clear(); + release(); } private native void init(SurfaceSession s, @@ -312,4 +312,6 @@ public class Surface implements Parcelable { throws OutOfResourcesException; private native void init(Parcel source); + + private native void release(); } diff --git a/core/jni/android_view_Surface.cpp b/core/jni/android_view_Surface.cpp index 54d8b14f062e7..5b675e53defc4 100644 --- a/core/jni/android_view_Surface.cpp +++ b/core/jni/android_view_Surface.cpp @@ -173,6 +173,15 @@ static void Surface_initParcel(JNIEnv* env, jobject clazz, jobject argParcel) } static void Surface_clear(JNIEnv* env, jobject clazz, uintptr_t *ostack) +{ + const sp& surface = getSurface(env, clazz); + if (Surface::isValid(surface)) { + surface->clear(); + } + setSurface(env, clazz, 0); +} + +static void Surface_release(JNIEnv* env, jobject clazz, uintptr_t *ostack) { setSurface(env, clazz, 0); } @@ -180,7 +189,7 @@ static void Surface_clear(JNIEnv* env, jobject clazz, uintptr_t *ostack) static jboolean Surface_isValid(JNIEnv* env, jobject clazz) { const sp& surface = getSurface(env, clazz); - return surface->isValid() ? JNI_TRUE : JNI_FALSE; + return Surface::isValid(surface) ? JNI_TRUE : JNI_FALSE; } static inline SkBitmap::Config convertPixelFormat(PixelFormat format) @@ -201,7 +210,7 @@ static inline SkBitmap::Config convertPixelFormat(PixelFormat format) static jobject Surface_lockCanvas(JNIEnv* env, jobject clazz, jobject dirtyRect) { const sp& surface = getSurface(env, clazz); - if (!surface->isValid()) + if (!Surface::isValid(surface)) return 0; // get dirty region @@ -270,7 +279,7 @@ static void Surface_unlockCanvasAndPost( } const sp& surface = getSurface(env, clazz); - if (!surface->isValid()) + if (!Surface::isValid(surface)) return; // detach the canvas from the surface @@ -337,7 +346,7 @@ static void Surface_setLayer( JNIEnv* env, jobject clazz, jint zorder) { const sp& surface = getSurface(env, clazz); - if (surface->isValid()) { + if (Surface::isValid(surface)) { if (surface->setLayer(zorder) < 0) { doThrow(env, "java/lang/IllegalArgumentException", NULL); } @@ -348,7 +357,7 @@ static void Surface_setPosition( JNIEnv* env, jobject clazz, jint x, jint y) { const sp& surface = getSurface(env, clazz); - if (surface->isValid()) { + if (Surface::isValid(surface)) { if (surface->setPosition(x, y) < 0) { doThrow(env, "java/lang/IllegalArgumentException", NULL); } @@ -359,7 +368,7 @@ static void Surface_setSize( JNIEnv* env, jobject clazz, jint w, jint h) { const sp& surface = getSurface(env, clazz); - if (surface->isValid()) { + if (Surface::isValid(surface)) { if (surface->setSize(w, h) < 0) { doThrow(env, "java/lang/IllegalArgumentException", NULL); } @@ -370,7 +379,7 @@ static void Surface_hide( JNIEnv* env, jobject clazz) { const sp& surface = getSurface(env, clazz); - if (surface->isValid()) { + if (Surface::isValid(surface)) { if (surface->hide() < 0) { doThrow(env, "java/lang/IllegalArgumentException", NULL); } @@ -381,7 +390,7 @@ static void Surface_show( JNIEnv* env, jobject clazz) { const sp& surface = getSurface(env, clazz); - if (surface->isValid()) { + if (Surface::isValid(surface)) { if (surface->show() < 0) { doThrow(env, "java/lang/IllegalArgumentException", NULL); } @@ -392,7 +401,7 @@ static void Surface_freeze( JNIEnv* env, jobject clazz) { const sp& surface = getSurface(env, clazz); - if (surface->isValid()) { + if (Surface::isValid(surface)) { if (surface->freeze() < 0) { doThrow(env, "java/lang/IllegalArgumentException", NULL); } @@ -403,7 +412,7 @@ static void Surface_unfreeze( JNIEnv* env, jobject clazz) { const sp& surface = getSurface(env, clazz); - if (surface->isValid()) { + if (Surface::isValid(surface)) { if (surface->unfreeze() < 0) { doThrow(env, "java/lang/IllegalArgumentException", NULL); } @@ -414,7 +423,7 @@ static void Surface_setFlags( JNIEnv* env, jobject clazz, jint flags, jint mask) { const sp& surface = getSurface(env, clazz); - if (surface->isValid()) { + if (Surface::isValid(surface)) { if (surface->setFlags(flags, mask) < 0) { doThrow(env, "java/lang/IllegalArgumentException", NULL); } @@ -425,7 +434,7 @@ static void Surface_setTransparentRegion( JNIEnv* env, jobject clazz, jobject argRegion) { const sp& surface = getSurface(env, clazz); - if (surface->isValid()) { + if (Surface::isValid(surface)) { SkRegion* nativeRegion = (SkRegion*)env->GetIntField(argRegion, no.native_region); if (surface->setTransparentRegionHint(Region(*nativeRegion)) < 0) { doThrow(env, "java/lang/IllegalArgumentException", NULL); @@ -437,7 +446,7 @@ static void Surface_setAlpha( JNIEnv* env, jobject clazz, jfloat alpha) { const sp& surface = getSurface(env, clazz); - if (surface->isValid()) { + if (Surface::isValid(surface)) { if (surface->setAlpha(alpha) < 0) { doThrow(env, "java/lang/IllegalArgumentException", NULL); } @@ -449,7 +458,7 @@ static void Surface_setMatrix( jfloat dsdx, jfloat dtdx, jfloat dsdy, jfloat dtdy) { const sp& surface = getSurface(env, clazz); - if (surface->isValid()) { + if (Surface::isValid(surface)) { if (surface->setMatrix(dsdx, dtdx, dsdy, dtdy) < 0) { doThrow(env, "java/lang/IllegalArgumentException", NULL); } @@ -461,7 +470,7 @@ static void Surface_setFreezeTint( jint tint) { const sp& surface = getSurface(env, clazz); - if (surface->isValid()) { + if (Surface::isValid(surface)) { if (surface->setFreezeTint(tint) < 0) { doThrow(env, "java/lang/IllegalArgumentException", NULL); } @@ -484,7 +493,7 @@ static void Surface_copyFrom( if (!Surface::isSameSurface(surface, rhs)) { // we reassign the surface only if it's a different one // otherwise we would loose our client-side state. - setSurface(env, clazz, rhs->dup()); + setSurface(env, clazz, rhs); } } @@ -540,7 +549,8 @@ static JNINativeMethod gSurfaceMethods[] = { {"nativeClassInit", "()V", (void*)nativeClassInit }, {"init", "(Landroid/view/SurfaceSession;IIIIII)V", (void*)Surface_init }, {"init", "(Landroid/os/Parcel;)V", (void*)Surface_initParcel }, - {"clear", "()V", (void*)Surface_clear }, + {"clear", "()V", (void*)Surface_clear }, + {"release", "()V", (void*)Surface_release }, {"copyFrom", "(Landroid/view/Surface;)V", (void*)Surface_copyFrom }, {"isValid", "()Z", (void*)Surface_isValid }, {"lockCanvasNative", "(Landroid/graphics/Rect;)Landroid/graphics/Canvas;", (void*)Surface_lockCanvas }, diff --git a/include/ui/Surface.h b/include/ui/Surface.h index d92da4919e63b..1637961b6af8b 100644 --- a/include/ui/Surface.h +++ b/include/ui/Surface.h @@ -90,9 +90,15 @@ public: uint32_t reserved[2]; }; - bool isValid() const { return this && mToken>=0 && mClient!=0; } + static bool isValid(const sp& surface) { + return (surface != 0) && surface->mToken>=0 && surface->mClient!=0; + } + SurfaceID ID() const { return mToken; } + // release surface data from java + void clear(); + status_t lock(SurfaceInfo* info, bool blocking = true); status_t lock(SurfaceInfo* info, Region* dirty, bool blocking = true); status_t unlockAndPost(); @@ -103,7 +109,6 @@ public: void setSwapRectangle(const Rect& r); const Rect& swapRectangle() const; - sp dup() const; static sp readFromParcel(Parcel* parcel); static status_t writeToParcel(const sp& surface, Parcel* parcel); static bool isSameSurface(const sp& lhs, const sp& rhs); @@ -149,6 +154,8 @@ private: ~Surface(); + void destroy(); + Region dirtyRegion() const; void setDirtyRegion(const Region& region) const; diff --git a/libs/ui/Surface.cpp b/libs/ui/Surface.cpp index 1ceece019527a..51abd95faa051 100644 --- a/libs/ui/Surface.cpp +++ b/libs/ui/Surface.cpp @@ -176,61 +176,60 @@ Surface::Surface(const sp& client, const_cast(android_native_window_t::flags) = 0; } -Surface::Surface(Surface const* rhs) - : mOwner(false) -{ - // FIXME: we really should get rid of this ctor. the memcpy below - //should be safe for now, but android_native_window_t is not supposed - // to be clonable. - memcpy( static_cast(this), - static_cast(rhs), - sizeof(android_native_window_t)); - - mToken = rhs->mToken; - mIdentity = rhs->mIdentity; - mClient = rhs->mClient; - mSurface = rhs->mSurface; - mBuffers[0] = rhs->mBuffers[0]; - mBuffers[1] = rhs->mBuffers[1]; - mFormat = rhs->mFormat; - mFlags = rhs->mFlags; - mSwapRectangle.makeInvalid(); -} - Surface::~Surface() { - if (mOwner && mToken>=0 && mClient!=0) { - mClient->destroySurface(mToken); - if (mBuffers[0] != 0) { - BufferMapper::get().unmap(mBuffers[0]->getHandle(), this); - } - if (mBuffers[1] != 0) { - BufferMapper::get().unmap(mBuffers[1]->getHandle(), this); + // this is a client-side operation, the surface is destroyed, unmap + // its buffers in this process. + for (int i=0 ; i<2 ; i++) { + if (mBuffers[i] != 0) { + BufferMapper::get().unmap(mBuffers[i]->getHandle(), this); } } + + destroy(); +} + +void Surface::destroy() +{ + // Destroy the surface in SurfaceFlinger if we were the owner + // (in any case, a client won't be able to, because it won't have the + // right permission). + if (mOwner && mToken>=0 && mClient!=0) { + mClient->destroySurface(mToken); + } + + // clear all references and trigger an IPC now, to make sure things + // happen without delay, since these resources are quite heavy. mClient.clear(); mSurface.clear(); IPCThreadState::self()->flushCommands(); } -sp Surface::dup() const +void Surface::clear() { - // FIXME: we should get rid of Surface::dup() - Surface const * r = this; - if (this && mOwner) { - // the only reason we need to do this is because of Java's garbage - // collector: because we're creating a copy of the Surface - // instead of a reference, we can guarantee that when our last - // reference goes away, the real surface will be deleted. - // Without this hack (the code is correct too), we'd have to - // wait for a GC for the surface to go away. - r = new Surface(this); - } - return const_cast(r); + // here, the window manager tells us explicitly that we should destroy + // the surface's resource. Soon after this call, it will also release + // its last reference (which will call the dtor); however, it is possible + // that a client living in the same process still holds references which + // would delay the call to the dtor -- that is why we need this explicit + // "clear()" call. + + // FIXME: we should probably unmap the buffers here. The problem is that + // the app could be in the middle of using them, and if we don't unmap now + // and we're in the system process, the mapping will be lost (because + // the buffer will be freed, and the handles destroyed) + + Mutex::Autolock _l(mSurfaceLock); + destroy(); } status_t Surface::validate(per_client_cblk_t const* cblk) const { + if (mToken<0 || mClient==0) { + LOGE("invalid token (%d, identity=%u) or client (%p)", + mToken, mIdentity, mClient.get()); + return NO_INIT; + } if (cblk == 0) { LOGE("cblk is null (surface id=%d, identity=%u)", mToken, mIdentity); return NO_INIT; @@ -330,6 +329,13 @@ int Surface::dequeueBuffer(android_native_buffer_t** buffer) int Surface::lockBuffer(android_native_buffer_t* buffer) { + Mutex::Autolock _l(mSurfaceLock); + + per_client_cblk_t* const cblk = mClient->mControl; + status_t err = validate(cblk); + if (err != NO_ERROR) + return err; + // FIXME: lockBuffer() needs proper implementation return 0; } @@ -543,7 +549,7 @@ status_t Surface::writeToParcel(const sp& surface, Parcel* parcel) uint32_t identity = 0; sp client; sp sur; - if (surface->isValid()) { + if (Surface::isValid(surface)) { token = surface->mToken; identity = surface->mIdentity; client = surface->mClient;