From 08da0bf17caeb564d7fe7a0b08b057df2f8ca45e Mon Sep 17 00:00:00 2001 From: Jesse Hall Date: Tue, 31 Jul 2012 12:16:31 -0700 Subject: [PATCH 1/4] Revert "Minimal changes to support multi-display HWC" This reverts commit bddd724b178b1263c16e41b564165fcd0e93ff83 Change-Id: Ib7db20b5b3de4779b6e173473a33976ae398abd4 --- opengl/tests/hwc/hwcColorEquiv.cpp | 12 ++-- opengl/tests/hwc/hwcCommit.cpp | 4 +- opengl/tests/hwc/hwcRects.cpp | 8 +-- opengl/tests/hwc/hwcStress.cpp | 8 +-- opengl/tests/hwc/hwcTestLib.cpp | 16 ++--- opengl/tests/hwc/hwcTestLib.h | 10 +-- .../DisplayHardware/HWComposer.cpp | 62 ++++++------------- .../DisplayHardware/HWComposer.h | 4 +- 8 files changed, 47 insertions(+), 77 deletions(-) diff --git a/opengl/tests/hwc/hwcColorEquiv.cpp b/opengl/tests/hwc/hwcColorEquiv.cpp index 160906ddb..ab5277eec 100644 --- a/opengl/tests/hwc/hwcColorEquiv.cpp +++ b/opengl/tests/hwc/hwcColorEquiv.cpp @@ -344,9 +344,9 @@ main(int argc, char *argv[]) hwcTestFillColorHBlend(equivFrame.get(), refFormat->format, startRefColor, endRefColor); - hwc_display_contents_1_t *list; - size_t size = sizeof(hwc_display_contents_1_t) + numFrames * sizeof(hwc_layer_1_t); - if ((list = (hwc_display_contents_1_t *) calloc(1, size)) == NULL) { + hwc_layer_list_1_t *list; + size_t size = sizeof(hwc_layer_list_1_t) + numFrames * sizeof(hwc_layer_1_t); + if ((list = (hwc_layer_list_1_t *) calloc(1, size)) == NULL) { testPrintE("Allocate list failed"); exit(11); } @@ -383,7 +383,7 @@ main(int argc, char *argv[]) // Perform prepare operation if (verbose) { testPrintI("Prepare:"); hwcTestDisplayList(list); } - hwcDevice->prepare(hwcDevice, 1, &list); + hwcDevice->prepare(hwcDevice, list); if (verbose) { testPrintI("Post Prepare:"); hwcTestDisplayListPrepareModifiable(list); @@ -393,9 +393,7 @@ main(int argc, char *argv[]) list->flags &= ~HWC_GEOMETRY_CHANGED; if (verbose) {hwcTestDisplayListHandles(list); } - list->dpy = dpy; - list->sur = surface; - hwcDevice->set(hwcDevice, 1, &list); + hwcDevice->set(hwcDevice, dpy, surface, list); testDelay(endDelay); diff --git a/opengl/tests/hwc/hwcCommit.cpp b/opengl/tests/hwc/hwcCommit.cpp index 3681fbbe9..d4873d8dc 100644 --- a/opengl/tests/hwc/hwcCommit.cpp +++ b/opengl/tests/hwc/hwcCommit.cpp @@ -1397,7 +1397,7 @@ void Rational::double2Rational(double f, Range nRange, Range dRange, // Given a list of rectangles, determine how many HWC will commit to render uint32_t numOverlays(list& rectList) { - hwc_display_contents_1_t *hwcList; + hwc_layer_list_1_t *hwcList; list > buffers; hwcList = hwcTestCreateLayerList(rectList.size()); @@ -1430,7 +1430,7 @@ uint32_t numOverlays(list& rectList) // Perform prepare operation if (verbose) { testPrintI("Prepare:"); hwcTestDisplayList(hwcList); } - hwcDevice->prepare(hwcDevice, 1, &hwcList); + hwcDevice->prepare(hwcDevice, hwcList); if (verbose) { testPrintI("Post Prepare:"); hwcTestDisplayListPrepareModifiable(hwcList); diff --git a/opengl/tests/hwc/hwcRects.cpp b/opengl/tests/hwc/hwcRects.cpp index ec0403f6e..e2f003980 100644 --- a/opengl/tests/hwc/hwcRects.cpp +++ b/opengl/tests/hwc/hwcRects.cpp @@ -307,7 +307,7 @@ main(int argc, char *argv[]) } // Create list of frames - hwc_display_contents_1_t *list; + hwc_layer_list_1_t *list; list = hwcTestCreateLayerList(rectangle.size()); if (list == NULL) { testPrintE("hwcTestCreateLayerList failed"); @@ -329,7 +329,7 @@ main(int argc, char *argv[]) // Perform prepare operation if (verbose) { testPrintI("Prepare:"); hwcTestDisplayList(list); } - hwcDevice->prepare(hwcDevice, 1, &list); + hwcDevice->prepare(hwcDevice, list); if (verbose) { testPrintI("Post Prepare:"); hwcTestDisplayListPrepareModifiable(list); @@ -341,9 +341,7 @@ main(int argc, char *argv[]) // Perform the set operation(s) if (verbose) {testPrintI("Set:"); } if (verbose) { hwcTestDisplayListHandles(list); } - list->dpy = dpy; - list->sur = surface; - hwcDevice->set(hwcDevice, 1, &list); + hwcDevice->set(hwcDevice, dpy, surface, list); testDelay(endDelay); diff --git a/opengl/tests/hwc/hwcStress.cpp b/opengl/tests/hwc/hwcStress.cpp index 3e8ea8dba..ccc732843 100644 --- a/opengl/tests/hwc/hwcStress.cpp +++ b/opengl/tests/hwc/hwcStress.cpp @@ -409,7 +409,7 @@ main(int argc, char *argv[]) // generated for this pass. srand48(pass); - hwc_display_contents_1_t *list; + hwc_layer_list_1_t *list; list = hwcTestCreateLayerList(testRandMod(frames.size()) + 1); if (list == NULL) { testPrintE("hwcTestCreateLayerList failed"); @@ -478,7 +478,7 @@ main(int argc, char *argv[]) // Perform prepare operation if (verbose) { testPrintI("Prepare:"); hwcTestDisplayList(list); } - hwcDevice->prepare(hwcDevice, 1, &list); + hwcDevice->prepare(hwcDevice, list); if (verbose) { testPrintI("Post Prepare:"); hwcTestDisplayListPrepareModifiable(list); @@ -491,9 +491,7 @@ main(int argc, char *argv[]) if (verbose) {testPrintI("Set:"); } for (unsigned int n1 = 0; n1 < numSet; n1++) { if (verbose) { hwcTestDisplayListHandles(list); } - list->dpy = dpy; - list->sur = surface; - hwcDevice->set(hwcDevice, 1, &list); + hwcDevice->set(hwcDevice, dpy, surface, list); // Prandomly select a new set of handles for (unsigned int n1 = 0; n1 < list->numHwLayers; n1++) { diff --git a/opengl/tests/hwc/hwcTestLib.cpp b/opengl/tests/hwc/hwcTestLib.cpp index d567e6eb3..c6dbe9dcb 100644 --- a/opengl/tests/hwc/hwcTestLib.cpp +++ b/opengl/tests/hwc/hwcTestLib.cpp @@ -399,12 +399,12 @@ const char *hwcTestGraphicFormat2str(uint32_t format) * Dynamically creates layer list with numLayers worth * of hwLayers entries. */ -hwc_display_contents_1_t *hwcTestCreateLayerList(size_t numLayers) +hwc_layer_list_1_t *hwcTestCreateLayerList(size_t numLayers) { - hwc_display_contents_1_t *list; + hwc_layer_list_1_t *list; - size_t size = sizeof(hwc_display_contents_1_t) + numLayers * sizeof(hwc_layer_1_t); - if ((list = (hwc_display_contents_1_t *) calloc(1, size)) == NULL) { + size_t size = sizeof(hwc_layer_list_1_t) + numLayers * sizeof(hwc_layer_1_t); + if ((list = (hwc_layer_list_1_t *) calloc(1, size)) == NULL) { return NULL; } list->flags = HWC_GEOMETRY_CHANGED; @@ -417,13 +417,13 @@ hwc_display_contents_1_t *hwcTestCreateLayerList(size_t numLayers) * hwcTestFreeLayerList * Frees memory previous allocated via hwcTestCreateLayerList(). */ -void hwcTestFreeLayerList(hwc_display_contents_1_t *list) +void hwcTestFreeLayerList(hwc_layer_list_1_t *list) { free(list); } // Display the settings of the layer list pointed to by list -void hwcTestDisplayList(hwc_display_contents_1_t *list) +void hwcTestDisplayList(hwc_layer_list_1_t *list) { testPrintI(" flags: %#x%s", list->flags, (list->flags & HWC_GEOMETRY_CHANGED) ? " GEOMETRY_CHANGED" : ""); @@ -494,7 +494,7 @@ void hwcTestDisplayList(hwc_display_contents_1_t *list) * Displays the portions of a list that are meant to be modified by * a prepare call. */ -void hwcTestDisplayListPrepareModifiable(hwc_display_contents_1_t *list) +void hwcTestDisplayListPrepareModifiable(hwc_layer_list_1_t *list) { uint32_t numOverlays = 0; for (unsigned int layer = 0; layer < list->numHwLayers; layer++) { @@ -522,7 +522,7 @@ void hwcTestDisplayListPrepareModifiable(hwc_display_contents_1_t *list) * * Displays the handles of all the graphic buffers in the list. */ -void hwcTestDisplayListHandles(hwc_display_contents_1_t *list) +void hwcTestDisplayListHandles(hwc_layer_list_1_t *list) { const unsigned int maxLayersPerLine = 6; diff --git a/opengl/tests/hwc/hwcTestLib.h b/opengl/tests/hwc/hwcTestLib.h index d7d5837ba..db3f5c12f 100644 --- a/opengl/tests/hwc/hwcTestLib.h +++ b/opengl/tests/hwc/hwcTestLib.h @@ -113,11 +113,11 @@ const struct hwcTestGraphicFormat *hwcTestGraphicFormatLookup(uint32_t id); const char *hwcTestGraphicFormat2str(uint32_t format); std::string hwcTestRect2str(const struct hwc_rect& rect); -hwc_display_contents_1_t *hwcTestCreateLayerList(size_t numLayers); -void hwcTestFreeLayerList(hwc_display_contents_1_t *list); -void hwcTestDisplayList(hwc_display_contents_1_t *list); -void hwcTestDisplayListPrepareModifiable(hwc_display_contents_1_t *list); -void hwcTestDisplayListHandles(hwc_display_contents_1_t *list); +hwc_layer_list_1_t *hwcTestCreateLayerList(size_t numLayers); +void hwcTestFreeLayerList(hwc_layer_list_1_t *list); +void hwcTestDisplayList(hwc_layer_list_1_t *list); +void hwcTestDisplayListPrepareModifiable(hwc_layer_list_1_t *list); +void hwcTestDisplayListHandles(hwc_layer_list_1_t *list); uint32_t hwcTestColor2Pixel(uint32_t format, ColorFract color, float alpha); void hwcTestColorConvert(uint32_t fromFormat, uint32_t toFormat, diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp index 843815951..0a633f01a 100644 --- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp +++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp @@ -57,12 +57,12 @@ namespace android { // can just use the v1.0 pointer without branches or casts. #if HWC_REMOVE_DEPRECATED_VERSIONS -// We need complete types to satisfy semantic checks, even though the code -// paths that use these won't get executed at runtime (and will likely be dead- -// code-eliminated). When we remove the code to support v0.3 we can remove +// We need complete types with to satisfy semantic checks, even though the +// code paths that use these won't get executed at runtime (and will likely be +// dead-code-eliminated). When we remove the code to support v0.3 we can remove // these as well. typedef hwc_layer_1_t hwc_layer_t; -typedef hwc_display_contents_1_t hwc_layer_list_t; +typedef hwc_layer_list_1_t hwc_layer_list_t; typedef hwc_composer_device_1_t hwc_composer_device_t; #endif @@ -80,7 +80,7 @@ static bool hwcHasVersion(const hwc_composer_device_1_t* hwc, uint32_t version) static size_t sizeofHwcLayerList(const hwc_composer_device_1_t* hwc, size_t numLayers) { if (hwcHasVersion(hwc, HWC_DEVICE_API_VERSION_1_0)) { - return sizeof(hwc_display_contents_1_t) + numLayers*sizeof(hwc_layer_1_t); + return sizeof(hwc_layer_list_1_t) + numLayers*sizeof(hwc_layer_1_t); } else { return sizeof(hwc_layer_list_t) + numLayers*sizeof(hwc_layer_t); } @@ -136,17 +136,11 @@ HWComposer::HWComposer( } if (mHwc) { - // always turn vsync off when we start - needVSyncThread = false; - if (hwcHasVersion(mHwc, HWC_DEVICE_API_VERSION_1_0)) { - mHwc->methods->eventControl(mHwc, 0, HWC_EVENT_VSYNC, 0); - } else if (hwcHasVersion(mHwc, HWC_DEVICE_API_VERSION_0_3)) { - hwc_composer_device_t* hwc0 = (hwc_composer_device_t*)mHwc; - err = hwc0->methods->eventControl(hwc0, HWC_EVENT_VSYNC, 0); - } else { - needVSyncThread = true; + if (hwcHasVersion(mHwc, HWC_DEVICE_API_VERSION_0_3)) { + // always turn vsync off when we start + mHwc->methods->eventControl(mHwc, HWC_EVENT_VSYNC, 0); + needVSyncThread = false; } - if (mHwc->registerProcs) { mCBContext->hwc = this; mCBContext->procs.invalidate = &hook_invalidate; @@ -200,12 +194,7 @@ void HWComposer::eventControl(int event, int enabled) { status_t err = NO_ERROR; if (mHwc && mHwc->common.version >= HWC_DEVICE_API_VERSION_0_3) { if (!mDebugForceFakeVSync) { - if (hwcHasVersion(mHwc, HWC_DEVICE_API_VERSION_1_0)) { - err = mHwc->methods->eventControl(mHwc, 0, event, enabled); - } else { - hwc_composer_device_t* hwc0 = (hwc_composer_device_t*)mHwc; - err = hwc0->methods->eventControl(hwc0, event, enabled); - } + err = mHwc->methods->eventControl(mHwc, event, enabled); // error here should not happen -- not sure what we should // do if it does. ALOGE_IF(err, "eventControl(%d, %d) failed %s", @@ -228,9 +217,8 @@ status_t HWComposer::createWorkList(size_t numLayers) { if (!mList || mCapacity < numLayers) { free(mList); size_t size = sizeofHwcLayerList(mHwc, numLayers); - mList = (hwc_display_contents_1_t*)malloc(size); + mList = (hwc_layer_list_1_t*)malloc(size); mCapacity = numLayers; - mList->flipFenceFd = -1; } mList->flags = HWC_GEOMETRY_CHANGED; mList->numHwLayers = numLayers; @@ -239,8 +227,7 @@ status_t HWComposer::createWorkList(size_t numLayers) { } status_t HWComposer::prepare() const { - int err = mHwc->prepare(mHwc, 1, - const_cast(&mList)); + int err = mHwc->prepare(mHwc, mList); if (err == NO_ERROR) { size_t numOVLayers = 0; size_t numFBLayers = 0; @@ -286,17 +273,9 @@ size_t HWComposer::getLayerCount(int type) const { status_t HWComposer::commit() const { int err = NO_ERROR; if (mHwc) { - if (mList) { - mList->dpy = mDpy; - mList->sur = mSur; - } - err = mHwc->set(mHwc, 1, const_cast(&mList)); + err = mHwc->set(mHwc, mDpy, mSur, mList); if (mList) { mList->flags &= ~HWC_GEOMETRY_CHANGED; - if (mList->flipFenceFd != -1) { - close(mList->flipFenceFd); - mList->flipFenceFd = -1; - } } } else { eglSwapBuffers(mDpy, mSur); @@ -306,20 +285,17 @@ status_t HWComposer::commit() const { status_t HWComposer::release() const { if (mHwc) { - if (hwcHasVersion(mHwc, HWC_DEVICE_API_VERSION_1_0)) { - mHwc->methods->eventControl(mHwc, 0, HWC_EVENT_VSYNC, 0); - } else if (hwcHasVersion(mHwc, HWC_DEVICE_API_VERSION_0_3)) { - hwc_composer_device_t* hwc0 = (hwc_composer_device_t*)mHwc; - hwc0->methods->eventControl(hwc0, HWC_EVENT_VSYNC, 0); + if (hwcHasVersion(mHwc, HWC_DEVICE_API_VERSION_0_3)) { + mHwc->methods->eventControl(mHwc, HWC_EVENT_VSYNC, 0); } - int err = mHwc->set(mHwc, 0, NULL); + int err = mHwc->set(mHwc, NULL, NULL, NULL); if (err < 0) { return (status_t)err; } if (hwcHasVersion(mHwc, HWC_DEVICE_API_VERSION_1_0)) { if (mHwc->methods && mHwc->methods->blank) { - err = mHwc->methods->blank(mHwc, 0, 1); + err = mHwc->methods->blank(mHwc, 1); } } return (status_t)err; @@ -331,7 +307,7 @@ status_t HWComposer::acquire() const { if (mHwc) { if (hwcHasVersion(mHwc, HWC_DEVICE_API_VERSION_1_0)) { if (mHwc->methods && mHwc->methods->blank) { - int err = mHwc->methods->blank(mHwc, 0, 0); + int err = mHwc->methods->blank(mHwc, 0); return (status_t)err; } } @@ -344,7 +320,7 @@ status_t HWComposer::disable() { if (mHwc) { free(mList); mList = NULL; - int err = mHwc->prepare(mHwc, 0, NULL); + int err = mHwc->prepare(mHwc, NULL); return (status_t)err; } return NO_ERROR; diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.h b/services/surfaceflinger/DisplayHardware/HWComposer.h index 87711096d..c2fff4fd9 100644 --- a/services/surfaceflinger/DisplayHardware/HWComposer.h +++ b/services/surfaceflinger/DisplayHardware/HWComposer.h @@ -36,7 +36,7 @@ extern "C" int clock_nanosleep(clockid_t clock_id, int flags, struct timespec *remain); struct hwc_composer_device_1; -struct hwc_display_contents_1; +struct hwc_layer_list_1; struct hwc_procs; namespace android { @@ -230,7 +230,7 @@ private: sp mFlinger; hw_module_t const* mModule; struct hwc_composer_device_1* mHwc; - struct hwc_display_contents_1* mList; + struct hwc_layer_list_1* mList; size_t mCapacity; mutable size_t mNumOVLayers; mutable size_t mNumFBLayers; From e5769db877e3ebc55195b4b53a95b51587d7b5b2 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Wed, 15 Aug 2012 13:46:03 -0700 Subject: [PATCH 2/4] we were mistakenly optimizing out SF's main transactions in some cases due to a typo, SF's main transaction was conditional to having a display transaction. more correct fix for 6970310 Bug: 6970310 Change-Id: Iafd8c4e02afa5db829cc1c65950cfcc74754c6af --- services/surfaceflinger/SurfaceFlinger.cpp | 55 ++++++++++++---------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 3a802ff80..9178dfcaf 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -891,8 +891,7 @@ void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags) * (perform the transaction for each of them if needed) */ - const bool layersNeedTransaction = transactionFlags & eTraversalNeeded; - if (layersNeedTransaction) { + if (transactionFlags & eTraversalNeeded) { for (size_t i=0 ; i& layer = currentLayers[i]; uint32_t trFlags = layer->getTransactionFlags(eTransactionNeeded); @@ -905,7 +904,7 @@ void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags) } /* - * Perform our own transaction if needed + * Perform display own transactions if needed */ if (transactionFlags & eDisplayTransactionNeeded) { @@ -978,31 +977,35 @@ void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags) } } } + } - if (currentLayers.size() > mDrawingState.layersSortedByZ.size()) { - // layers have been added - mVisibleRegionsDirty = true; - } + /* + * Perform our own transaction if needed + */ - // some layers might have been removed, so - // we need to update the regions they're exposing. - if (mLayersRemoved) { - mLayersRemoved = false; - mVisibleRegionsDirty = true; - const LayerVector& previousLayers(mDrawingState.layersSortedByZ); - const size_t count = previousLayers.size(); - for (size_t i=0 ; i& layer(previousLayers[i]); - if (currentLayers.indexOf(layer) < 0) { - // this layer is not visible anymore - // TODO: we could traverse the tree from front to back and - // compute the actual visible region - // TODO: we could cache the transformed region - Layer::State front(layer->drawingState()); - Region visibleReg = front.transform.transform( - Region(Rect(front.active.w, front.active.h))); - invalidateLayerStack(front.layerStack, visibleReg); - } + if (currentLayers.size() > mDrawingState.layersSortedByZ.size()) { + // layers have been added + mVisibleRegionsDirty = true; + } + + // some layers might have been removed, so + // we need to update the regions they're exposing. + if (mLayersRemoved) { + mLayersRemoved = false; + mVisibleRegionsDirty = true; + const LayerVector& previousLayers(mDrawingState.layersSortedByZ); + const size_t count = previousLayers.size(); + for (size_t i=0 ; i& layer(previousLayers[i]); + if (currentLayers.indexOf(layer) < 0) { + // this layer is not visible anymore + // TODO: we could traverse the tree from front to back and + // compute the actual visible region + // TODO: we could cache the transformed region + Layer::State front(layer->drawingState()); + Region visibleReg = front.transform.transform( + Region(Rect(front.active.w, front.active.h))); + invalidateLayerStack(front.layerStack, visibleReg); } } } From f3a4c96f7abfd43032f5648a26f683f9f5e45e70 Mon Sep 17 00:00:00 2001 From: Jamie Gennis Date: Mon, 20 Aug 2012 14:28:53 -0700 Subject: [PATCH 3/4] Revert "SurfaceTexture: inherit from ConsumerBase" This reverts commit ed059a8d754770c3cf28b78dba30f7a6ba475dbe Change-Id: I72542c2595771a40c2c88251e0d6eb54e305b99b --- include/gui/ConsumerBase.h | 8 + include/gui/SurfaceTexture.h | 113 ++++++++++---- libs/gui/ConsumerBase.cpp | 3 +- libs/gui/SurfaceTexture.cpp | 286 ++++++++++++++++++++++++----------- 4 files changed, 288 insertions(+), 122 deletions(-) diff --git a/include/gui/ConsumerBase.h b/include/gui/ConsumerBase.h index 1f643a32e..d2bf0f649 100644 --- a/include/gui/ConsumerBase.h +++ b/include/gui/ConsumerBase.h @@ -189,6 +189,14 @@ protected: // if none is supplied sp mBufferQueue; + // mAttached indicates whether the ConsumerBase is currently attached to + // an OpenGL ES context. For legacy reasons, this is initialized to true, + // indicating that the ConsumerBase is considered to be attached to + // whatever context is current at the time of the first updateTexImage call. + // It is set to false by detachFromContext, and then set to true again by + // attachToContext. + bool mAttached; + // mMutex is the mutex used to prevent concurrent access to the member // variables of ConsumerBase objects. It must be locked whenever the // member variables are accessed. diff --git a/include/gui/SurfaceTexture.h b/include/gui/SurfaceTexture.h index 0a83ce6af..66c390a84 100644 --- a/include/gui/SurfaceTexture.h +++ b/include/gui/SurfaceTexture.h @@ -24,7 +24,6 @@ #include #include -#include #include @@ -40,9 +39,20 @@ namespace android { class String8; -class SurfaceTexture : public ConsumerBase { +class SurfaceTexture : public virtual RefBase, + protected BufferQueue::ConsumerListener { public: - typedef ConsumerBase::FrameAvailableListener FrameAvailableListener; + struct FrameAvailableListener : public virtual RefBase { + // onFrameAvailable() is called each time an additional frame becomes + // available for consumption. This means that frames that are queued + // while in asynchronous mode only trigger the callback if no previous + // frames are pending. Frames queued while in synchronous mode always + // trigger the callback. + // + // This is called without any lock held and can be called concurrently + // by multiple threads. + virtual void onFrameAvailable() = 0; + }; // SurfaceTexture constructs a new SurfaceTexture object. tex indicates the // name of the OpenGL ES texture to which images are to be streamed. @@ -72,6 +82,8 @@ public: GLenum texTarget = GL_TEXTURE_EXTERNAL_OES, bool useFenceSync = true, const sp &bufferQueue = 0); + virtual ~SurfaceTexture(); + // updateTexImage sets the image contents of the target texture to that of // the most recently queued buffer. // @@ -120,6 +132,16 @@ public: // documented by the source. int64_t getTimestamp(); + // setFrameAvailableListener sets the listener object that will be notified + // when a new frame becomes available. + void setFrameAvailableListener(const sp& listener); + + // getAllocator retrieves the binder object that must be referenced as long + // as the GraphicBuffers dequeued from this SurfaceTexture are referenced. + // Holding this binder reference prevents SurfaceFlinger from freeing the + // buffers before the client is done with them. + sp getAllocator(); + // setDefaultBufferSize is used to set the size of buffers returned by // requestBuffers when a with and height of zero is requested. // A call to setDefaultBufferSize() may trigger requestBuffers() to @@ -158,6 +180,17 @@ public: // synchronous mode. bool isSynchronousMode() const; + // abandon frees all the buffers and puts the SurfaceTexture into the + // 'abandoned' state. Once put in this state the SurfaceTexture can never + // leave it. When in the 'abandoned' state, all methods of the + // ISurfaceTexture interface will fail with the NO_INIT error. + // + // Note that while calling this method causes all the buffers to be freed + // from the perspective of the the SurfaceTexture, if there are additional + // references on the buffers (e.g. if a buffer is referenced by a client or + // by OpenGL ES as a texture) then those buffer will remain allocated. + void abandon(); + // set the name of the SurfaceTexture that will be used to identify it in // log messages. void setName(const String8& name); @@ -171,9 +204,7 @@ public: // getBufferQueue returns the BufferQueue object to which this // SurfaceTexture is connected. - sp getBufferQueue() const { - return mBufferQueue; - } + sp getBufferQueue() const; // detachFromContext detaches the SurfaceTexture from the calling thread's // current OpenGL ES context. This context must be the same as the context @@ -202,25 +233,17 @@ public: // current at the time of the last call to detachFromContext. status_t attachToContext(GLuint tex); + // dump our state in a String + virtual void dump(String8& result) const; + virtual void dump(String8& result, const char* prefix, char* buffer, size_t SIZE) const; + protected: - // abandonLocked overrides the ConsumerBase method to clear - // mCurrentTextureBuf in addition to the ConsumerBase behavior. - virtual void abandonLocked(); - - // dumpLocked overrides the ConsumerBase method to dump SurfaceTexture- - // specific info in addition to the ConsumerBase behavior. - virtual void dumpLocked(String8& result, const char* prefix, char* buffer, - size_t size) const; - - // acquireBufferLocked overrides the ConsumerBase method to update the - // mEglSlots array in addition to the ConsumerBase behavior. - virtual status_t acquireBufferLocked(BufferQueue::BufferItem *item); - - // releaseBufferLocked overrides the ConsumerBase method to update the - // mEglSlots array in addition to the ConsumerBase. - virtual status_t releaseBufferLocked(int buf, EGLDisplay display, - EGLSyncKHR eglFence, const sp& fence); + // Implementation of the BufferQueue::ConsumerListener interface. These + // calls are used to notify the SurfaceTexture of asynchronous events in the + // BufferQueue. + virtual void onFrameAvailable(); + virtual void onBuffersReleased(); static bool isExternalFormat(uint32_t format); @@ -328,9 +351,11 @@ private: struct EGLSlot { EGLSlot() : mEglImage(EGL_NO_IMAGE_KHR), - mEglFence(EGL_NO_SYNC_KHR) { + mFence(EGL_NO_SYNC_KHR) { } + sp mGraphicBuffer; + // mEglImage is the EGLImage created from mGraphicBuffer. EGLImageKHR mEglImage; @@ -338,7 +363,14 @@ private: // associated with this buffer slot may be dequeued. It is initialized // to EGL_NO_SYNC_KHR when the buffer is created and (optionally, based // on a compile-time option) set to a new sync object in updateTexImage. - EGLSyncKHR mEglFence; + EGLSyncKHR mFence; + + // mReleaseFence is a fence which will signal when the buffer + // associated with this buffer slot is no longer being used by the + // consumer and can be overwritten. The buffer can be dequeued before + // the fence signals; the producer is responsible for delaying writes + // until it signals. + sp mReleaseFence; }; // mEglDisplay is the EGLDisplay with which this SurfaceTexture is currently @@ -360,7 +392,23 @@ private: // slot that has not yet been used. The buffer allocated to a slot will also // be replaced if the requested buffer usage or geometry differs from that // of the buffer allocated to a slot. - EGLSlot mEglSlots[BufferQueue::NUM_BUFFER_SLOTS]; + EGLSlot mEGLSlots[BufferQueue::NUM_BUFFER_SLOTS]; + + // mAbandoned indicates that the BufferQueue will no longer be used to + // consume images buffers pushed to it using the ISurfaceTexture interface. + // It is initialized to false, and set to true in the abandon method. A + // BufferQueue that has been abandoned will return the NO_INIT error from + // all ISurfaceTexture methods capable of returning an error. + bool mAbandoned; + + // mName is a string used to identify the SurfaceTexture in log messages. + // It can be set by the setName method. + String8 mName; + + // mFrameAvailableListener is the listener object that will be called when a + // new frame becomes available. If it is not NULL it will be called from + // queueBuffer. + sp mFrameAvailableListener; // mCurrentTexture is the buffer slot index of the buffer that is currently // bound to the OpenGL texture. It is initialized to INVALID_BUFFER_SLOT, @@ -370,13 +418,22 @@ private: // reset mCurrentTexture to INVALID_BUFFER_SLOT. int mCurrentTexture; - // mAttached indicates whether the ConsumerBase is currently attached to + // The SurfaceTexture has-a BufferQueue and is responsible for creating this object + // if none is supplied + sp mBufferQueue; + + // mAttached indicates whether the SurfaceTexture is currently attached to // an OpenGL ES context. For legacy reasons, this is initialized to true, - // indicating that the ConsumerBase is considered to be attached to + // indicating that the SurfaceTexture is considered to be attached to // whatever context is current at the time of the first updateTexImage call. // It is set to false by detachFromContext, and then set to true again by // attachToContext. bool mAttached; + + // mMutex is the mutex used to prevent concurrent access to the member + // variables of SurfaceTexture objects. It must be locked whenever the + // member variables are accessed. + mutable Mutex mMutex; }; // ---------------------------------------------------------------------------- diff --git a/libs/gui/ConsumerBase.cpp b/libs/gui/ConsumerBase.cpp index 17bbfd11e..af19ac081 100644 --- a/libs/gui/ConsumerBase.cpp +++ b/libs/gui/ConsumerBase.cpp @@ -53,8 +53,7 @@ static int32_t createProcessUniqueId() { } ConsumerBase::ConsumerBase(const sp& bufferQueue) : - mAbandoned(false), - mBufferQueue(bufferQueue) { + mBufferQueue(bufferQueue) { // Choose a name using the PID and a process-unique ID. mName = String8::format("unnamed-%d-%d", getpid(), createProcessUniqueId()); diff --git a/libs/gui/SurfaceTexture.cpp b/libs/gui/SurfaceTexture.cpp index 451ccc26e..66660810f 100644 --- a/libs/gui/SurfaceTexture.cpp +++ b/libs/gui/SurfaceTexture.cpp @@ -26,8 +26,6 @@ #include #include -#include - #include #include #include @@ -98,10 +96,14 @@ static float mtxRot270[16] = { static void mtxMul(float out[16], const float a[16], const float b[16]); +// Get an ID that's unique within this process. +static int32_t createProcessUniqueId() { + static volatile int32_t globalCounter = 0; + return android_atomic_inc(&globalCounter); +} SurfaceTexture::SurfaceTexture(GLuint tex, bool allowSynchronousMode, GLenum texTarget, bool useFenceSync, const sp &bufferQueue) : - ConsumerBase(bufferQueue == 0 ? new BufferQueue(allowSynchronousMode) : bufferQueue), mCurrentTransform(0), mCurrentTimestamp(0), mFilteringEnabled(true), @@ -114,15 +116,47 @@ SurfaceTexture::SurfaceTexture(GLuint tex, bool allowSynchronousMode, mTexTarget(texTarget), mEglDisplay(EGL_NO_DISPLAY), mEglContext(EGL_NO_CONTEXT), + mAbandoned(false), mCurrentTexture(BufferQueue::INVALID_BUFFER_SLOT), mAttached(true) { + // Choose a name using the PID and a process-unique ID. + mName = String8::format("unnamed-%d-%d", getpid(), createProcessUniqueId()); ST_LOGV("SurfaceTexture"); + if (bufferQueue == 0) { + ST_LOGV("Creating a new BufferQueue"); + mBufferQueue = new BufferQueue(allowSynchronousMode); + } + else { + mBufferQueue = bufferQueue; + } memcpy(mCurrentTransformMatrix, mtxIdentity, sizeof(mCurrentTransformMatrix)); - mBufferQueue->setConsumerUsageBits(DEFAULT_USAGE_FLAGS); + // Note that we can't create an sp<...>(this) in a ctor that will not keep a + // reference once the ctor ends, as that would cause the refcount of 'this' + // dropping to 0 at the end of the ctor. Since all we need is a wp<...> + // that's what we create. + wp listener; + sp proxy; + listener = static_cast(this); + proxy = new BufferQueue::ProxyConsumerListener(listener); + + status_t err = mBufferQueue->consumerConnect(proxy); + if (err != NO_ERROR) { + ST_LOGE("SurfaceTexture: error connecting to BufferQueue: %s (%d)", + strerror(-err), err); + } else { + mBufferQueue->setConsumerName(mName); + mBufferQueue->setConsumerUsageBits(DEFAULT_USAGE_FLAGS); + } +} + +SurfaceTexture::~SurfaceTexture() { + ST_LOGV("~SurfaceTexture"); + + abandon(); } status_t SurfaceTexture::setBufferCountServer(int bufferCount) { @@ -143,42 +177,6 @@ status_t SurfaceTexture::updateTexImage() { return SurfaceTexture::updateTexImage(NULL); } -status_t SurfaceTexture::acquireBufferLocked(BufferQueue::BufferItem *item) { - status_t err = ConsumerBase::acquireBufferLocked(item); - if (err != NO_ERROR) { - return err; - } - - int slot = item->mBuf; - if (item->mGraphicBuffer != NULL) { - if (mEglSlots[slot].mEglImage != EGL_NO_IMAGE_KHR) { - eglDestroyImageKHR(mEglDisplay, mEglSlots[slot].mEglImage); - mEglSlots[slot].mEglImage = EGL_NO_IMAGE_KHR; - } - } - - // Update the GL texture object. We may have to do this even when - // item.mGraphicBuffer == NULL, if we destroyed the EGLImage when - // detaching from a context but the buffer has not been re-allocated. - EGLImageKHR image = createImage(mEglDisplay, mSlots[slot].mGraphicBuffer); - if (image == EGL_NO_IMAGE_KHR) { - return UNKNOWN_ERROR; - } - mEglSlots[slot].mEglImage = image; - - return NO_ERROR; -} - -status_t SurfaceTexture::releaseBufferLocked(int buf, EGLDisplay display, - EGLSyncKHR eglFence, const sp& fence) { - status_t err = ConsumerBase::releaseBufferLocked(buf, mEglDisplay, - eglFence, fence); - - mEglSlots[mCurrentTexture].mEglFence = EGL_NO_SYNC_KHR; - - return err; -} - status_t SurfaceTexture::updateTexImage(BufferRejecter* rejecter) { ATRACE_CALL(); ST_LOGV("updateTexImage"); @@ -219,65 +217,97 @@ status_t SurfaceTexture::updateTexImage(BufferRejecter* rejecter) { // In asynchronous mode the list is guaranteed to be one buffer // deep, while in synchronous mode we use the oldest buffer. - err = acquireBufferLocked(&item); + err = mBufferQueue->acquireBuffer(&item); if (err == NO_ERROR) { int buf = item.mBuf; + // This buffer was newly allocated, so we need to clean up on our side + if (item.mGraphicBuffer != NULL) { + mEGLSlots[buf].mGraphicBuffer = 0; + if (mEGLSlots[buf].mEglImage != EGL_NO_IMAGE_KHR) { + eglDestroyImageKHR(dpy, mEGLSlots[buf].mEglImage); + mEGLSlots[buf].mEglImage = EGL_NO_IMAGE_KHR; + } + mEGLSlots[buf].mGraphicBuffer = item.mGraphicBuffer; + } // we call the rejecter here, in case the caller has a reason to // not accept this buffer. this is used by SurfaceFlinger to // reject buffers which have the wrong size - if (rejecter && rejecter->reject(mSlots[buf].mGraphicBuffer, item)) { - releaseBufferLocked(buf, dpy, EGL_NO_SYNC_KHR, item.mFence); + if (rejecter && rejecter->reject(mEGLSlots[buf].mGraphicBuffer, item)) { + mBufferQueue->releaseBuffer(buf, dpy, EGL_NO_SYNC_KHR, item.mFence); glBindTexture(mTexTarget, mTexName); return NO_ERROR; } - GLint error; - while ((error = glGetError()) != GL_NO_ERROR) { - ST_LOGW("updateTexImage: clearing GL error: %#04x", error); - } - - EGLImageKHR image = mEglSlots[buf].mEglImage; - glBindTexture(mTexTarget, mTexName); - glEGLImageTargetTexture2DOES(mTexTarget, (GLeglImageOES)image); - - while ((error = glGetError()) != GL_NO_ERROR) { - ST_LOGE("updateTexImage: error binding external texture image %p " - "(slot %d): %#04x", image, buf, error); - err = UNKNOWN_ERROR; + // Update the GL texture object. We may have to do this even when + // item.mGraphicBuffer == NULL, if we destroyed the EGLImage when + // detaching from a context but the buffer has not been re-allocated. + EGLImageKHR image = mEGLSlots[buf].mEglImage; + if (image == EGL_NO_IMAGE_KHR) { + if (mEGLSlots[buf].mGraphicBuffer == NULL) { + ST_LOGE("updateTexImage: buffer at slot %d is null", buf); + err = BAD_VALUE; + } else { + image = createImage(dpy, mEGLSlots[buf].mGraphicBuffer); + mEGLSlots[buf].mEglImage = image; + if (image == EGL_NO_IMAGE_KHR) { + // NOTE: if dpy was invalid, createImage() is guaranteed to + // fail. so we'd end up here. + err = UNKNOWN_ERROR; + } + } } if (err == NO_ERROR) { - err = syncForReleaseLocked(dpy); + GLint error; + while ((error = glGetError()) != GL_NO_ERROR) { + ST_LOGW("updateTexImage: clearing GL error: %#04x", error); + } + + glBindTexture(mTexTarget, mTexName); + glEGLImageTargetTexture2DOES(mTexTarget, (GLeglImageOES)image); + + while ((error = glGetError()) != GL_NO_ERROR) { + ST_LOGE("updateTexImage: error binding external texture image %p " + "(slot %d): %#04x", image, buf, error); + err = UNKNOWN_ERROR; + } + + if (err == NO_ERROR) { + err = syncForReleaseLocked(dpy); + } } if (err != NO_ERROR) { // Release the buffer we just acquired. It's not safe to // release the old buffer, so instead we just drop the new frame. - releaseBufferLocked(buf, dpy, EGL_NO_SYNC_KHR, item.mFence); + mBufferQueue->releaseBuffer(buf, dpy, EGL_NO_SYNC_KHR, item.mFence); return err; } ST_LOGV("updateTexImage: (slot=%d buf=%p) -> (slot=%d buf=%p)", mCurrentTexture, mCurrentTextureBuf != NULL ? mCurrentTextureBuf->handle : 0, - buf, mSlots[buf].mGraphicBuffer->handle); + buf, item.mGraphicBuffer != NULL ? item.mGraphicBuffer->handle : 0); // release old buffer if (mCurrentTexture != BufferQueue::INVALID_BUFFER_SLOT) { - status_t status = releaseBufferLocked(mCurrentTexture, dpy, - mEglSlots[mCurrentTexture].mEglFence, - mSlots[mCurrentTexture].mFence); - if (status != NO_ERROR && status != BufferQueue::STALE_BUFFER_SLOT) { - ST_LOGE("updateTexImage: failed to release buffer: %s (%d)", - strerror(-status), status); + status_t status = mBufferQueue->releaseBuffer(mCurrentTexture, dpy, + mEGLSlots[mCurrentTexture].mFence, + mEGLSlots[mCurrentTexture].mReleaseFence); + mEGLSlots[mCurrentTexture].mFence = EGL_NO_SYNC_KHR; + mEGLSlots[mCurrentTexture].mReleaseFence.clear(); + if (status == BufferQueue::STALE_BUFFER_SLOT) { + freeBufferLocked(mCurrentTexture); + } else if (status != NO_ERROR) { + ST_LOGE("updateTexImage: released invalid buffer"); err = status; } } // Update the SurfaceTexture state. mCurrentTexture = buf; - mCurrentTextureBuf = mSlots[buf].mGraphicBuffer; + mCurrentTextureBuf = mEGLSlots[buf].mGraphicBuffer; mCurrentCrop = item.mCrop; mCurrentTransform = item.mTransform; mCurrentScalingMode = item.mScalingMode; @@ -300,20 +330,20 @@ void SurfaceTexture::setReleaseFence(int fenceFd) { sp fence(new Fence(fenceFd)); if (fenceFd == -1 || mCurrentTexture == BufferQueue::INVALID_BUFFER_SLOT) return; - if (!mSlots[mCurrentTexture].mFence.get()) { - mSlots[mCurrentTexture].mFence = fence; + if (!mEGLSlots[mCurrentTexture].mReleaseFence.get()) { + mEGLSlots[mCurrentTexture].mReleaseFence = fence; } else { sp mergedFence = Fence::merge( String8("SurfaceTexture merged release"), - mSlots[mCurrentTexture].mFence, fence); + mEGLSlots[mCurrentTexture].mReleaseFence, fence); if (!mergedFence.get()) { ST_LOGE("failed to merge release fences"); // synchronization is broken, the best we can do is hope fences // signal in order so the new fence will act like a union - mSlots[mCurrentTexture].mFence = fence; + mEGLSlots[mCurrentTexture].mReleaseFence = fence; return; } - mSlots[mCurrentTexture].mFence = mergedFence; + mEGLSlots[mCurrentTexture].mReleaseFence = mergedFence; } } @@ -360,10 +390,10 @@ status_t SurfaceTexture::detachFromContext() { // SurfaceTexture gets attached to a new OpenGL ES context (and thus gets a // new EGLDisplay). for (int i =0; i < BufferQueue::NUM_BUFFER_SLOTS; i++) { - EGLImageKHR img = mEglSlots[i].mEglImage; + EGLImageKHR img = mEGLSlots[i].mEglImage; if (img != EGL_NO_IMAGE_KHR) { eglDestroyImageKHR(mEglDisplay, img); - mEglSlots[i].mEglImage = EGL_NO_IMAGE_KHR; + mEGLSlots[i].mEglImage = EGL_NO_IMAGE_KHR; } } @@ -451,7 +481,7 @@ status_t SurfaceTexture::syncForReleaseLocked(EGLDisplay dpy) { ST_LOGV("syncForReleaseLocked"); if (mUseFenceSync && mCurrentTexture != BufferQueue::INVALID_BUFFER_SLOT) { - EGLSyncKHR fence = mEglSlots[mCurrentTexture].mEglFence; + EGLSyncKHR fence = mEGLSlots[mCurrentTexture].mFence; if (fence != EGL_NO_SYNC_KHR) { // There is already a fence for the current slot. We need to wait // on that before replacing it with another fence to ensure that all @@ -479,7 +509,7 @@ status_t SurfaceTexture::syncForReleaseLocked(EGLDisplay dpy) { return UNKNOWN_ERROR; } glFlush(); - mEglSlots[mCurrentTexture].mEglFence = fence; + mEGLSlots[mCurrentTexture].mFence = fence; } return OK; @@ -577,12 +607,10 @@ void SurfaceTexture::computeCurrentTransformMatrix() { // only need to shrink by a half a pixel. shrinkAmount = 0.5; break; - default: // If we don't recognize the format, we must assume the // worst case (that we care about), which is YUV420. shrinkAmount = 1.0; - break; } } @@ -622,6 +650,13 @@ nsecs_t SurfaceTexture::getTimestamp() { return mCurrentTimestamp; } +void SurfaceTexture::setFrameAvailableListener( + const sp& listener) { + ST_LOGV("setFrameAvailableListener"); + Mutex::Autolock lock(mMutex); + mFrameAvailableListener = listener; +} + EGLImageKHR SurfaceTexture::createImage(EGLDisplay dpy, const sp& graphicBuffer) { EGLClientBuffer cbuf = (EGLClientBuffer)graphicBuffer->getNativeBuffer(); @@ -701,21 +736,35 @@ bool SurfaceTexture::isSynchronousMode() const { void SurfaceTexture::freeBufferLocked(int slotIndex) { ST_LOGV("freeBufferLocked: slotIndex=%d", slotIndex); + mEGLSlots[slotIndex].mGraphicBuffer = 0; if (slotIndex == mCurrentTexture) { mCurrentTexture = BufferQueue::INVALID_BUFFER_SLOT; } - EGLImageKHR img = mEglSlots[slotIndex].mEglImage; + EGLImageKHR img = mEGLSlots[slotIndex].mEglImage; if (img != EGL_NO_IMAGE_KHR) { ST_LOGV("destroying EGLImage dpy=%p img=%p", mEglDisplay, img); eglDestroyImageKHR(mEglDisplay, img); } - mEglSlots[slotIndex].mEglImage = EGL_NO_IMAGE_KHR; + mEGLSlots[slotIndex].mEglImage = EGL_NO_IMAGE_KHR; } -void SurfaceTexture::abandonLocked() { - ST_LOGV("abandonLocked"); - mCurrentTextureBuf.clear(); - ConsumerBase::abandonLocked(); +void SurfaceTexture::abandon() { + ST_LOGV("abandon"); + Mutex::Autolock lock(mMutex); + + if (!mAbandoned) { + mAbandoned = true; + mCurrentTextureBuf.clear(); + + // destroy all egl buffers + for (int i =0; i < BufferQueue::NUM_BUFFER_SLOTS; i++) { + freeBufferLocked(i); + } + + // disconnect from the BufferQueue + mBufferQueue->consumerDisconnect(); + mBufferQueue.clear(); + } } void SurfaceTexture::setName(const String8& name) { @@ -747,18 +796,71 @@ status_t SurfaceTexture::setSynchronousMode(bool enabled) { return mBufferQueue->setSynchronousMode(enabled); } -void SurfaceTexture::dumpLocked(String8& result, const char* prefix, - char* buffer, size_t size) const +// Used for refactoring, should not be in final interface +sp SurfaceTexture::getBufferQueue() const { + Mutex::Autolock lock(mMutex); + return mBufferQueue; +} + +void SurfaceTexture::onFrameAvailable() { + ST_LOGV("onFrameAvailable"); + + sp listener; + { // scope for the lock + Mutex::Autolock lock(mMutex); + listener = mFrameAvailableListener; + } + + if (listener != NULL) { + ST_LOGV("actually calling onFrameAvailable"); + listener->onFrameAvailable(); + } +} + +void SurfaceTexture::onBuffersReleased() { + ST_LOGV("onBuffersReleased"); + + Mutex::Autolock lock(mMutex); + + if (mAbandoned) { + // Nothing to do if we're already abandoned. + return; + } + + uint32_t mask = 0; + mBufferQueue->getReleasedBuffers(&mask); + for (int i = 0; i < BufferQueue::NUM_BUFFER_SLOTS; i++) { + if (mask & (1 << i)) { + freeBufferLocked(i); + } + } +} + +void SurfaceTexture::dump(String8& result) const { - snprintf(buffer, size, - "%smTexName=%d mCurrentTexture=%d\n" - "%smCurrentCrop=[%d,%d,%d,%d] mCurrentTransform=%#x\n", - prefix, mTexName, mCurrentTexture, prefix, mCurrentCrop.left, - mCurrentCrop.top, mCurrentCrop.right, mCurrentCrop.bottom, - mCurrentTransform); + char buffer[1024]; + dump(result, "", buffer, 1024); +} + +void SurfaceTexture::dump(String8& result, const char* prefix, + char* buffer, size_t SIZE) const +{ + Mutex::Autolock _l(mMutex); + snprintf(buffer, SIZE, "%smTexName=%d, mAbandoned=%d\n", prefix, mTexName, + int(mAbandoned)); result.append(buffer); - ConsumerBase::dumpLocked(result, prefix, buffer, size); + snprintf(buffer, SIZE, + "%snext : {crop=[%d,%d,%d,%d], transform=0x%02x, current=%d}\n", + prefix, mCurrentCrop.left, + mCurrentCrop.top, mCurrentCrop.right, mCurrentCrop.bottom, + mCurrentTransform, mCurrentTexture + ); + result.append(buffer); + + if (!mAbandoned) { + mBufferQueue->dump(result, prefix, buffer, SIZE); + } } static void mtxMul(float out[16], const float a[16], const float b[16]) { From bd589e324898ed77dff9cd3516d88ce4aca68352 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Mon, 20 Aug 2012 20:07:34 -0700 Subject: [PATCH 4/4] fix various issues in SF's EventThread - one issues caused most timestamps to be reported as 0 - on rare occasions an uninitialized variable could be used - vsync counts per connection were accessed unthreadsafely we now have 2 lists of connections in the main loop, one just keeps a list of strong refs to the connections because once we have a strong ref we're not allowed to release it while holding the lock. the 2nd list holds the connections that have a vsync event to be reported. all the calculations are made with the lock held. Change-Id: Iacfad3745b05df79d9ece3719bd4c34ddbfd5b83 --- services/surfaceflinger/EventThread.cpp | 101 ++++++++++++------------ services/surfaceflinger/EventThread.h | 1 - 2 files changed, 51 insertions(+), 51 deletions(-) diff --git a/services/surfaceflinger/EventThread.cpp b/services/surfaceflinger/EventThread.cpp index 3833f4839..59390c0ce 100644 --- a/services/surfaceflinger/EventThread.cpp +++ b/services/surfaceflinger/EventThread.cpp @@ -19,6 +19,8 @@ #include #include +#include + #include #include #include @@ -123,35 +125,53 @@ bool EventThread::threadLoop() { nsecs_t timestamp; size_t vsyncCount; - size_t activeEvents; DisplayEventReceiver::Event vsync; Vector< sp > activeConnections; + Vector< sp > signalConnections; do { + // release our references + signalConnections.clear(); + activeConnections.clear(); + Mutex::Autolock _l(mLock); + // latch VSYNC event if any + bool waitForVSync = false; + vsyncCount = mVSyncCount; timestamp = mVSyncTimestamp; mVSyncTimestamp = 0; - // check if we should be waiting for VSYNC events - activeEvents = 0; - bool waitForNextVsync = false; + // find out connections waiting for VSYNC events size_t count = mDisplayEventConnections.size(); for (size_t i=0 ; i connection(mDisplayEventConnections[i].promote()); if (connection != NULL) { activeConnections.add(connection); if (connection->count >= 0) { - // at least one continuous mode or active one-shot event - waitForNextVsync = true; - activeEvents++; - break; + // we need vsync events because at least + // one connection is waiting for it + waitForVSync = true; + if (connection->count == 0) { + // fired this time around + if (timestamp) { + // only "consume" this event if we're going to + // report it + connection->count = -1; + } + signalConnections.add(connection); + } else if (connection->count == 1 || + (vsyncCount % connection->count) == 0) { + // continuous event, and time to report it + signalConnections.add(connection); + } } } } if (timestamp) { - if (!waitForNextVsync) { + // we have a vsync event we can dispatch + if (!waitForVSync) { // we received a VSYNC but we have no clients // don't report it, and disable VSYNC events disableVSyncLocked(); @@ -164,14 +184,14 @@ bool EventThread::threadLoop() { // we'll wait to receive the event and we'll // reevaluate whether we need to dispatch it and/or // disable VSYNC events then. - if (waitForNextVsync) { + if (waitForVSync) { // enable enableVSyncLocked(); } } // wait for something to happen - if (mUseSoftwareVSync && waitForNextVsync) { + if (CC_UNLIKELY(mUseSoftwareVSync && waitForVSync)) { // h/w vsync cannot be used (screen is off), so we use // a timeout instead. it doesn't matter how imprecise this // is, we just need to make sure to serve the clients @@ -180,54 +200,35 @@ bool EventThread::threadLoop() { mVSyncCount++; } } else { - mCondition.wait(mLock); + if (!timestamp || signalConnections.isEmpty()) { + // This is where we spend most of our time, waiting + // for a vsync events and registered clients + mCondition.wait(mLock); + } } - vsyncCount = mVSyncCount; - } while (!activeConnections.size()); - - if (!activeEvents) { - // no events to return. start over. - // (here we make sure to exit the scope of this function - // so that we release our Connection references) - return true; - } + } while (!timestamp || signalConnections.isEmpty()); // dispatch vsync events to listeners... vsync.header.type = DisplayEventReceiver::DISPLAY_EVENT_VSYNC; vsync.header.timestamp = timestamp; vsync.vsync.count = vsyncCount; - const size_t count = activeConnections.size(); + const size_t count = signalConnections.size(); for (size_t i=0 ; i& conn(activeConnections[i]); + const sp& conn(signalConnections[i]); // now see if we still need to report this VSYNC event - const int32_t vcount = conn->count; - if (vcount >= 0) { - bool reportVsync = false; - if (vcount == 0) { - // fired this time around - conn->count = -1; - reportVsync = true; - } else if (vcount == 1 || (vsyncCount % vcount) == 0) { - // continuous event, and time to report it - reportVsync = true; - } - - if (reportVsync) { - status_t err = conn->postEvent(vsync); - if (err == -EAGAIN || err == -EWOULDBLOCK) { - // The destination doesn't accept events anymore, it's probably - // full. For now, we just drop the events on the floor. - // Note that some events cannot be dropped and would have to be - // re-sent later. Right-now we don't have the ability to do - // this, but it doesn't matter for VSYNC. - } else if (err < 0) { - // handle any other error on the pipe as fatal. the only - // reasonable thing to do is to clean-up this connection. - // The most common error we'll get here is -EPIPE. - removeDisplayEventConnection(activeConnections[i]); - } - } + status_t err = conn->postEvent(vsync); + if (err == -EAGAIN || err == -EWOULDBLOCK) { + // The destination doesn't accept events anymore, it's probably + // full. For now, we just drop the events on the floor. + // Note that some events cannot be dropped and would have to be + // re-sent later. Right-now we don't have the ability to do + // this, but it doesn't matter for VSYNC. + } else if (err < 0) { + // handle any other error on the pipe as fatal. the only + // reasonable thing to do is to clean-up this connection. + // The most common error we'll get here is -EPIPE. + removeDisplayEventConnection(signalConnections[i]); } } diff --git a/services/surfaceflinger/EventThread.h b/services/surfaceflinger/EventThread.h index d23b9fad9..aa0ea7f3c 100644 --- a/services/surfaceflinger/EventThread.h +++ b/services/surfaceflinger/EventThread.h @@ -47,7 +47,6 @@ class EventThread : public Thread { // count >= 1 : continuous event. count is the vsync rate // count == 0 : one-shot event that has not fired // count ==-1 : one-shot event that fired this round / disabled - // count ==-2 : one-shot event that fired the round before int32_t count; private: