From 2d14a0ed4f5861bfa67e9db138ffdc70d2d5e6e4 Mon Sep 17 00:00:00 2001 From: Eric Penner Date: Mon, 25 Aug 2014 20:16:37 -0700 Subject: [PATCH] GLConsumer: Fix eglTerminate/eglInit edge case. If a display is terminated and then initialized, we can't detect this using the display itself (it has the same value), but all EglImages still become invalid for the display. This patch detects this during image binding and forces creation of a new EglImage. Bug: 10430249 Change-Id: I75101c50962f21263dca3ec6e241a2e5a3c23dad --- include/gui/GLConsumer.h | 4 +- libs/gui/GLConsumer.cpp | 32 ++++++++--- .../SurfaceTextureMultiContextGL_test.cpp | 55 +++++++++++++++++++ 3 files changed, 81 insertions(+), 10 deletions(-) diff --git a/include/gui/GLConsumer.h b/include/gui/GLConsumer.h index 37530db91..f91fe46d9 100644 --- a/include/gui/GLConsumer.h +++ b/include/gui/GLConsumer.h @@ -287,7 +287,9 @@ private: // createIfNeeded creates an EGLImage if required (we haven't created // one yet, or the EGLDisplay or crop-rect has changed). - status_t createIfNeeded(EGLDisplay display, const Rect& cropRect); + status_t createIfNeeded(EGLDisplay display, + const Rect& cropRect, + bool forceCreate = false); // This calls glEGLImageTargetTexture2DOES to bind the image to the // texture in the specified texture target. diff --git a/libs/gui/GLConsumer.cpp b/libs/gui/GLConsumer.cpp index 939c4a9e4..ccafe81ce 100644 --- a/libs/gui/GLConsumer.cpp +++ b/libs/gui/GLConsumer.cpp @@ -462,24 +462,37 @@ status_t GLConsumer::bindTextureImageLocked() { } status_t err = mCurrentTextureImage->createIfNeeded(mEglDisplay, - mCurrentCrop); - + mCurrentCrop); if (err != NO_ERROR) { ST_LOGW("bindTextureImage: can't create image on display=%p slot=%d", mEglDisplay, mCurrentTexture); return UNKNOWN_ERROR; } - mCurrentTextureImage->bindToTextureTarget(mTexTarget); - while ((error = glGetError()) != GL_NO_ERROR) { - ST_LOGE("bindTextureImage: error binding external image: %#04x", error); - return UNKNOWN_ERROR; + // In the rare case that the display is terminated and then initialized + // again, we can't detect that the display changed (it didn't), but the + // image is invalid. In this case, repeat the exact same steps while + // forcing the creation of a new image. + if ((error = glGetError()) != GL_NO_ERROR) { + glBindTexture(mTexTarget, mTexName); + status_t err = mCurrentTextureImage->createIfNeeded(mEglDisplay, + mCurrentCrop, + true); + if (err != NO_ERROR) { + ST_LOGW("bindTextureImage: can't create image on display=%p slot=%d", + mEglDisplay, mCurrentTexture); + return UNKNOWN_ERROR; + } + mCurrentTextureImage->bindToTextureTarget(mTexTarget); + if ((error = glGetError()) != GL_NO_ERROR) { + ST_LOGE("bindTextureImage: error binding external image: %#04x", error); + return UNKNOWN_ERROR; + } } // Wait for the new buffer to be ready. return doGLFenceWaitLocked(); - } status_t GLConsumer::checkAndUpdateEglStateLocked(bool contextCheck) { @@ -1056,12 +1069,13 @@ GLConsumer::EglImage::~EglImage() { } status_t GLConsumer::EglImage::createIfNeeded(EGLDisplay eglDisplay, - const Rect& cropRect) { + const Rect& cropRect, + bool forceCreation) { // If there's an image and it's no longer valid, destroy it. bool haveImage = mEglImage != EGL_NO_IMAGE_KHR; bool displayInvalid = mEglDisplay != eglDisplay; bool cropInvalid = hasEglAndroidImageCrop() && mCropRect != cropRect; - if (haveImage && (displayInvalid || cropInvalid)) { + if (haveImage && (displayInvalid || cropInvalid || forceCreation)) { if (!eglDestroyImageKHR(mEglDisplay, mEglImage)) { ALOGE("createIfNeeded: eglDestroyImageKHR failed"); } diff --git a/libs/gui/tests/SurfaceTextureMultiContextGL_test.cpp b/libs/gui/tests/SurfaceTextureMultiContextGL_test.cpp index 115a47d0f..1cd101e19 100644 --- a/libs/gui/tests/SurfaceTextureMultiContextGL_test.cpp +++ b/libs/gui/tests/SurfaceTextureMultiContextGL_test.cpp @@ -386,4 +386,59 @@ TEST_F(SurfaceTextureMultiContextGLTest, ASSERT_EQ(OK, mST->updateTexImage()); } +TEST_F(SurfaceTextureMultiContextGLTest, + AttachAfterDisplayTerminatedSucceeds) { + ASSERT_EQ(NO_ERROR, mST->setDefaultMaxBufferCount(2)); + + // produce two frames and consume them both on the primary context + ASSERT_NO_FATAL_FAILURE(produceOneRGBA8Frame(mANW)); + mFW->waitForFrame(); + ASSERT_EQ(OK, mST->updateTexImage()); + + ASSERT_NO_FATAL_FAILURE(produceOneRGBA8Frame(mANW)); + mFW->waitForFrame(); + ASSERT_EQ(OK, mST->updateTexImage()); + + // produce one more frame + ASSERT_NO_FATAL_FAILURE(produceOneRGBA8Frame(mANW)); + + // Detach from the primary context. + ASSERT_EQ(OK, mST->releaseTexImage()); + ASSERT_EQ(OK, mST->detachFromContext()); + + // Terminate and then initialize the display. All contexts, surfaces + // and images are invalid at this point. + EGLDisplay display = eglGetDisplay(EGL_DEFAULT_DISPLAY); + ASSERT_NE(EGL_NO_DISPLAY, mEglDisplay); + EGLint majorVersion = 0; + EGLint minorVersion = 0; + EXPECT_TRUE(eglTerminate(display)); + EXPECT_TRUE(eglInitialize(mEglDisplay, &majorVersion, &minorVersion)); + ASSERT_EQ(EGL_SUCCESS, eglGetError()); + + // The surface is invalid so create it again. + EGLint pbufferAttribs[] = { + EGL_WIDTH, 64, + EGL_HEIGHT, 64, + EGL_NONE }; + mEglSurface = eglCreatePbufferSurface(mEglDisplay, mGlConfig, + pbufferAttribs); + + // The second context is invalid so create it again. + mSecondEglContext = eglCreateContext(mEglDisplay, mGlConfig, + EGL_NO_CONTEXT, getContextAttribs()); + ASSERT_EQ(EGL_SUCCESS, eglGetError()); + ASSERT_NE(EGL_NO_CONTEXT, mSecondEglContext); + + ASSERT_TRUE(eglMakeCurrent(mEglDisplay, mEglSurface, mEglSurface, + mSecondEglContext)); + ASSERT_EQ(EGL_SUCCESS, eglGetError()); + + // Now attach to and consume final frame on secondary context. + ASSERT_EQ(OK, mST->attachToContext(SECOND_TEX_ID)); + mFW->waitForFrame(); + ASSERT_EQ(OK, mST->updateTexImage()); +} + + } // namespace android