From 08da0bf17caeb564d7fe7a0b08b057df2f8ca45e Mon Sep 17 00:00:00 2001 From: Jesse Hall Date: Tue, 31 Jul 2012 12:16:31 -0700 Subject: [PATCH 01/17] 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 02/17] 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 03/17] 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 04/17] 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: From 11676d963f3a5c0148ec4d764b5ab55ae9cd40aa Mon Sep 17 00:00:00 2001 From: Ramanan Rajeswaran Date: Wed, 22 Aug 2012 14:23:50 -0700 Subject: [PATCH 05/17] Revert "Added display initialization method" This reverts commit 3f3956236aac97b6aa25fa89f0983d5e9d065fdb Change-Id: Ia2a15d9a5db88add6019edf9d955cef1f73d432d --- services/surfaceflinger/SurfaceFlinger.cpp | 44 +++++----------------- services/surfaceflinger/SurfaceFlinger.h | 9 +---- 2 files changed, 11 insertions(+), 42 deletions(-) diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index aba701ca8..c63d0cf14 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -137,8 +137,15 @@ void SurfaceFlinger::binderDied(const wp& who) { // the window manager died on us. prepare its eulogy. - // restore initial conditions (default device unblank, etc) - initializeDisplays(); + // reset screen orientation + Vector state; + Vector displays; + DisplayState d; + d.what = DisplayState::eOrientationChanged; + d.token = mDefaultDisplays[DisplayDevice::DISPLAY_ID_MAIN]; + d.orientation = DisplayState::eOrientationDefault; + displays.add(d); + setTransactionState(state, displays, 0); // restart the boot-animation startBootAnim(); @@ -424,9 +431,6 @@ status_t SurfaceFlinger::readyToRun() // We're now ready to accept clients... mReadyToRunBarrier.open(); - // set initial conditions (e.g. unblank default device) - initializeDisplays(); - // start boot animation startBootAnim(); @@ -1697,36 +1701,6 @@ status_t SurfaceFlinger::onLayerDestroyed(const wp& layer) // --------------------------------------------------------------------------- -void SurfaceFlinger::onInitializeDisplays() { - // reset screen orientation - Vector state; - Vector displays; - DisplayState d; - d.what = DisplayState::eOrientationChanged; - d.token = mDefaultDisplays[DisplayDevice::DISPLAY_ID_MAIN]; - d.orientation = DisplayState::eOrientationDefault; - displays.add(d); - setTransactionState(state, displays, 0); - - // XXX: this should init default device to "unblank" and all other devices to "blank" - onScreenAcquired(); -} - -void SurfaceFlinger::initializeDisplays() { - class MessageScreenInitialized : public MessageBase { - SurfaceFlinger* flinger; - public: - MessageScreenInitialized(SurfaceFlinger* flinger) : flinger(flinger) { } - virtual bool handler() { - flinger->onInitializeDisplays(); - return true; - } - }; - sp msg = new MessageScreenInitialized(this); - postMessageAsync(msg); // we may be called from main thread, use async message -} - - void SurfaceFlinger::onScreenAcquired() { ALOGD("Screen about to return, flinger = %p", this); sp hw(getDefaultDisplayDevice()); // XXX: this should be per DisplayDevice diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index b1fe7383c..1f799068d 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -231,11 +231,9 @@ private: void signalLayerUpdate(); void signalRefresh(); - // called on the main thread in response to initializeDisplays() - void onInitializeDisplays(); - // called on the main thread in response to blank() + // called on the main thread in response to screenReleased() void onScreenReleased(); - // called on the main thread in response to unblank() + // called on the main thread in response to screenAcquired() void onScreenAcquired(); void handleMessageTransaction(); @@ -323,9 +321,6 @@ private: /* ------------------------------------------------------------------------ * Display and layer stack management */ - // called when starting, or restarting after system_server death - void initializeDisplays(); - sp getDisplayDevice(DisplayID dpy) const { return mDisplays.valueFor(dpy); } From d026a1e163c21bc229962f4662c3a1db7e0f443c Mon Sep 17 00:00:00 2001 From: Wink Saville Date: Tue, 28 Aug 2012 16:20:50 -0700 Subject: [PATCH 06/17] A vendor ril depends on a native screen shot code. Add a temporary shim until the vendor fixes the ril. Bug: 7073467 Change-Id: Ia95a58bd90677c03406c988d1c29ae785f8662f2 --- include/gui/SurfaceComposerClient.h | 5 +++++ libs/gui/SurfaceComposerClient.cpp | 8 ++++++++ 2 files changed, 13 insertions(+) diff --git a/include/gui/SurfaceComposerClient.h b/include/gui/SurfaceComposerClient.h index a143d812e..73214a415 100644 --- a/include/gui/SurfaceComposerClient.h +++ b/include/gui/SurfaceComposerClient.h @@ -148,6 +148,11 @@ class ScreenshotClient public: ScreenshotClient(); + // TODO: Remove me. Do not use. + // This is a compatibility shim for one product whose drivers are depending on + // this legacy function (when they shouldn't). + status_t update(); + // frees the previous screenshot and capture a new one status_t update(const sp& display); status_t update(const sp& display, diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index e4922a440..0ffa93253 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -558,6 +558,14 @@ ScreenshotClient::ScreenshotClient() : mWidth(0), mHeight(0), mFormat(PIXEL_FORMAT_NONE) { } +// TODO: Remove me. Do not use. +// This is a compatibility shim for one product whose drivers are depending on +// this legacy function (when they shouldn't). +status_t ScreenshotClient::update() { + sp sm(ComposerService::getComposerService()); + return update(sm->getBuiltInDisplay(0)); +} + status_t ScreenshotClient::update(const sp& display) { sp s(ComposerService::getComposerService()); if (s == NULL) return NO_INIT; From 6338654ed2e9f9816395d23e7b21cfc93fa94ac4 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Wed, 29 Aug 2012 16:59:24 -0700 Subject: [PATCH 07/17] we were sometimes not setting fences properly this would happen when the composition was handled entirely in h/w composer, in this case, we would not set the fences for any involved layers. Bug: 7049373 Change-Id: I1439dc156ce23c24041cdfbbebfe8ff4fdf790f8 --- services/surfaceflinger/SurfaceFlinger.cpp | 64 +++++++++++++--------- 1 file changed, 37 insertions(+), 27 deletions(-) diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 11624323e..2576d874f 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -1264,15 +1264,13 @@ void SurfaceFlinger::doDisplayComposition(const sp& hw, void SurfaceFlinger::doComposeSurfaces(const sp& hw, const Region& dirty) { + const int32_t id = hw->getHwcDisplayId(); HWComposer& hwc(getHwComposer()); - int32_t id = hw->getHwcDisplayId(); HWComposer::LayerListIterator cur = hwc.begin(id); const HWComposer::LayerListIterator end = hwc.end(id); - const bool hasGlesComposition = hwc.hasGlesComposition(id); - const bool hasHwcComposition = hwc.hasHwcComposition(id); - if (cur==end || hasGlesComposition) { - + const bool hasGlesComposition = hwc.hasGlesComposition(id) || (cur==end); + if (hasGlesComposition) { DisplayDevice::makeCurrent(hw, mEGLContext); // set the frame buffer @@ -1280,6 +1278,7 @@ void SurfaceFlinger::doComposeSurfaces(const sp& hw, const glLoadIdentity(); // Never touch the framebuffer if we don't have any framebuffer layers + const bool hasHwcComposition = hwc.hasHwcComposition(id); if (hasHwcComposition) { // when using overlays, we assume a fully transparent framebuffer // NOTE: we could reduce how much we need to clear, for instance @@ -1296,39 +1295,50 @@ void SurfaceFlinger::doComposeSurfaces(const sp& hw, const drawWormhole(hw, region); } } + } - /* - * and then, render the layers targeted at the framebuffer - */ + /* + * and then, render the layers targeted at the framebuffer + */ - const Vector< sp >& layers(hw->getVisibleLayersSortedByZ()); - const size_t count = layers.size(); - const Transform& tr = hw->getTransform(); - for (size_t i=0 ; i >& layers(hw->getVisibleLayersSortedByZ()); + const size_t count = layers.size(); + const Transform& tr = hw->getTransform(); + if (cur != end) { + // we're using h/w composer + for (size_t i=0 ; i& layer(layers[i]); const Region clip(dirty.intersect(tr.transform(layer->visibleRegion))); - if (cur != end) { - // we're using h/w composer - if (!clip.isEmpty()) { - if (cur->getCompositionType() == HWC_OVERLAY) { - if (i && (cur->getHints() & HWC_HINT_CLEAR_FB) - && layer->isOpaque()) { + if (!clip.isEmpty()) { + switch (cur->getCompositionType()) { + case HWC_OVERLAY: { + if ((cur->getHints() & HWC_HINT_CLEAR_FB) + && i + && layer->isOpaque() + && hasGlesComposition) { // never clear the very first layer since we're // guaranteed the FB is already cleared layer->clearWithOpenGL(hw, clip); } - } else { - layer->draw(hw, clip); + break; + } + case HWC_FRAMEBUFFER: { + layer->draw(hw, clip); + break; } - layer->setAcquireFence(hw, *cur); - } - ++cur; - } else { - // we're not using h/w composer - if (!clip.isEmpty()) { - layer->draw(hw, clip); } } + layer->setAcquireFence(hw, *cur); + } + } else { + // we're not using h/w composer + for (size_t i=0 ; i& layer(layers[i]); + const Region clip(dirty.intersect( + tr.transform(layer->visibleRegion))); + if (!clip.isEmpty()) { + layer->draw(hw, clip); + } } } } From 2cf855fbef641abdda69abe39005ea3d6c7830f0 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Wed, 5 Sep 2012 16:00:56 -0700 Subject: [PATCH 08/17] fix a problem where all hwc layers would have the SKIP flags set the problem was that LayerBase::setPerFrameData() was always setting this flag. in fact there was no reason to do this at that point since the layer is initialized to a default state in setGeometry(). Bug: 7111259 Change-Id: Ib37b0dd7391a6163070e9aca025512159c1705f9 --- services/surfaceflinger/DisplayHardware/HWComposer.cpp | 1 + services/surfaceflinger/LayerBase.cpp | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp index 23290e345..0034019c8 100644 --- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp +++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp @@ -546,6 +546,7 @@ public: getLayer()->compositionType = HWC_FRAMEBUFFER; getLayer()->hints = 0; getLayer()->flags = HWC_SKIP_LAYER; + getLayer()->handle = 0; getLayer()->transform = 0; getLayer()->blending = HWC_BLENDING_NONE; getLayer()->visibleRegionScreen.numRects = 0; diff --git a/services/surfaceflinger/LayerBase.cpp b/services/surfaceflinger/LayerBase.cpp index 87dc572aa..311d95fce 100644 --- a/services/surfaceflinger/LayerBase.cpp +++ b/services/surfaceflinger/LayerBase.cpp @@ -292,7 +292,6 @@ void LayerBase::setGeometry( void LayerBase::setPerFrameData(const sp& hw, HWComposer::HWCLayerInterface& layer) { - layer.setBuffer(0); // we have to set the visible region on every frame because // we currently free it during onLayerDisplayed(), which is called // after HWComposer::commit() -- every frame. From 02de802182b368f1a32373e8f7a2996b13f9ca5c Mon Sep 17 00:00:00 2001 From: Jesse Hall Date: Wed, 5 Sep 2012 13:03:10 -0700 Subject: [PATCH 09/17] Always reset layer acquireFenceFd after commit If SurfaceFlinger needs to refresh the screen but the dirty region is empty, it won't set the layer acquire fences, and stale file descriptors will be passed to HWC commit(). Now we make sure to clear the stale file descriptors for each layer right after commit(). Bug: 7078301 Change-Id: I6953ff91fc5488f105b30b07306f9c45a4c3f780 --- services/surfaceflinger/DisplayHardware/HWComposer.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp index 0034019c8..dca27baa2 100644 --- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp +++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp @@ -599,6 +599,8 @@ public: visibleRegion.numRects = 0; visibleRegion.rects = NULL; } + + getLayer()->acquireFenceFd = -1; } }; From ef27e9cea08c0f33cf64891a453510b0398d86e8 Mon Sep 17 00:00:00 2001 From: Jesse Hall Date: Tue, 18 Sep 2012 11:39:40 -0700 Subject: [PATCH 10/17] Check that HWC exists before trying to use it Bug: 7185810 Change-Id: I1271d6ba397f3abf0ef166b8d03b9b26b72e28d7 --- services/surfaceflinger/DisplayHardware/HWComposer.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp index 9c04fc035..8d0772269 100644 --- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp +++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp @@ -648,7 +648,7 @@ status_t HWComposer::acquire() const { } int HWComposer::getVisualID() const { - if (hwcHasApiVersion(mHwc, HWC_DEVICE_API_VERSION_1_1)) { + if (mHwc && hwcHasApiVersion(mHwc, HWC_DEVICE_API_VERSION_1_1)) { // FIXME: temporary hack until HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED // is supported by the implementation. we can only be in this case // if we have HWC 1.1 @@ -665,7 +665,7 @@ bool HWComposer::supportsFramebufferTarget() const { int HWComposer::fbPost(int32_t id, const sp& acquireFence, const sp& buffer) { - if (hwcHasApiVersion(mHwc, HWC_DEVICE_API_VERSION_1_1)) { + if (mHwc && hwcHasApiVersion(mHwc, HWC_DEVICE_API_VERSION_1_1)) { return setFramebufferTarget(id, acquireFence, buffer); } else { if (acquireFence != NULL) { @@ -676,7 +676,7 @@ int HWComposer::fbPost(int32_t id, } int HWComposer::fbCompositionComplete() { - if (hwcHasApiVersion(mHwc, HWC_DEVICE_API_VERSION_1_1)) + if (mHwc && hwcHasApiVersion(mHwc, HWC_DEVICE_API_VERSION_1_1)) return NO_ERROR; if (mFbDev->compositionComplete) { From e272b25aa9ec1ada8b69bb7fe71ca284a50571f0 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Tue, 25 Sep 2012 19:16:28 -0700 Subject: [PATCH 11/17] only abort when errors happen on the main display Bug: 7232690 Change-Id: I2c4b35a82f131da26deea738ef294e100e536d15 --- services/surfaceflinger/DisplayDevice.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp index 56852dae3..47dd0735d 100644 --- a/services/surfaceflinger/DisplayDevice.cpp +++ b/services/surfaceflinger/DisplayDevice.cpp @@ -228,11 +228,14 @@ void DisplayDevice::swapBuffers(HWComposer& hwc) const { } } - // TODO: we should at least handle EGL_CONTEXT_LOST, by recreating the - // context and resetting our state. - LOG_ALWAYS_FATAL_IF(!success, - "eglSwapBuffers(%p, %p) failed with 0x%8x", - mDisplay, mSurface, eglGetError()); + if (!success) { + EGLint error = eglGetError(); + if (error == EGL_CONTEXT_LOST || + mType == DisplayDevice::DISPLAY_PRIMARY) { + LOG_ALWAYS_FATAL("eglSwapBuffers(%p, %p) failed with 0x%08x", + mDisplay, mSurface, eglGetError()); + } + } } void DisplayDevice::onSwapBuffersCompleted(HWComposer& hwc) const { From 51af710d3be9fae5e33afb1c74331ea038ba14e4 Mon Sep 17 00:00:00 2001 From: Iliyan Malchev Date: Thu, 27 Sep 2012 15:08:29 -0700 Subject: [PATCH 12/17] Revert "Compatibility work around for bad graphics driver dependency." This reverts commit a50b51c03aca449920fc8581a738032a7bce7150 Change-Id: Ibdcd776a7f241dbb2475403ea04f939249774c41 --- include/gui/SurfaceComposerClient.h | 5 ----- libs/gui/SurfaceComposerClient.cpp | 9 --------- 2 files changed, 14 deletions(-) diff --git a/include/gui/SurfaceComposerClient.h b/include/gui/SurfaceComposerClient.h index 7a14c6a08..bae388615 100644 --- a/include/gui/SurfaceComposerClient.h +++ b/include/gui/SurfaceComposerClient.h @@ -67,11 +67,6 @@ public: // Get information about a display static status_t getDisplayInfo(const sp& display, DisplayInfo* info); - // TODO: Remove me. Do not use. - // This is a compatibility shim for one product whose drivers are depending on - // this legacy function (when they shouldn't). - static status_t getDisplayInfo(int32_t displayId, DisplayInfo* info); - // ------------------------------------------------------------------------ // surface creation / destruction diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index 9cf9cb52c..4165d01f7 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -552,15 +552,6 @@ status_t SurfaceComposerClient::getDisplayInfo( return ComposerService::getComposerService()->getDisplayInfo(display, info); } -// TODO: Remove me. Do not use. -// This is a compatibility shim for one product whose drivers are depending on -// this legacy function (when they shouldn't). -status_t SurfaceComposerClient::getDisplayInfo( - int32_t displayId, DisplayInfo* info) -{ - return getDisplayInfo(getBuiltInDisplay(displayId), info); -} - // ---------------------------------------------------------------------------- ScreenshotClient::ScreenshotClient() From 078da147f4113d73171d58f3811b937fd815362c Mon Sep 17 00:00:00 2001 From: Dianne Hackborn Date: Wed, 3 Oct 2012 09:43:01 -0700 Subject: [PATCH 13/17] Fix issue #7271589: Cannot set Ocean HD live wallpaper Change-Id: Id7662c503815293040c240232a6622bd6f6eab37 --- build/phone-xhdpi-2048-dalvik-heap.mk | 2 +- build/tablet-10in-xhdpi-2048-dalvik-heap.mk | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/build/phone-xhdpi-2048-dalvik-heap.mk b/build/phone-xhdpi-2048-dalvik-heap.mk index 8a3f6bbfd..042a6e69e 100644 --- a/build/phone-xhdpi-2048-dalvik-heap.mk +++ b/build/phone-xhdpi-2048-dalvik-heap.mk @@ -20,7 +20,7 @@ PRODUCT_PROPERTY_OVERRIDES += \ dalvik.vm.heapstartsize=8m \ dalvik.vm.heapgrowthlimit=192m \ - dalvik.vm.heapsize=768m \ + dalvik.vm.heapsize=512m \ dalvik.vm.heaptargetutilization=0.75 \ dalvik.vm.heapminfree=512k \ dalvik.vm.heapmaxfree=8m diff --git a/build/tablet-10in-xhdpi-2048-dalvik-heap.mk b/build/tablet-10in-xhdpi-2048-dalvik-heap.mk index b4d9cea23..1721fcc2c 100644 --- a/build/tablet-10in-xhdpi-2048-dalvik-heap.mk +++ b/build/tablet-10in-xhdpi-2048-dalvik-heap.mk @@ -19,7 +19,7 @@ PRODUCT_PROPERTY_OVERRIDES += \ dalvik.vm.heapstartsize=16m \ dalvik.vm.heapgrowthlimit=192m \ - dalvik.vm.heapsize=768m \ + dalvik.vm.heapsize=512m \ dalvik.vm.heaptargetutilization=0.75 \ dalvik.vm.heapminfree=512k \ dalvik.vm.heapmaxfree=8m From eb1caaea6970d2cf870d26cadb40871f6b9784ab Mon Sep 17 00:00:00 2001 From: Andy McFadden Date: Tue, 2 Oct 2012 19:04:45 -0700 Subject: [PATCH 14/17] Fix crashes after HDMI disconnect The display was being removed from SurfaceFlinger's list before we had a chance to reset HWComposer's layer list, so we were passing stale data into the hardware composer (which has its own per-display data). This resulted in "invalid gralloc handle" complaints. We now clear the layer list immediately after removing the display. The display was being removed while its EGLSurface was still "current", resulting in "cancelBuffer: BufferQueue has been abandoned" complaints. We now call makeCurrent on the primary display before removing the external display. Bug 7274254 Change-Id: Ia59e3a61d7ec46488b96bf93ec5e4ed3488b70e4 --- .../DisplayHardware/HWComposer.cpp | 16 ++++++++++++++++ .../surfaceflinger/DisplayHardware/HWComposer.h | 3 +++ services/surfaceflinger/SurfaceFlinger.cpp | 6 ++++++ 3 files changed, 25 insertions(+) diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp index fb93c477d..302ce8255 100644 --- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp +++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp @@ -540,8 +540,13 @@ status_t HWComposer::prepare() { // DO NOT reset the handle field to NULL, because it's possible // that we have nothing to redraw (eg: eglSwapBuffers() not called) // in which case, we should continue to use the same buffer. + LOG_FATAL_IF(disp.list == NULL); disp.framebufferTarget->compositionType = HWC_FRAMEBUFFER_TARGET; } + if (!disp.connected && disp.list != NULL) { + ALOGW("WARNING: disp %d: connected, non-null list, layers=%d", + i, disp.list->numHwLayers); + } mLists[i] = disp.list; if (mLists[i]) { if (hwcHasApiVersion(mHwc, HWC_DEVICE_API_VERSION_1_2)) { @@ -664,6 +669,17 @@ status_t HWComposer::acquire(int disp) const { return NO_ERROR; } +void HWComposer::disconnectDisplay(int disp) { + LOG_ALWAYS_FATAL_IF(disp < 0 || disp == HWC_DISPLAY_PRIMARY || + disp >= HWC_NUM_DISPLAY_TYPES); + DisplayData& dd(mDisplayData[disp]); + if (dd.list != NULL) { + free(dd.list); + dd.list = NULL; + dd.framebufferTarget = NULL; // points into dd.list + } +} + int HWComposer::getVisualID() const { if (mHwc && hwcHasApiVersion(mHwc, HWC_DEVICE_API_VERSION_1_1)) { // FIXME: temporary hack until HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.h b/services/surfaceflinger/DisplayHardware/HWComposer.h index 633ca9c13..af33999dc 100644 --- a/services/surfaceflinger/DisplayHardware/HWComposer.h +++ b/services/surfaceflinger/DisplayHardware/HWComposer.h @@ -97,6 +97,9 @@ public: // acquire hardware resources and unblank screen status_t acquire(int disp) const; + // reset state when an external, non-virtual display is disconnected + void disconnectDisplay(int disp); + // create a work list for numLayers layer. sets HWC_GEOMETRY_CHANGED. status_t createWorkList(int32_t id, size_t numLayers); diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index a8d20bb1b..ebec4cb82 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -1123,7 +1123,13 @@ void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags) if (j < 0) { // in drawing state but not in current state if (!draw[i].isMainDisplay()) { + // Call makeCurrent() on the primary display so we can + // be sure that nothing associated with this display + // is current. + const sp& hw(getDefaultDisplayDevice()); + DisplayDevice::makeCurrent(mEGLDisplay, hw, mEGLContext); mDisplays.removeItem(draw.keyAt(i)); + getHwComposer().disconnectDisplay(draw[i].type); } else { ALOGW("trying to remove the main display"); } From 947abbdd2cd6bf0143b659415ff67a6940d5c16f Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Thu, 4 Oct 2012 02:34:38 -0700 Subject: [PATCH 15/17] make sure we don't call into the HWC HAL when not needed when enabling/disabling vsync we now make sure to not call into the HAL if the state wouldn't change. Bug: 7274951 Change-Id: Ie24a6d68888a51b577acf9c2a973d85437cbacaf --- .../DisplayHardware/HWComposer.cpp | 48 +++++++++++++------ .../DisplayHardware/HWComposer.h | 13 +++-- services/surfaceflinger/EventThread.cpp | 4 +- services/surfaceflinger/SurfaceFlinger.cpp | 4 +- services/surfaceflinger/SurfaceFlinger.h | 2 +- 5 files changed, 47 insertions(+), 24 deletions(-) diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp index 302ce8255..597e5d630 100644 --- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp +++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp @@ -147,7 +147,7 @@ HWComposer::HWComposer( // don't need a vsync thread if we have a hardware composer needVSyncThread = false; // always turn vsync off when we start - mHwc->eventControl(mHwc, HWC_DISPLAY_PRIMARY, HWC_EVENT_VSYNC, 0); + eventControl(HWC_DISPLAY_PRIMARY, HWC_EVENT_VSYNC, 0); // these IDs are always reserved for (size_t i=0 ; ieventControl(mHwc, HWC_DISPLAY_PRIMARY, HWC_EVENT_VSYNC, 0); + eventControl(HWC_DISPLAY_PRIMARY, HWC_EVENT_VSYNC, 0); } if (mVSyncThread != NULL) { mVSyncThread->requestExitAndWait(); @@ -443,16 +443,32 @@ bool HWComposer::isConnected(int disp) const { return mDisplayData[disp].connected; } -void HWComposer::eventControl(int event, int enabled) { +void HWComposer::eventControl(int disp, int event, int enabled) { + if (uint32_t(disp)>31 || !mAllocatedDisplayIDs.hasBit(disp)) { + return; + } status_t err = NO_ERROR; - if (mHwc) { - if (!mDebugForceFakeVSync) { - err = mHwc->eventControl(mHwc, 0, event, enabled); - // error here should not happen -- not sure what we should - // do if it does. - ALOGE_IF(err, "eventControl(%d, %d) failed %s", - event, enabled, strerror(-err)); + if (mHwc && !mDebugForceFakeVSync) { + // NOTE: we use our own internal lock here because we have to call + // into the HWC with the lock held, and we want to make sure + // that even if HWC blocks (which it shouldn't), it won't + // affect other threads. + Mutex::Autolock _l(mEventControlLock); + const int32_t eventBit = 1UL << event; + const int32_t newValue = enabled ? eventBit : 0; + const int32_t oldValue = mDisplayData[disp].events & eventBit; + if (newValue != oldValue) { + ATRACE_CALL(); + err = mHwc->eventControl(mHwc, disp, event, enabled); + if (!err) { + int32_t& events(mDisplayData[disp].events); + events = (events & ~eventBit) | newValue; + } } + // error here should not happen -- not sure what we should + // do if it does. + ALOGE_IF(err, "eventControl(%d, %d) failed %s", + event, enabled, strerror(-err)); } if (err == NO_ERROR && mVSyncThread != NULL) { @@ -652,16 +668,16 @@ status_t HWComposer::commit() { return (status_t)err; } -status_t HWComposer::release(int disp) const { +status_t HWComposer::release(int disp) { LOG_FATAL_IF(disp >= HWC_NUM_DISPLAY_TYPES); if (mHwc) { - mHwc->eventControl(mHwc, disp, HWC_EVENT_VSYNC, 0); + eventControl(disp, HWC_EVENT_VSYNC, 0); return (status_t)mHwc->blank(mHwc, disp, 1); } return NO_ERROR; } -status_t HWComposer::acquire(int disp) const { +status_t HWComposer::acquire(int disp) { LOG_FATAL_IF(disp >= HWC_NUM_DISPLAY_TYPES); if (mHwc) { return (status_t)mHwc->blank(mHwc, disp, 0); @@ -965,8 +981,10 @@ HWComposer::VSyncThread::VSyncThread(HWComposer& hwc) void HWComposer::VSyncThread::setEnabled(bool enabled) { Mutex::Autolock _l(mLock); - mEnabled = enabled; - mCondition.signal(); + if (mEnabled != enabled) { + mEnabled = enabled; + mCondition.signal(); + } } void HWComposer::VSyncThread::onFirstRef() { diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.h b/services/surfaceflinger/DisplayHardware/HWComposer.h index af33999dc..269e1472a 100644 --- a/services/surfaceflinger/DisplayHardware/HWComposer.h +++ b/services/surfaceflinger/DisplayHardware/HWComposer.h @@ -92,10 +92,10 @@ public: status_t commit(); // release hardware resources and blank screen - status_t release(int disp) const; + status_t release(int disp); // acquire hardware resources and unblank screen - status_t acquire(int disp) const; + status_t acquire(int disp); // reset state when an external, non-virtual display is disconnected void disconnectDisplay(int disp); @@ -226,7 +226,7 @@ public: EVENT_VSYNC = HWC_EVENT_VSYNC }; - void eventControl(int event, int enabled); + void eventControl(int disp, int event, int enabled); // Query display parameters. Pass in a display index (e.g. // HWC_DISPLAY_PRIMARY). @@ -289,7 +289,7 @@ private: DisplayData() : xdpi(0), ydpi(0), refresh(0), connected(false), hasFbComp(false), hasOvComp(false), capacity(0), list(NULL), - framebufferTarget(NULL), fbTargetHandle(NULL) { } + framebufferTarget(NULL), fbTargetHandle(NULL), events(0) { } ~DisplayData() { free(list); } @@ -306,6 +306,8 @@ private: hwc_display_contents_1* list; hwc_layer_1* framebufferTarget; buffer_handle_t fbTargetHandle; + // protected by mEventControlLock + int32_t events; }; sp mFlinger; @@ -327,6 +329,9 @@ private: // protected by mLock mutable Mutex mLock; mutable nsecs_t mLastHwVSync; + + // thread-safe + mutable Mutex mEventControlLock; }; // --------------------------------------------------------------------------- diff --git a/services/surfaceflinger/EventThread.cpp b/services/surfaceflinger/EventThread.cpp index 6b7c78bbd..edb9fa580 100644 --- a/services/surfaceflinger/EventThread.cpp +++ b/services/surfaceflinger/EventThread.cpp @@ -308,14 +308,14 @@ Vector< sp > EventThread::waitForEvent( void EventThread::enableVSyncLocked() { if (!mUseSoftwareVSync) { // never enable h/w VSYNC when screen is off - mFlinger->eventControl(SurfaceFlinger::EVENT_VSYNC, true); + mFlinger->eventControl(HWC_DISPLAY_PRIMARY, SurfaceFlinger::EVENT_VSYNC, true); mPowerHAL.vsyncHint(true); } mDebugVsyncEnabled = true; } void EventThread::disableVSyncLocked() { - mFlinger->eventControl(SurfaceFlinger::EVENT_VSYNC, false); + mFlinger->eventControl(HWC_DISPLAY_PRIMARY, SurfaceFlinger::EVENT_VSYNC, false); mPowerHAL.vsyncHint(false); mDebugVsyncEnabled = false; } diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index ebec4cb82..8160a7aba 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -777,8 +777,8 @@ void SurfaceFlinger::onHotplugReceived(int type, bool connected) { } } -void SurfaceFlinger::eventControl(int event, int enabled) { - getHwComposer().eventControl(event, enabled); +void SurfaceFlinger::eventControl(int disp, int event, int enabled) { + getHwComposer().eventControl(disp, event, enabled); } void SurfaceFlinger::onMessageReceived(int32_t what) { diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 82728484a..5bb370368 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -123,7 +123,7 @@ public: // enable/disable h/w composer event // TODO: this should be made accessible only to EventThread - void eventControl(int event, int enabled); + void eventControl(int disp, int event, int enabled); // called on the main thread by MessageQueue when an internal message // is received From 3e9f3b67b17afa4d064ac36a5bf5d9eeb1991b2f Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Fri, 5 Oct 2012 17:28:04 -0700 Subject: [PATCH 16/17] ugly, temporary, workaroung for a problem where a binder thread spins forever Bug: 7289992 Change-Id: I0c3d482a1af57e5f444be2ba7f2751ac3e954af2 --- libs/binder/IPCThreadState.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/libs/binder/IPCThreadState.cpp b/libs/binder/IPCThreadState.cpp index 6e83faab7..03c10828d 100644 --- a/libs/binder/IPCThreadState.cpp +++ b/libs/binder/IPCThreadState.cpp @@ -482,6 +482,18 @@ void IPCThreadState::joinThreadPool(bool isMain) if(result == TIMED_OUT && !isMain) { break; } + + // HACK HACK HACK HACK HACK HACK HACK HACK HACK HACK HACK HACK HACK + // FIXME: we sometimes get unexplained EINVAL which causes this + // thread to spin forever. TEMPORARILY allow it to exit. + // We should probably assert on eng builds + if(result == -EINVAL && !isMain) { + ALOGE("**** THREAD %p (PID %d) ERROR (%d) LEAVING THE THREAD POOL\n", + (void*)pthread_self(), getpid(), result); + break; + } + // HACK HACK HACK HACK HACK HACK HACK HACK HACK HACK HACK HACK HACK + } while (result != -ECONNREFUSED && result != -EBADF); LOG_THREADPOOL("**** THREAD %p (PID %d) IS LEAVING THE THREAD POOL err=%p\n", From 1b10e25356fed2d9d3480485638e737332be6ef7 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Wed, 24 Oct 2012 16:29:17 -0700 Subject: [PATCH 17/17] partially implement external display clipping we perform external display clipping only on the GL side (ie: not done on the h/w composer side, which is harder and would be too risky). in practice this means that WFD will be clipped properly, while HDMI *may* or may not depending on how hwc is used. Bug: 7149437 Change-Id: I92d4d04220db72b6ffb134c7fa7a93af569723a5 --- services/surfaceflinger/SurfaceFlinger.cpp | 25 ++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 26e9c6054..7ee6e5ead 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -1500,6 +1500,28 @@ void SurfaceFlinger::doComposeSurfaces(const sp& hw, const drawWormhole(hw, region); } } + + if (hw->getDisplayType() >= DisplayDevice::DISPLAY_EXTERNAL) { + // TODO: just to be on the safe side, we don't set the + // scissor on the main display. It should never be needed + // anyways (though in theory it could since the API allows it). + const Rect& bounds(hw->getBounds()); + const Transform& tr(hw->getTransform()); + const Rect scissor(tr.transform(hw->getViewport())); + if (scissor != bounds) { + // scissor doesn't match the screen's dimensions, so we + // need to clear everything outside of it and enable + // the GL scissor so we don't draw anything where we shouldn't + const GLint height = hw->getHeight(); + glScissor(scissor.left, height - scissor.bottom, + scissor.getWidth(), scissor.getHeight()); + // clear everything unscissored + glClearColor(0, 0, 0, 0); + glClear(GL_COLOR_BUFFER_BIT); + // enable scissor for this frame + glEnable(GL_SCISSOR_TEST); + } + } } /* @@ -1552,6 +1574,9 @@ void SurfaceFlinger::doComposeSurfaces(const sp& hw, const } } } + + // disable scissor at the end of the frame + glDisable(GL_SCISSOR_TEST); } void SurfaceFlinger::drawWormhole(const sp& hw,