From 2970703fb5c9047df16fdcd1dfad63cc5fa63e66 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Tue, 9 Aug 2011 15:48:43 -0700 Subject: [PATCH 1/8] fix a crasher in dumpsys Bug: 5141729 Change-Id: Ib104d49c8660621180966be099198fe29c5bebf5 --- libs/gui/SurfaceTexture.cpp | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/libs/gui/SurfaceTexture.cpp b/libs/gui/SurfaceTexture.cpp index be71c946b..e0c8d8303 100644 --- a/libs/gui/SurfaceTexture.cpp +++ b/libs/gui/SurfaceTexture.cpp @@ -981,19 +981,25 @@ void SurfaceTexture::dump(String8& result, const char* prefix, for (int i=0 ; i& buf(slot.mGraphicBuffer); snprintf(buffer, SIZE, "%s%s[%02d] " - "%p [%4ux%4u:%4u,%3X] " "state=%-8s, crop=[%d,%d,%d,%d], " - "transform=0x%02x, timestamp=%lld\n", + "transform=0x%02x, timestamp=%lld", prefix, (i==mCurrentTexture)?">":" ", i, - buf->handle, buf->width, buf->height, buf->stride, buf->format, stateName(slot.mBufferState), slot.mCrop.left, slot.mCrop.top, slot.mCrop.right, slot.mCrop.bottom, slot.mTransform, slot.mTimestamp ); result.append(buffer); + + const sp& buf(slot.mGraphicBuffer); + if (buf != NULL) { + snprintf(buffer, SIZE, + ", %p [%4ux%4u:%4u,%3X]", + buf->handle, buf->width, buf->height, buf->stride, buf->format); + result.append(buffer); + } + result.append("\n"); } } From 4d952c0d0aa5d2be48da4ace260773dd8b5ada36 Mon Sep 17 00:00:00 2001 From: Jamie Gennis Date: Wed, 17 Aug 2011 18:19:00 -0700 Subject: [PATCH 2/8] SurfaceTexture: fix queues-to-composer This change fixes the NATIVE_WINDOW_QUEUES_TO_WINDOW_COMPOSER query of Surface and SurfaceTextureClient. Surface now uses the inherited SurfaceTextureClient implementation of this query. SurfaceTextureClient now queries SurfaceFlinger to determine whether buffers that are queued to its ISurfaceTexture will be sent to SurfaceFlinger (as opposed to some other process). Change-Id: Iff187e72f30d454229f07f896b438198978270a8 --- include/surfaceflinger/ISurfaceComposer.h | 5 +++-- libs/gui/ISurfaceComposer.cpp | 26 +++++++++++++--------- libs/gui/Surface.cpp | 3 --- libs/gui/SurfaceTextureClient.cpp | 12 +++++++++- services/surfaceflinger/Layer.cpp | 5 +++++ services/surfaceflinger/Layer.h | 3 +++ services/surfaceflinger/LayerBase.cpp | 4 ++++ services/surfaceflinger/LayerBase.h | 1 + services/surfaceflinger/SurfaceFlinger.cpp | 23 ++++++++++++------- services/surfaceflinger/SurfaceFlinger.h | 2 +- 10 files changed, 58 insertions(+), 26 deletions(-) diff --git a/include/surfaceflinger/ISurfaceComposer.h b/include/surfaceflinger/ISurfaceComposer.h index dba98a32c..6b31ca4cb 100644 --- a/include/surfaceflinger/ISurfaceComposer.h +++ b/include/surfaceflinger/ISurfaceComposer.h @@ -132,9 +132,10 @@ public: virtual status_t turnElectronBeamOff(int32_t mode) = 0; virtual status_t turnElectronBeamOn(int32_t mode) = 0; - /* verify that an ISurface was created by SurfaceFlinger. + /* verify that an ISurfaceTexture was created by SurfaceFlinger. */ - virtual bool authenticateSurface(const sp& surface) const = 0; + virtual bool authenticateSurfaceTexture( + const sp& surface) const = 0; }; // ---------------------------------------------------------------------------- diff --git a/libs/gui/ISurfaceComposer.cpp b/libs/gui/ISurfaceComposer.cpp index c1156d5c8..030a83e96 100644 --- a/libs/gui/ISurfaceComposer.cpp +++ b/libs/gui/ISurfaceComposer.cpp @@ -31,6 +31,8 @@ #include +#include + #include // --------------------------------------------------------------------------- @@ -166,35 +168,36 @@ public: return reply.readInt32(); } - virtual bool authenticateSurface(const sp& surface) const + virtual bool authenticateSurfaceTexture( + const sp& surfaceTexture) const { Parcel data, reply; int err = NO_ERROR; err = data.writeInterfaceToken( ISurfaceComposer::getInterfaceDescriptor()); if (err != NO_ERROR) { - LOGE("ISurfaceComposer::authenticateSurface: error writing " + LOGE("ISurfaceComposer::authenticateSurfaceTexture: error writing " "interface descriptor: %s (%d)", strerror(-err), -err); return false; } - err = data.writeStrongBinder(surface->asBinder()); + err = data.writeStrongBinder(surfaceTexture->asBinder()); if (err != NO_ERROR) { - LOGE("ISurfaceComposer::authenticateSurface: error writing strong " - "binder to parcel: %s (%d)", strerror(-err), -err); + LOGE("ISurfaceComposer::authenticateSurfaceTexture: error writing " + "strong binder to parcel: %s (%d)", strerror(-err), -err); return false; } err = remote()->transact(BnSurfaceComposer::AUTHENTICATE_SURFACE, data, &reply); if (err != NO_ERROR) { - LOGE("ISurfaceComposer::authenticateSurface: error performing " - "transaction: %s (%d)", strerror(-err), -err); + LOGE("ISurfaceComposer::authenticateSurfaceTexture: error " + "performing transaction: %s (%d)", strerror(-err), -err); return false; } int32_t result = 0; err = reply.readInt32(&result); if (err != NO_ERROR) { - LOGE("ISurfaceComposer::authenticateSurface: error retrieving " - "result: %s (%d)", strerror(-err), -err); + LOGE("ISurfaceComposer::authenticateSurfaceTexture: error " + "retrieving result: %s (%d)", strerror(-err), -err); return false; } return result != 0; @@ -291,8 +294,9 @@ status_t BnSurfaceComposer::onTransact( } break; case AUTHENTICATE_SURFACE: { CHECK_INTERFACE(ISurfaceComposer, data, reply); - sp surface = interface_cast(data.readStrongBinder()); - int32_t result = authenticateSurface(surface) ? 1 : 0; + sp surfaceTexture = + interface_cast(data.readStrongBinder()); + int32_t result = authenticateSurfaceTexture(surfaceTexture) ? 1 : 0; reply->writeInt32(result); } break; default: diff --git a/libs/gui/Surface.cpp b/libs/gui/Surface.cpp index 2c702511d..54d04aada 100644 --- a/libs/gui/Surface.cpp +++ b/libs/gui/Surface.cpp @@ -342,9 +342,6 @@ sp Surface::asBinder() const { int Surface::query(int what, int* value) const { switch (what) { - case NATIVE_WINDOW_QUEUES_TO_WINDOW_COMPOSER: - *value = 1; - return NO_ERROR; case NATIVE_WINDOW_CONCRETE_TYPE: *value = NATIVE_WINDOW_SURFACE; return NO_ERROR; diff --git a/libs/gui/SurfaceTextureClient.cpp b/libs/gui/SurfaceTextureClient.cpp index e91be84c4..5a35b4d48 100644 --- a/libs/gui/SurfaceTextureClient.cpp +++ b/libs/gui/SurfaceTextureClient.cpp @@ -18,6 +18,8 @@ //#define LOG_NDEBUG 0 #include +#include +#include #include @@ -234,7 +236,15 @@ int SurfaceTextureClient::query(int what, int* value) const { } break; case NATIVE_WINDOW_QUEUES_TO_WINDOW_COMPOSER: - *value = 0; + { + sp composer( + ComposerService::getComposerService()); + if (composer->authenticateSurfaceTexture(mSurfaceTexture)) { + *value = 1; + } else { + *value = 0; + } + } return NO_ERROR; case NATIVE_WINDOW_CONCRETE_TYPE: *value = NATIVE_WINDOW_SURFACE_TEXTURE_CLIENT; diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 0425fc3ba..64eaecc86 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -133,6 +133,11 @@ sp Layer::createSurface() return sur; } +wp Layer::getSurfaceTextureBinder() const +{ + return mSurfaceTexture->asBinder(); +} + 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 d3ddab4f2..5f0be8074 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -74,6 +74,9 @@ public: virtual bool isProtected() const; virtual void onRemoved(); + // LayerBaseClient interface + virtual wp getSurfaceTextureBinder() const; + // only for debugging inline const sp& getFreezeLock() const { return mFreezeLock; } diff --git a/services/surfaceflinger/LayerBase.cpp b/services/surfaceflinger/LayerBase.cpp index e04c533ef..7bf73d9ae 100644 --- a/services/surfaceflinger/LayerBase.cpp +++ b/services/surfaceflinger/LayerBase.cpp @@ -544,6 +544,10 @@ wp LayerBaseClient::getSurfaceBinder() const { return mClientSurfaceBinder; } +wp LayerBaseClient::getSurfaceTextureBinder() const { + return 0; +} + void LayerBaseClient::dump(String8& result, char* buffer, size_t SIZE) const { LayerBase::dump(result, buffer, SIZE); diff --git a/services/surfaceflinger/LayerBase.h b/services/surfaceflinger/LayerBase.h index faf71ddbe..a3d36444b 100644 --- a/services/surfaceflinger/LayerBase.h +++ b/services/surfaceflinger/LayerBase.h @@ -288,6 +288,7 @@ public: sp getSurface(); wp getSurfaceBinder() const; + virtual wp getSurfaceTextureBinder() const; virtual sp getLayerBaseClient() const { return const_cast(this); } diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index a68ab3055..50afb3d8b 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -353,9 +353,10 @@ void SurfaceFlinger::signalEvent() { mEventQueue.invalidate(); } -bool SurfaceFlinger::authenticateSurface(const sp& surface) const { +bool SurfaceFlinger::authenticateSurfaceTexture( + const sp& surfaceTexture) const { Mutex::Autolock _l(mStateLock); - sp surfBinder(surface->asBinder()); + sp surfaceTextureBinder(surfaceTexture->asBinder()); // Check the visible layer list for the ISurface const LayerVector& currentLayers = mCurrentState.layersSortedByZ; @@ -363,14 +364,17 @@ bool SurfaceFlinger::authenticateSurface(const sp& surface) const { for (size_t i=0 ; i& layer(currentLayers[i]); sp lbc(layer->getLayerBaseClient()); - if (lbc != NULL && lbc->getSurfaceBinder() == surfBinder) { - return true; + if (lbc != NULL) { + wp lbcBinder = lbc->getSurfaceTextureBinder(); + if (lbcBinder == surfaceTextureBinder) { + return true; + } } } // Check the layers in the purgatory. This check is here so that if a - // Surface gets destroyed before all the clients are done using it, the - // error will not be reported as "surface XYZ is not authenticated", but + // SurfaceTexture gets destroyed before all the clients are done using it, + // the error will not be reported as "surface XYZ is not authenticated", but // will instead fail later on when the client tries to use the surface, // which should be reported as "surface XYZ returned an -ENODEV". The // purgatorized layers are no less authentic than the visible ones, so this @@ -379,8 +383,11 @@ bool SurfaceFlinger::authenticateSurface(const sp& surface) const { for (size_t i=0 ; i& layer(mLayerPurgatory.itemAt(i)); sp lbc(layer->getLayerBaseClient()); - if (lbc != NULL && lbc->getSurfaceBinder() == surfBinder) { - return true; + if (lbc != NULL) { + wp lbcBinder = lbc->getSurfaceTextureBinder(); + if (lbcBinder == surfaceTextureBinder) { + return true; + } } } diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 89df0de55..1738238ab 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -171,7 +171,7 @@ public: virtual status_t freezeDisplay(DisplayID dpy, uint32_t flags); virtual status_t unfreezeDisplay(DisplayID dpy, uint32_t flags); virtual int setOrientation(DisplayID dpy, int orientation, uint32_t flags); - virtual bool authenticateSurface(const sp& surface) const; + virtual bool authenticateSurfaceTexture(const sp& surface) const; virtual status_t captureScreen(DisplayID dpy, sp* heap, From 9918e7e7852adcd3282eccc46582b21fd1956853 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Thu, 25 Aug 2011 17:03:30 -0700 Subject: [PATCH 3/8] make sure to re-initialize SurfaceTexture to its default state on disconnect this caused problems where the NavigationBar would disapear or be drawn in the wrong orientation. Change-Id: I083c41338db83a4afd14f427caec2f31c180d734 --- libs/gui/SurfaceTexture.cpp | 5 ++++- libs/gui/SurfaceTextureClient.cpp | 11 +++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/libs/gui/SurfaceTexture.cpp b/libs/gui/SurfaceTexture.cpp index ac9b33b61..79a01a351 100644 --- a/libs/gui/SurfaceTexture.cpp +++ b/libs/gui/SurfaceTexture.cpp @@ -608,6 +608,9 @@ status_t SurfaceTexture::disconnect(int api) { if (mConnectedApi == api) { drainQueueAndFreeBuffersLocked(); mConnectedApi = NO_CONNECTED_API; + mNextCrop.makeInvalid(); + mNextScalingMode = NATIVE_WINDOW_SCALING_MODE_FREEZE; + mNextTransform = 0; mDequeueCondition.signal(); } else { LOGE("disconnect: connected to another api (cur=%d, req=%d)", @@ -1022,7 +1025,7 @@ void SurfaceTexture::dump(String8& result, const char* prefix, mCurrentCrop.top, mCurrentCrop.right, mCurrentCrop.bottom, mCurrentTransform, mCurrentTexture, prefix, mNextCrop.left, mNextCrop.top, mNextCrop.right, mNextCrop.bottom, - mCurrentTransform, fifoSize, fifo.string() + mNextTransform, fifoSize, fifo.string() ); result.append(buffer); diff --git a/libs/gui/SurfaceTextureClient.cpp b/libs/gui/SurfaceTextureClient.cpp index 5a35b4d48..710ef94b8 100644 --- a/libs/gui/SurfaceTextureClient.cpp +++ b/libs/gui/SurfaceTextureClient.cpp @@ -407,8 +407,15 @@ int SurfaceTextureClient::disconnect(int api) { LOGV("SurfaceTextureClient::disconnect"); Mutex::Autolock lock(mMutex); int err = mSurfaceTexture->disconnect(api); - if (!err && api == NATIVE_WINDOW_API_CPU) { - mConnectedToCpu = false; + if (!err) { + freeAllBuffers(); + mReqFormat = 0; + mReqWidth = 0; + mReqHeight = 0; + mReqUsage = 0; + if (api == NATIVE_WINDOW_API_CPU) { + mConnectedToCpu = false; + } } return err; } From 64746ef0fea164bc99879dbe58fa2ef72bbbd065 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Thu, 1 Sep 2011 18:52:12 -0700 Subject: [PATCH 4/8] Fix various flickering / artifacts these were due to the "preserve backbuffer" optimization interfering with hw composer. basically the screen needed to be redrawn in the areas that move from GL to overlay. Bug: 5245513 Change-Id: I9bf75c4fe905f3ef62005e52108b94edae692304 --- services/surfaceflinger/SurfaceFlinger.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 598220fd4..b4c5decae 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -817,6 +817,20 @@ void SurfaceFlinger::handleWorkList() mHwWorkListDirty = false; HWComposer& hwc(graphicPlane(0).displayHardware().getHwComposer()); if (hwc.initCheck() == NO_ERROR) { + + const DisplayHardware& hw(graphicPlane(0).displayHardware()); + uint32_t flags = hw.getFlags(); + if ((flags & DisplayHardware::SWAP_RECTANGLE) || + (flags & DisplayHardware::BUFFER_PRESERVED)) + { + // we need to redraw everything (the whole screen) + // NOTE: we could be more subtle here and redraw only + // the area which will end-up in an overlay. But since this + // shouldn't happen often, we invalidate everything. + mDirtyRegion.set(hw.bounds()); + mInvalidRegion = mDirtyRegion; + } + const Vector< sp >& currentLayers(mVisibleLayersSortedByZ); const size_t count = currentLayers.size(); hwc.createWorkList(count); From 8275d8eedf98a4da3b0cc84cabe11da66bd16bad Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Fri, 2 Sep 2011 12:22:39 -0700 Subject: [PATCH 5/8] fix a bug that caused the off animation to not show sometimes this happened when the overlays were in use, since the animation is rendered in the FB and the FB is not used. we now have a way to turn hwc off temporarily. Change-Id: I3385f0c25bb9cc91948e7b26e7cd31ed18c36ace --- .../DisplayHardware/HWComposer.cpp | 17 +++++++++++++++-- .../surfaceflinger/DisplayHardware/HWComposer.h | 3 +++ services/surfaceflinger/SurfaceFlinger.cpp | 6 ++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp index 7d1bdf002..0ff1cce81 100644 --- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp +++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp @@ -108,8 +108,21 @@ status_t HWComposer::commit() const { } status_t HWComposer::release() const { - int err = mHwc->set(mHwc, NULL, NULL, NULL); - return (status_t)err; + if (mHwc) { + int err = mHwc->set(mHwc, NULL, NULL, NULL); + return (status_t)err; + } + return NO_ERROR; +} + +status_t HWComposer::disable() { + if (mHwc) { + free(mList); + mList = NULL; + int err = mHwc->prepare(mHwc, NULL); + return (status_t)err; + } + return NO_ERROR; } size_t HWComposer::getNumLayers() const { diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.h b/services/surfaceflinger/DisplayHardware/HWComposer.h index 983898a42..77c1a4b2a 100644 --- a/services/surfaceflinger/DisplayHardware/HWComposer.h +++ b/services/surfaceflinger/DisplayHardware/HWComposer.h @@ -50,6 +50,9 @@ public: // Asks the HAL what it can do status_t prepare() const; + // disable hwc until next createWorkList + status_t disable(); + // commits the list status_t commit() const; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index b4c5decae..4a3a8ea6c 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -2113,6 +2113,12 @@ status_t SurfaceFlinger::turnElectronBeamOffImplLocked(int32_t mode) // we're already off return NO_ERROR; } + + // turn off hwc while we're doing the animation + hw.getHwComposer().disable(); + // and make sure to turn it back on (if needed) next time we compose + invalidateHwcGeometry(); + if (mode & ISurfaceComposer::eElectronBeamAnimationOff) { electronBeamOffAnimationImplLocked(); } From 5a7f381667b91d6519521d80080290c399492208 Mon Sep 17 00:00:00 2001 From: Bart Sears Date: Sun, 25 Sep 2011 14:30:21 -0700 Subject: [PATCH 6/8] Revert "Transfer large bitmaps using ashmem. Bug: 5224703" This reverts commit 56c58f66b97d22fe7e7de1f7d9548bcbe1973029 This CL was causing the browser to crash when adding bookmarks, visiting the bookmarks page, and sharing pages (see bug http://b/issue?id=5369231 --- include/binder/Parcel.h | 49 ++------------------- libs/binder/Parcel.cpp | 97 ----------------------------------------- 2 files changed, 3 insertions(+), 143 deletions(-) diff --git a/include/binder/Parcel.h b/include/binder/Parcel.h index 53a53c422..bfe13f01b 100644 --- a/include/binder/Parcel.h +++ b/include/binder/Parcel.h @@ -38,9 +38,6 @@ struct flat_binder_object; // defined in support_p/binder_module.h class Parcel { public: - class ReadableBlob; - class WritableBlob; - Parcel(); ~Parcel(); @@ -112,13 +109,7 @@ public: // Place a file descriptor into the parcel. A dup of the fd is made, which // will be closed once the parcel is destroyed. status_t writeDupFileDescriptor(int fd); - - // Writes a blob to the parcel. - // If the blob is small, then it is stored in-place, otherwise it is - // transferred by way of an anonymous shared memory region. - // The caller should call release() on the blob after writing its contents. - status_t writeBlob(size_t len, WritableBlob* outBlob); - + status_t writeObject(const flat_binder_object& val, bool nullMetaData); // Like Parcel.java's writeNoException(). Just writes a zero int32. @@ -166,11 +157,7 @@ public: // Retrieve a file descriptor from the parcel. This returns the raw fd // in the parcel, which you do not own -- use dup() to get your own copy. int readFileDescriptor() const; - - // Reads a blob from the parcel. - // The caller should call release() on the blob after reading its contents. - status_t readBlob(size_t len, ReadableBlob* outBlob) const; - + const flat_binder_object* readObject(bool nullMetaData) const; // Explicitly close all file descriptors in the parcel. @@ -190,7 +177,7 @@ public: release_func relFunc, void* relCookie); void print(TextOutput& to, uint32_t flags = 0) const; - + private: Parcel(const Parcel& o); Parcel& operator=(const Parcel& o); @@ -228,36 +215,6 @@ private: release_func mOwner; void* mOwnerCookie; - - class Blob { - public: - Blob(); - ~Blob(); - - void release(); - inline size_t size() const { return mSize; } - - protected: - void init(bool mapped, void* data, size_t size); - void clear(); - - bool mMapped; - void* mData; - size_t mSize; - }; - -public: - class ReadableBlob : public Blob { - friend class Parcel; - public: - inline const void* data() const { return mData; } - }; - - class WritableBlob : public Blob { - friend class Parcel; - public: - inline void* data() { return mData; } - }; }; // --------------------------------------------------------------------------- diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp index 126d3f57a..a0fc4d05b 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -30,14 +30,12 @@ #include #include #include -#include #include #include #include #include -#include #ifndef INT32_MAX #define INT32_MAX ((int32_t)(2147483647)) @@ -56,9 +54,6 @@ // Note: must be kept in sync with android/os/Parcel.java's EX_HAS_REPLY_HEADER #define EX_HAS_REPLY_HEADER -128 -// Maximum size of a blob to transfer in-place. -static const size_t IN_PLACE_BLOB_LIMIT = 40 * 1024; - // XXX This can be made public if we want to provide // support for typed data. struct small_flat_data @@ -711,47 +706,6 @@ status_t Parcel::writeDupFileDescriptor(int fd) return writeObject(obj, true); } -status_t Parcel::writeBlob(size_t len, WritableBlob* outBlob) -{ - if (len <= IN_PLACE_BLOB_LIMIT) { - LOGV("writeBlob: write in place"); - void* ptr = writeInplace(len); - if (!ptr) return NO_MEMORY; - - outBlob->init(false /*mapped*/, ptr, len); - return NO_ERROR; - } - - LOGV("writeBlob: write to ashmem"); - int fd = ashmem_create_region("Parcel Blob", len); - if (fd < 0) return NO_MEMORY; - - status_t status; - int result = ashmem_set_prot_region(fd, PROT_READ | PROT_WRITE); - if (result < 0) { - status = -result; - } else { - void* ptr = ::mmap(NULL, len, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); - if (ptr == MAP_FAILED) { - status = -errno; - } else { - result = ashmem_set_prot_region(fd, PROT_READ); - if (result < 0) { - status = -result; - } else { - status = writeFileDescriptor(fd); - if (!status) { - outBlob->init(true /*mapped*/, ptr, len); - return NO_ERROR; - } - } - } - ::munmap(ptr, len); - } - ::close(fd); - return status; -} - status_t Parcel::write(const Flattenable& val) { status_t err; @@ -1071,28 +1025,6 @@ int Parcel::readFileDescriptor() const return BAD_TYPE; } -status_t Parcel::readBlob(size_t len, ReadableBlob* outBlob) const -{ - if (len <= IN_PLACE_BLOB_LIMIT) { - LOGV("readBlob: read in place"); - const void* ptr = readInplace(len); - if (!ptr) return BAD_VALUE; - - outBlob->init(false /*mapped*/, const_cast(ptr), len); - return NO_ERROR; - } - - LOGV("readBlob: read from ashmem"); - int fd = readFileDescriptor(); - if (fd == int(BAD_TYPE)) return BAD_VALUE; - - void* ptr = ::mmap(NULL, len, PROT_READ, MAP_SHARED, fd, 0); - if (!ptr) return NO_MEMORY; - - outBlob->init(true /*mapped*/, ptr, len); - return NO_ERROR; -} - status_t Parcel::read(Flattenable& val) const { // size @@ -1520,33 +1452,4 @@ void Parcel::scanForFds() const mFdsKnown = true; } -// --- Parcel::Blob --- - -Parcel::Blob::Blob() : - mMapped(false), mData(NULL), mSize(0) { -} - -Parcel::Blob::~Blob() { - release(); -} - -void Parcel::Blob::release() { - if (mMapped && mData) { - ::munmap(mData, mSize); - } - clear(); -} - -void Parcel::Blob::init(bool mapped, void* data, size_t size) { - mMapped = mapped; - mData = data; - mSize = size; -} - -void Parcel::Blob::clear() { - mMapped = false; - mData = NULL; - mSize = 0; -} - }; // namespace android From 5795536fa1d539d7c6c7773e734663dfa7820d18 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Sun, 13 Nov 2011 20:50:07 -0800 Subject: [PATCH 7/8] fix crash when validating an invalid EGL objects the code that validated EGL objects dereferenced the object to access its EGLDisplay -- needed for validation (!). This was wrong for two reasons, first we dereferenced the object before validating it (potentially leading to a crash), secondly we didn't validate that the object existed in the right EGLDisplay. We now use the EGLDisplay passed by the user API. Change-Id: I66f9e851d4f8507892a6b1fee3065f124c4e7138 --- opengl/libs/EGL/egl.cpp | 14 ++++++---- opengl/libs/EGL/eglApi.cpp | 48 ++++++++++++++++----------------- opengl/libs/EGL/egl_display.cpp | 8 +++--- opengl/libs/EGL/egl_display.h | 8 +++--- opengl/libs/EGL/egl_object.cpp | 4 +-- opengl/libs/EGL/egl_object.h | 7 ++--- 6 files changed, 48 insertions(+), 41 deletions(-) diff --git a/opengl/libs/EGL/egl.cpp b/opengl/libs/EGL/egl.cpp index 1e4319510..6ad06afff 100644 --- a/opengl/libs/EGL/egl.cpp +++ b/opengl/libs/EGL/egl.cpp @@ -212,16 +212,20 @@ egl_connection_t* validate_display_config(EGLDisplay dpy, EGLConfig config, EGLImageKHR egl_get_image_for_current_context(EGLImageKHR image) { - ImageRef _i(image); - if (!_i.get()) - return EGL_NO_IMAGE_KHR; - EGLContext context = egl_tls_t::getContext(); if (context == EGL_NO_CONTEXT || image == EGL_NO_IMAGE_KHR) return EGL_NO_IMAGE_KHR; egl_context_t const * const c = get_context(context); - if (c == NULL) // this should never happen + if (c == NULL) // this should never happen, by construction + return EGL_NO_IMAGE_KHR; + + egl_display_t* display = egl_display_t::get(c->dpy); + if (display == NULL) // this should never happen, by construction + return EGL_NO_IMAGE_KHR; + + ImageRef _i(display, image); + if (!_i.get()) return EGL_NO_IMAGE_KHR; // here we don't validate the context because if it's been marked for diff --git a/opengl/libs/EGL/eglApi.cpp b/opengl/libs/EGL/eglApi.cpp index 63f02e494..095f10c80 100644 --- a/opengl/libs/EGL/eglApi.cpp +++ b/opengl/libs/EGL/eglApi.cpp @@ -451,7 +451,7 @@ EGLBoolean eglDestroySurface(EGLDisplay dpy, EGLSurface surface) egl_display_t const * const dp = validate_display(dpy); if (!dp) return EGL_FALSE; - SurfaceRef _s(surface); + SurfaceRef _s(dp, surface); if (!_s.get()) return setError(EGL_BAD_SURFACE, EGL_FALSE); @@ -472,7 +472,7 @@ EGLBoolean eglQuerySurface( EGLDisplay dpy, EGLSurface surface, egl_display_t const * const dp = validate_display(dpy); if (!dp) return EGL_FALSE; - SurfaceRef _s(surface); + SurfaceRef _s(dp, surface); if (!_s.get()) return setError(EGL_BAD_SURFACE, EGL_FALSE); @@ -541,7 +541,7 @@ EGLBoolean eglDestroyContext(EGLDisplay dpy, EGLContext ctx) if (!dp) return EGL_FALSE; - ContextRef _c(ctx); + ContextRef _c(dp, ctx); if (!_c.get()) return setError(EGL_BAD_CONTEXT, EGL_FALSE); @@ -592,9 +592,9 @@ EGLBoolean eglMakeCurrent( EGLDisplay dpy, EGLSurface draw, } // get a reference to the object passed in - ContextRef _c(ctx); - SurfaceRef _d(draw); - SurfaceRef _r(read); + ContextRef _c(dp, ctx); + SurfaceRef _d(dp, draw); + SurfaceRef _r(dp, read); // validate the context (if not EGL_NO_CONTEXT) if ((ctx != EGL_NO_CONTEXT) && !_c.get()) { @@ -696,7 +696,7 @@ EGLBoolean eglQueryContext( EGLDisplay dpy, EGLContext ctx, egl_display_t const * const dp = validate_display(dpy); if (!dp) return EGL_FALSE; - ContextRef _c(ctx); + ContextRef _c(dp, ctx); if (!_c.get()) return setError(EGL_BAD_CONTEXT, EGL_FALSE); egl_context_t * const c = get_context(ctx); @@ -944,7 +944,7 @@ EGLBoolean eglSwapBuffers(EGLDisplay dpy, EGLSurface draw) egl_display_t const * const dp = validate_display(dpy); if (!dp) return EGL_FALSE; - SurfaceRef _s(draw); + SurfaceRef _s(dp, draw); if (!_s.get()) return setError(EGL_BAD_SURFACE, EGL_FALSE); @@ -960,7 +960,7 @@ EGLBoolean eglCopyBuffers( EGLDisplay dpy, EGLSurface surface, egl_display_t const * const dp = validate_display(dpy); if (!dp) return EGL_FALSE; - SurfaceRef _s(surface); + SurfaceRef _s(dp, surface); if (!_s.get()) return setError(EGL_BAD_SURFACE, EGL_FALSE); @@ -1002,7 +1002,7 @@ EGLBoolean eglSurfaceAttrib( egl_display_t const * const dp = validate_display(dpy); if (!dp) return EGL_FALSE; - SurfaceRef _s(surface); + SurfaceRef _s(dp, surface); if (!_s.get()) return setError(EGL_BAD_SURFACE, EGL_FALSE); @@ -1022,7 +1022,7 @@ EGLBoolean eglBindTexImage( egl_display_t const * const dp = validate_display(dpy); if (!dp) return EGL_FALSE; - SurfaceRef _s(surface); + SurfaceRef _s(dp, surface); if (!_s.get()) return setError(EGL_BAD_SURFACE, EGL_FALSE); @@ -1042,7 +1042,7 @@ EGLBoolean eglReleaseTexImage( egl_display_t const * const dp = validate_display(dpy); if (!dp) return EGL_FALSE; - SurfaceRef _s(surface); + SurfaceRef _s(dp, surface); if (!_s.get()) return setError(EGL_BAD_SURFACE, EGL_FALSE); @@ -1201,7 +1201,7 @@ EGLBoolean eglLockSurfaceKHR(EGLDisplay dpy, EGLSurface surface, egl_display_t const * const dp = validate_display(dpy); if (!dp) return EGL_FALSE; - SurfaceRef _s(surface); + SurfaceRef _s(dp, surface); if (!_s.get()) return setError(EGL_BAD_SURFACE, EGL_FALSE); @@ -1220,7 +1220,7 @@ EGLBoolean eglUnlockSurfaceKHR(EGLDisplay dpy, EGLSurface surface) egl_display_t const * const dp = validate_display(dpy); if (!dp) return EGL_FALSE; - SurfaceRef _s(surface); + SurfaceRef _s(dp, surface); if (!_s.get()) return setError(EGL_BAD_SURFACE, EGL_FALSE); @@ -1241,7 +1241,7 @@ EGLImageKHR eglCreateImageKHR(EGLDisplay dpy, EGLContext ctx, EGLenum target, if (!dp) return EGL_NO_IMAGE_KHR; if (ctx != EGL_NO_CONTEXT) { - ContextRef _c(ctx); + ContextRef _c(dp, ctx); if (!_c.get()) return setError(EGL_BAD_CONTEXT, EGL_NO_IMAGE_KHR); egl_context_t * const c = get_context(ctx); @@ -1310,7 +1310,7 @@ EGLBoolean eglDestroyImageKHR(EGLDisplay dpy, EGLImageKHR img) egl_display_t const * const dp = validate_display(dpy); if (!dp) return EGL_FALSE; - ImageRef _i(img); + ImageRef _i(dp, img); if (!_i.get()) return setError(EGL_BAD_PARAMETER, EGL_FALSE); egl_image_t* image = get_image(img); @@ -1349,7 +1349,7 @@ EGLSyncKHR eglCreateSyncKHR(EGLDisplay dpy, EGLenum type, const EGLint *attrib_l if (!dp) return EGL_NO_SYNC_KHR; EGLContext ctx = eglGetCurrentContext(); - ContextRef _c(ctx); + ContextRef _c(dp, ctx); if (!_c.get()) return setError(EGL_BAD_CONTEXT, EGL_NO_SYNC_KHR); @@ -1372,12 +1372,12 @@ EGLBoolean eglDestroySyncKHR(EGLDisplay dpy, EGLSyncKHR sync) egl_display_t const * const dp = validate_display(dpy); if (!dp) return EGL_FALSE; - SyncRef _s(sync); + SyncRef _s(dp, sync); if (!_s.get()) return setError(EGL_BAD_PARAMETER, EGL_FALSE); egl_sync_t* syncObject = get_sync(sync); EGLContext ctx = syncObject->context; - ContextRef _c(ctx); + ContextRef _c(dp, ctx); if (!_c.get()) return setError(EGL_BAD_CONTEXT, EGL_FALSE); @@ -1399,12 +1399,12 @@ EGLint eglClientWaitSyncKHR(EGLDisplay dpy, EGLSyncKHR sync, EGLint flags, EGLTi egl_display_t const * const dp = validate_display(dpy); if (!dp) return EGL_FALSE; - SyncRef _s(sync); + SyncRef _s(dp, sync); if (!_s.get()) return setError(EGL_BAD_PARAMETER, EGL_FALSE); egl_sync_t* syncObject = get_sync(sync); EGLContext ctx = syncObject->context; - ContextRef _c(ctx); + ContextRef _c(dp, ctx); if (!_c.get()) return setError(EGL_BAD_CONTEXT, EGL_FALSE); @@ -1424,13 +1424,13 @@ EGLBoolean eglGetSyncAttribKHR(EGLDisplay dpy, EGLSyncKHR sync, EGLint attribute egl_display_t const * const dp = validate_display(dpy); if (!dp) return EGL_FALSE; - SyncRef _s(sync); + SyncRef _s(dp, sync); if (!_s.get()) return setError(EGL_BAD_PARAMETER, EGL_FALSE); egl_sync_t* syncObject = get_sync(sync); EGLContext ctx = syncObject->context; - ContextRef _c(ctx); + ContextRef _c(dp, ctx); if (!_c.get()) return setError(EGL_BAD_CONTEXT, EGL_FALSE); @@ -1455,7 +1455,7 @@ EGLBoolean eglSetSwapRectangleANDROID(EGLDisplay dpy, EGLSurface draw, egl_display_t const * const dp = validate_display(dpy); if (!dp) return EGL_FALSE; - SurfaceRef _s(draw); + SurfaceRef _s(dp, draw); if (!_s.get()) return setError(EGL_BAD_SURFACE, EGL_FALSE); diff --git a/opengl/libs/EGL/egl_display.cpp b/opengl/libs/EGL/egl_display.cpp index 558ca77ff..2935832ad 100644 --- a/opengl/libs/EGL/egl_display.cpp +++ b/opengl/libs/EGL/egl_display.cpp @@ -62,11 +62,13 @@ void egl_display_t::removeObject(egl_object_t* object) { objects.remove(object); } -bool egl_display_t::getObject(egl_object_t* object) { +bool egl_display_t::getObject(egl_object_t* object) const { Mutex::Autolock _l(lock); if (objects.indexOf(object) >= 0) { - object->incRef(); - return true; + if (object->getDisplay() == this) { + object->incRef(); + return true; + } } return false; } diff --git a/opengl/libs/EGL/egl_display.h b/opengl/libs/EGL/egl_display.h index e0a367d4c..94077bea1 100644 --- a/opengl/libs/EGL/egl_display.h +++ b/opengl/libs/EGL/egl_display.h @@ -81,7 +81,7 @@ public: // remove object from this display's list void removeObject(egl_object_t* object); // add reference to this object. returns true if this is a valid object. - bool getObject(egl_object_t* object); + bool getObject(egl_object_t* object) const; static egl_display_t* get(EGLDisplay dpy); @@ -119,9 +119,9 @@ public: egl_config_t* configs; private: - uint32_t refs; - Mutex lock; - SortedVector objects; + uint32_t refs; + mutable Mutex lock; + SortedVector objects; }; // ---------------------------------------------------------------------------- diff --git a/opengl/libs/EGL/egl_object.cpp b/opengl/libs/EGL/egl_object.cpp index dbf9a010e..20cdc7eb4 100644 --- a/opengl/libs/EGL/egl_object.cpp +++ b/opengl/libs/EGL/egl_object.cpp @@ -55,10 +55,10 @@ void egl_object_t::destroy() { } } -bool egl_object_t::get() { +bool egl_object_t::get(egl_display_t const* display, egl_object_t* object) { // used by LocalRef, this does an incRef() atomically with // checking that the object is valid. - return display->getObject(this); + return display->getObject(object); } // ---------------------------------------------------------------------------- diff --git a/opengl/libs/EGL/egl_object.h b/opengl/libs/EGL/egl_object.h index 46f7139ad..df1b261dd 100644 --- a/opengl/libs/EGL/egl_object.h +++ b/opengl/libs/EGL/egl_object.h @@ -52,10 +52,11 @@ public: inline int32_t incRef() { return android_atomic_inc(&count); } inline int32_t decRef() { return android_atomic_dec(&count); } + inline egl_display_t* getDisplay() const { return display; } private: void terminate(); - bool get(); + static bool get(egl_display_t const* display, egl_object_t* object); public: template @@ -66,9 +67,9 @@ public: public: ~LocalRef(); explicit LocalRef(egl_object_t* rhs); - explicit LocalRef(T o) : ref(0) { + explicit LocalRef(egl_display_t const* display, T o) : ref(0) { egl_object_t* native = reinterpret_cast(o); - if (o && native->get()) { + if (o && egl_object_t::get(display, native)) { ref = native; } } From ec200fadc525e57234f4ee56b40b1b249d34a71b Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Wed, 16 Nov 2011 15:59:13 -0800 Subject: [PATCH 8/8] Revert "enable ALLOW_DEQUEUE_CURRENT_BUFFER for tegra devices" This reverts commit e7758be6da85728df6b4215f413660c67c5a9740. Seemed to cause failures un SurfaceTexture. Bug: 5627450 --- libs/gui/Android.mk | 4 ---- libs/gui/SurfaceTexture.cpp | 10 +++------- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/libs/gui/Android.mk b/libs/gui/Android.mk index 9767568be..ed319f5e9 100644 --- a/libs/gui/Android.mk +++ b/libs/gui/Android.mk @@ -32,10 +32,6 @@ LOCAL_SHARED_LIBRARIES := \ LOCAL_MODULE:= libgui -ifeq ($(TARGET_BOARD_PLATFORM), tegra) - LOCAL_CFLAGS += -DALLOW_DEQUEUE_CURRENT_BUFFER -endif - include $(BUILD_SHARED_LIBRARY) ifeq (,$(ONE_SHOT_MAKEFILE)) diff --git a/libs/gui/SurfaceTexture.cpp b/libs/gui/SurfaceTexture.cpp index 374f3c524..6f842067c 100644 --- a/libs/gui/SurfaceTexture.cpp +++ b/libs/gui/SurfaceTexture.cpp @@ -36,12 +36,8 @@ #include #include -#ifdef ALLOW_DEQUEUE_CURRENT_BUFFER -#define FLAG_ALLOW_DEQUEUE_CURRENT_BUFFER true -#warning "ALLOW_DEQUEUE_CURRENT_BUFFER enabled" -#else -#define FLAG_ALLOW_DEQUEUE_CURRENT_BUFFER false -#endif + +#define ALLOW_DEQUEUE_CURRENT_BUFFER false // Macros for including the SurfaceTexture name in log messages #define ST_LOGV(x, ...) LOGV("[%s] "x, mName.string(), ##__VA_ARGS__) @@ -327,7 +323,7 @@ status_t SurfaceTexture::dequeueBuffer(int *outBuf, uint32_t w, uint32_t h, LOGW_IF((state == BufferSlot::FREE) && (mCurrentTexture==i), "dequeueBuffer: buffer %d is both FREE and current!", i); - if (FLAG_ALLOW_DEQUEUE_CURRENT_BUFFER) { + if (ALLOW_DEQUEUE_CURRENT_BUFFER) { if (state == BufferSlot::FREE || i == mCurrentTexture) { foundSync = i; if (i != mCurrentTexture) {