From 8499e93f4ddb19bb66c7ab57add1d6d803c4fc26 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Wed, 25 Sep 2013 20:40:07 -0700 Subject: [PATCH 1/5] fix crashers with wifi/virtual displays Bug: 10647742 Change-Id: I4b8ed9da52ef95af3a3b3a04b98514a3776a674d --- .../DisplayHardware/VirtualDisplaySurface.cpp | 11 +++++++++++ services/surfaceflinger/SurfaceFlinger.cpp | 6 ++++++ 2 files changed, 17 insertions(+) diff --git a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp index c06043d4f..88ef3928c 100644 --- a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp +++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp @@ -258,6 +258,17 @@ status_t VirtualDisplaySurface::dequeueBuffer(int* pslot, sp* fence, bool Source source = fbSourceForCompositionType(mCompositionType); if (source == SOURCE_SINK) { + + if (mOutputProducerSlot < 0) { + // Last chance bailout if something bad happened earlier. For example, + // in a GLES configuration, if the sink disappears then dequeueBuffer + // will fail, the GLES driver won't queue a buffer, but SurfaceFlinger + // will soldier on. So we end up here without a buffer. There should + // be lots of scary messages in the log just before this. + VDS_LOGE("dequeueBuffer: no buffer, bailing out"); + return NO_MEMORY; + } + // We already dequeued the output buffer. If the GLES driver wants // something incompatible, we have to cancel and get a new one. This // will mean that HWC will see a different output buffer between diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 879c91187..f83cc0669 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -963,6 +963,12 @@ void SurfaceFlinger::postFramebuffer() hwc.commit(); } + // make the default display current because the VirtualDisplayDevice code cannot + // deal with dequeueBuffer() being called outside of the composition loop; however + // the code below can call glFlush() which is allowed (and does in some case) call + // dequeueBuffer(). + getDefaultDisplayDevice()->makeCurrent(mEGLDisplay, mEGLContext); + for (size_t dpy=0 ; dpy hw(mDisplays[dpy]); const Vector< sp >& currentLayers(hw->getVisibleLayersSortedByZ()); From 6f2bc240e96e82058135f0eccf08594e4301ef3d Mon Sep 17 00:00:00 2001 From: Jesse Hall Date: Fri, 27 Sep 2013 22:10:47 -0700 Subject: [PATCH 2/5] Fix two EGLConfig selection bugs This fixes two bugs introduced by Change-Id: Ia8cc084c02a0e3de910def024da8a08d02bbd89d (a) There is no invalid EGLConfig value, in particular zero is valid. Checking return values of eglGetConfigs and eglChooseConfig is the only way to determine success. (b) The "simple" EGLConfig query used as the emulator fallback should not include EGL_RECORDABLE; the emulator doesn't have it. Bug: 10935622 Change-Id: Ib798a24e7cf06a679811c46eaa45d39174a715ec --- services/surfaceflinger/SurfaceFlinger.cpp | 39 ++++++++++------------ services/surfaceflinger/SurfaceFlinger.h | 4 +-- 2 files changed, 20 insertions(+), 23 deletions(-) diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index f83cc0669..4f139feda 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -366,12 +366,10 @@ public: operator EGLint const* () const { return &mList.keyAt(0).v; } }; -EGLConfig SurfaceFlinger::selectEGLConfig(EGLDisplay display, EGLint nativeVisualId, - EGLint renderableType) { +status_t SurfaceFlinger::selectEGLConfig(EGLDisplay display, EGLint nativeVisualId, + EGLint renderableType, EGLConfig* config) { // select our EGLConfig. It must support EGL_RECORDABLE_ANDROID if // it is to be used with WIFI displays - EGLConfig config; - EGLint dummy; status_t err; EGLint wantedAttribute; EGLint wantedAttributeValue; @@ -390,22 +388,18 @@ EGLConfig SurfaceFlinger::selectEGLConfig(EGLDisplay display, EGLint nativeVisua } else { // if no renderable type specified, fallback to a simplified query - attribs[EGL_RECORDABLE_ANDROID] = EGL_TRUE; wantedAttribute = EGL_NATIVE_VISUAL_ID; wantedAttributeValue = nativeVisualId; } err = selectConfigForAttribute(display, attribs, wantedAttribute, - wantedAttributeValue, &config); - if (!err) - goto success; - - return 0; - -success: - if (eglGetConfigAttrib(display, config, EGL_CONFIG_CAVEAT, &dummy)) - ALOGW_IF(dummy == EGL_SLOW_CONFIG, "EGL_SLOW_CONFIG selected!"); - return config; + wantedAttributeValue, config); + if (err == NO_ERROR) { + EGLint caveat; + if (eglGetConfigAttrib(display, *config, EGL_CONFIG_CAVEAT, &caveat)) + ALOGW_IF(caveat == EGL_SLOW_CONFIG, "EGL_SLOW_CONFIG selected!"); + } + return err; } void SurfaceFlinger::init() { @@ -413,6 +407,7 @@ void SurfaceFlinger::init() { ALOGI( "SurfaceFlinger's main thread ready to run. " "Initializing graphics H/W..."); + status_t err; Mutex::Autolock _l(mStateLock); // initialize EGL for the default display @@ -425,21 +420,23 @@ void SurfaceFlinger::init() { *static_cast(this)); // First try to get an ES2 config - mEGLConfig = selectEGLConfig(mEGLDisplay, mHwc->getVisualID(), EGL_OPENGL_ES2_BIT); + err = selectEGLConfig(mEGLDisplay, mHwc->getVisualID(), EGL_OPENGL_ES2_BIT, + &mEGLConfig); - if (!mEGLConfig) { + if (err != NO_ERROR) { // If ES2 fails, try ES1 - mEGLConfig = selectEGLConfig(mEGLDisplay, mHwc->getVisualID(), EGL_OPENGL_ES_BIT); + err = selectEGLConfig(mEGLDisplay, mHwc->getVisualID(), + EGL_OPENGL_ES_BIT, &mEGLConfig); } - if (!mEGLConfig) { + if (err != NO_ERROR) { // still didn't work, probably because we're on the emulator... // try a simplified query ALOGW("no suitable EGLConfig found, trying a simpler query"); - mEGLConfig = selectEGLConfig(mEGLDisplay, mHwc->getVisualID(), 0); + err = selectEGLConfig(mEGLDisplay, mHwc->getVisualID(), 0, &mEGLConfig); } - if (!mEGLConfig) { + if (err != NO_ERROR) { // this EGL is too lame for android LOG_ALWAYS_FATAL("no suitable EGLConfig found, giving up"); } diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 989e439ba..0e9955c53 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -317,8 +317,8 @@ private: */ static status_t selectConfigForAttribute(EGLDisplay dpy, EGLint const* attrs, EGLint attribute, EGLint value, EGLConfig* outConfig); - static EGLConfig selectEGLConfig(EGLDisplay disp, EGLint visualId, - EGLint renderableType); + static status_t selectEGLConfig(EGLDisplay disp, EGLint visualId, + EGLint renderableType, EGLConfig* config); size_t getMaxTextureSize() const; size_t getMaxViewportDims() const; From 8eb7a481abbbc3cfe713b43158f1913f958357cf Mon Sep 17 00:00:00 2001 From: Jesse Hall Date: Mon, 30 Sep 2013 16:49:30 -0700 Subject: [PATCH 3/5] Treat composition frames with no layers as using GLES composition When there are no window layers for a display, SurfaceFlinger clears the undefined region using GLES. Some of the places that check for GLES composition weren't considering this special case, in particular: - We were skipping the eglSwapBuffers() on these frames. - We were putting VirtualDisplaySurface in HWC-only composition mode. This change centralizes the logic for this special case. Bug: 10957068 Change-Id: I2deaf2ed101e8ea76708862a6bb67751b6078794 --- services/surfaceflinger/DisplayHardware/HWComposer.cpp | 9 +++++++++ services/surfaceflinger/SurfaceFlinger.cpp | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp index c851a2c06..7132b2f1b 100644 --- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp +++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp @@ -611,6 +611,10 @@ status_t HWComposer::prepare() { // here we're just making sure that "skip" layers are set // to HWC_FRAMEBUFFER and we're also counting how many layers // we have of each type. + // + // If there are no window layers, we treat the display has having FB + // composition, because SurfaceFlinger will use GLES to draw the + // wormhole region. for (size_t i=0 ; inumHwLayers == (disp.framebufferTarget ? 1 : 0)) { + disp.hasFbComp = true; + } + } else { + disp.hasFbComp = true; } } } diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 4f139feda..b5b0f2c00 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -1524,7 +1524,7 @@ void SurfaceFlinger::doComposeSurfaces(const sp& hw, const HWComposer::LayerListIterator cur = hwc.begin(id); const HWComposer::LayerListIterator end = hwc.end(id); - const bool hasGlesComposition = hwc.hasGlesComposition(id) || (cur==end); + bool hasGlesComposition = hwc.hasGlesComposition(id); if (hasGlesComposition) { if (!hw->makeCurrent(mEGLDisplay, mEGLContext)) { ALOGW("DisplayDevice::makeCurrent failed. Aborting surface composition for display %s", From d5350dea05571218290c583a9c482749bda9f7cc Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Tue, 1 Oct 2013 15:36:52 -0700 Subject: [PATCH 4/5] only clear FB when asked for the opaque layer a layer need to be considered NOT opaque if it has a plane-alpha. Bug: 10846930 Change-Id: Ibd8981b63ede4560c7096bacc4cff46a7eb2a8bb --- services/surfaceflinger/SurfaceFlinger.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index b5b0f2c00..e3745489b 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -1597,9 +1597,10 @@ void SurfaceFlinger::doComposeSurfaces(const sp& hw, const if (!clip.isEmpty()) { switch (cur->getCompositionType()) { case HWC_OVERLAY: { + const Layer::State& state(layer->getDrawingState()); if ((cur->getHints() & HWC_HINT_CLEAR_FB) && i - && layer->isOpaque() + && layer->isOpaque() && (state.alpha == 0xFF) && hasGlesComposition) { // never clear the very first layer since we're // guaranteed the FB is already cleared From 062019b8ceafe55dc4045de78c3c291a22a6b459 Mon Sep 17 00:00:00 2001 From: Jesse Hall Date: Tue, 1 Oct 2013 17:25:20 -0700 Subject: [PATCH 5/5] Set the outbuf acquire fence after we actually have it. In GLES-only mode, we don't have the outbuf acquire fence until after GLES composition is done for the frame. We were setting the fence in HWC's state immediately after dequeueing the buffer from the consumer, before GLES had started. This fence got passed through HWC and on to the consumer, so the consumer was reading the buffer before GLES was done writing to it. Now we update HWC's state just before set(), when we know we have the right fence. Bug: 11000763 Change-Id: Iea9db4c69634c352dc2d600f0bdb6bef2a432636 --- .../DisplayHardware/VirtualDisplaySurface.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp index 88ef3928c..c5a14b076 100644 --- a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp +++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp @@ -147,6 +147,10 @@ status_t VirtualDisplaySurface::advanceFrame() { mFbProducerSlot, fbBuffer.get(), mOutputProducerSlot, outBuffer.get()); + // At this point we know the output buffer acquire fence, + // so update HWC state with it. + mHwc.setOutputBuffer(mDisplayId, mOutputFence, outBuffer); + return mHwc.fbPost(mDisplayId, mFbFence, fbBuffer); } @@ -415,7 +419,11 @@ status_t VirtualDisplaySurface::refreshOutputBuffer() { return result; mOutputProducerSlot = mapSource2ProducerSlot(SOURCE_SINK, sslot); - result = mHwc.setOutputBuffer(mDisplayId, mOutputFence, + // On GLES-only frames, we don't have the right output buffer acquire fence + // until after GLES calls queueBuffer(). So here we just set the buffer + // (for use in HWC prepare) but not the fence; we'll call this again with + // the proper fence once we have it. + result = mHwc.setOutputBuffer(mDisplayId, Fence::NO_FENCE, mProducerBuffers[mOutputProducerSlot]); return result;