Release egl::Surface on Texture image changes.

src/libANGLE/Surface.cpp

@@ -32,8 +32,7 @@ Surface::Surface(rx::SurfaceImpl *impl, EGLint surfaceType, const egl::Config *c
       // FIXME: Determine actual pixel aspect ratio
       mPixelAspectRatio(static_cast<EGLint>(1.0 * EGL_DISPLAY_SCALING)),
       mRenderBuffer(EGL_BACK_BUFFER),
-      mSwapBehavior(EGL_BUFFER_PRESERVED),
-      mTexture(NULL)
+      mSwapBehavior(EGL_BUFFER_PRESERVED)
 {
     addRef();
 
@@ -55,14 +54,14 @@ Surface::Surface(rx::SurfaceImpl *impl, EGLint surfaceType, const egl::Config *c
 
 Surface::~Surface()
 {
-    if (mTexture)
+    if (mTexture.get())
     {
         if (mImplementation)
         {
             mImplementation->releaseTexImage(EGL_BACK_BUFFER);
         }
-        mTexture->releaseTexImage();
-        mTexture = NULL;
+        mTexture->releaseTexImageFromSurface();
+        mTexture.set(nullptr);
     }
 
     SafeDelete(mImplementation);
@@ -145,23 +144,28 @@ EGLint Surface::getHeight() const
 
 Error Surface::bindTexImage(gl::Texture *texture, EGLint buffer)
 {
-    ASSERT(!mTexture);
+    ASSERT(!mTexture.get());
 
-    texture->bindTexImage(this);
-    mTexture = texture;
+    texture->bindTexImageFromSurface(this);
+    mTexture.set(texture);
     return mImplementation->bindTexImage(buffer);
 }
 
 Error Surface::releaseTexImage(EGLint buffer)
 {
-    ASSERT(mTexture);
-    gl::Texture *boundTexture = mTexture;
-    mTexture = NULL;
+    ASSERT(mTexture.get());
+    mTexture->releaseTexImageFromSurface();
+    mTexture.set(nullptr);
 
-    boundTexture->releaseTexImage();
     return mImplementation->releaseTexImage(buffer);
 }
 
+void Surface::releaseTexImageFromTexture()
+{
+    ASSERT(mTexture.get());
+    mTexture.set(nullptr);
+}
+
 GLenum Surface::getAttachmentInternalFormat(const gl::FramebufferAttachment::Target &target) const
 {
     const egl::Config *config = getConfig();

src/libANGLE/Surface.h

@@ -60,7 +60,7 @@ class Surface final : public gl::FramebufferAttachmentObject
     EGLenum getTextureFormat() const;
     EGLenum getTextureTarget() const;
 
-    gl::Texture *getBoundTexture() const { return mTexture; }
+    gl::Texture *getBoundTexture() const { return mTexture.get(); }
 
     EGLint isFixedSize() const;
 
@@ -74,6 +74,10 @@ class Surface final : public gl::FramebufferAttachmentObject
     virtual ~Surface();
     rx::FramebufferAttachmentObjectImpl *getAttachmentImpl() const override { return mImplementation; }
 
+    // ANGLE-only method, used internally
+    friend class gl::Texture;
+    void releaseTexImageFromTexture();
+
     rx::SurfaceImpl *mImplementation;
 
     EGLint mType;
@@ -93,7 +97,7 @@ class Surface final : public gl::FramebufferAttachmentObject
     EGLenum mRenderBuffer;         // Render buffer
     EGLenum mSwapBehavior;         // Buffer swap behavior
 
-    gl::Texture *mTexture;
+    BindingPointer<gl::Texture> mTexture;
 };
 
 }

src/libANGLE/Texture.cpp

@@ -167,14 +167,15 @@ Error Texture::setImage(GLenum target, size_t level, GLenum internalFormat, cons
 {
     ASSERT(target == mTarget || (mTarget == GL_TEXTURE_CUBE_MAP && IsCubeMapTextureTarget(target)));
 
+    // Release from previous calls to eglBindTexImage, to avoid calling the Impl after
+    releaseTexImageInternal();
+
     Error error = mTexture->setImage(target, level, internalFormat, size, format, type, unpack, pixels);
     if (error.isError())
     {
         return error;
     }
 
-    releaseTexImage();
-
     setImageDesc(target, level, ImageDesc(size, GetSizedInternalFormat(internalFormat, type)));
 
     return Error(GL_NO_ERROR);
@@ -193,14 +194,15 @@ Error Texture::setCompressedImage(GLenum target, size_t level, GLenum internalFo
 {
     ASSERT(target == mTarget || (mTarget == GL_TEXTURE_CUBE_MAP && IsCubeMapTextureTarget(target)));
 
+    // Release from previous calls to eglBindTexImage, to avoid calling the Impl after
+    releaseTexImageInternal();
+
     Error error = mTexture->setCompressedImage(target, level, internalFormat, size, unpack, pixels);
     if (error.isError())
     {
         return error;
     }
 
-    releaseTexImage();
-
     setImageDesc(target, level, ImageDesc(size, GetSizedInternalFormat(internalFormat, GL_UNSIGNED_BYTE)));
 
     return Error(GL_NO_ERROR);
@@ -219,14 +221,15 @@ Error Texture::copyImage(GLenum target, size_t level, const Rectangle &sourceAre
 {
     ASSERT(target == mTarget || (mTarget == GL_TEXTURE_CUBE_MAP && IsCubeMapTextureTarget(target)));
 
+    // Release from previous calls to eglBindTexImage, to avoid calling the Impl after
+    releaseTexImageInternal();
+
     Error error = mTexture->copyImage(target, level, sourceArea, internalFormat, source);
     if (error.isError())
     {
         return error;
     }
 
-    releaseTexImage();
-
     setImageDesc(target, level, ImageDesc(Extents(sourceArea.width, sourceArea.height, 1),
                                           GetSizedInternalFormat(internalFormat, GL_UNSIGNED_BYTE)));
 
@@ -245,14 +248,15 @@ Error Texture::setStorage(GLenum target, size_t levels, GLenum internalFormat, c
 {
     ASSERT(target == mTarget);
 
+    // Release from previous calls to eglBindTexImage, to avoid calling the Impl after
+    releaseTexImageInternal();
+
     Error error = mTexture->setStorage(target, levels, internalFormat, size);
     if (error.isError())
     {
         return error;
     }
 
-    releaseTexImage();
-
     mImmutableLevelCount = levels;
     clearImageDescs();
     setImageDescChain(levels, size, internalFormat);
@@ -263,14 +267,15 @@ Error Texture::setStorage(GLenum target, size_t levels, GLenum internalFormat, c
 
 Error Texture::generateMipmaps()
 {
+    // Release from previous calls to eglBindTexImage, to avoid calling the Impl after
+    releaseTexImageInternal();
+
     Error error = mTexture->generateMipmaps(getSamplerState());
     if (error.isError())
     {
         return error;
     }
 
-    releaseTexImage();
-
     const ImageDesc &baseImageInfo = getImageDesc(getBaseImageTarget(), 0);
     size_t mipLevels = log2(std::max(std::max(baseImageInfo.size.width, baseImageInfo.size.height), baseImageInfo.size.depth)) + 1;
     setImageDescChain(mipLevels, baseImageInfo.size, baseImageInfo.internalFormat);
@@ -341,11 +346,15 @@ void Texture::clearImageDescs()
     mCompletenessCache.cacheValid = false;
 }
 
-void Texture::bindTexImage(egl::Surface *surface)
+void Texture::bindTexImageFromSurface(egl::Surface *surface)
 {
     ASSERT(surface);
 
-    releaseTexImage();
+    if (mBoundSurface)
+    {
+        releaseTexImageFromSurface();
+    }
+
     mTexture->bindTexImage(surface);
     mBoundSurface = surface;
 
@@ -356,16 +365,26 @@ void Texture::bindTexImage(egl::Surface *surface)
     setImageDesc(mTarget, 0, desc);
 }
 
-void Texture::releaseTexImage()
+void Texture::releaseTexImageFromSurface()
+{
+    ASSERT(mBoundSurface);
+    mBoundSurface = nullptr;
+    mTexture->releaseTexImage();
+
+    // Erase the image info for level 0
+    ASSERT(mTarget == GL_TEXTURE_2D);
+    clearImageDesc(mTarget, 0);
+}
+
+void Texture::releaseTexImageInternal()
 {
     if (mBoundSurface)
     {
-        mBoundSurface = NULL;
-        mTexture->releaseTexImage();
+        // Notify the surface
+        mBoundSurface->releaseTexImageFromTexture();
 
-        // Erase the image info for level 0
-        ASSERT(mTarget == GL_TEXTURE_2D);
-        clearImageDesc(mTarget, 0);
+        // Then, call the same method as from the surface
+        releaseTexImageFromSurface();
     }
 }
 

src/libANGLE/Texture.h

@@ -78,9 +78,6 @@ class Texture final : public FramebufferAttachmentObject
     bool isImmutable() const;
     GLsizei immutableLevelCount();
 
-    void bindTexImage(egl::Surface *surface);
-    void releaseTexImage();
-
     rx::TextureImpl *getImplementation() { return mTexture; }
     const rx::TextureImpl *getImplementation() const { return mTexture; }
 
@@ -93,6 +90,11 @@ class Texture final : public FramebufferAttachmentObject
   private:
     rx::FramebufferAttachmentObjectImpl *getAttachmentImpl() const override { return mTexture; }
 
+    // ANGLE-only method, used internally
+    friend class egl::Surface;
+    void bindTexImageFromSurface(egl::Surface *surface);
+    void releaseTexImageFromSurface();
+
     rx::TextureImpl *mTexture;
 
     SamplerState mSamplerState;
@@ -123,6 +125,7 @@ class Texture final : public FramebufferAttachmentObject
     void setImageDescChain(size_t levels, Extents baseSize, GLenum sizedInternalFormat);
     void clearImageDesc(GLenum target, size_t level);
     void clearImageDescs();
+    void releaseTexImageInternal();
 
     std::vector<ImageDesc> mImageDescs;
 

src/tests/gl_tests/PbufferTest.cpp

@@ -266,5 +266,68 @@ TEST_P(PbufferTest, TextureSizeReset)
     EXPECT_PIXEL_EQ(0, 0, 0, 0, 0, 255);
 }
 
+// Bind a Pbuffer, redefine the texture, and verify it renders correctly
+TEST_P(PbufferTest, BindTexImageAndRedefineTexture)
+{
+    if (!mSupportsPbuffers)
+    {
+        std::cout << "Test skipped because Pbuffers are not supported." << std::endl;
+        return;
+    }
+
+    if (!mSupportsBindTexImage)
+    {
+        std::cout << "Test skipped because Pbuffer does not support binding to RGBA textures." << std::endl;
+        return;
+    }
+
+    EGLWindow *window = getEGLWindow();
+
+    // Apply the Pbuffer and clear it to purple
+    eglMakeCurrent(window->getDisplay(), mPbuffer, mPbuffer, window->getContext());
+    ASSERT_EGL_SUCCESS();
+
+    glViewport(0, 0, mPbufferSize, mPbufferSize);
+    glClearColor(1.0f, 0.0f, 1.0f, 1.0f);
+    glClear(GL_COLOR_BUFFER_BIT);
+    ASSERT_GL_NO_ERROR();
+
+    EXPECT_PIXEL_EQ(mPbufferSize / 2, mPbufferSize / 2, 255, 0, 255, 255);
+
+    // Apply the window surface
+    eglMakeCurrent(window->getDisplay(), window->getSurface(), window->getSurface(), window->getContext());
+
+    // Create a texture and bind the Pbuffer to it
+    GLuint texture = 0;
+    glGenTextures(1, &texture);
+    glBindTexture(GL_TEXTURE_2D, texture);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
+    EXPECT_GL_NO_ERROR();
+
+    eglBindTexImage(window->getDisplay(), mPbuffer, EGL_BACK_BUFFER);
+    glViewport(0, 0, window->getWidth(), window->getHeight());
+    ASSERT_EGL_SUCCESS();
+
+    // Redefine the texture
+    unsigned int pixelValue = 0xFFFF00FF;
+    std::vector<unsigned int> pixelData(window->getWidth() * window->getHeight(), pixelValue);
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, window->getWidth(), window->getHeight(), 0, GL_RGBA, GL_UNSIGNED_BYTE, &pixelData[0]);
+
+    // Draw a quad and verify that it is magenta
+    glUseProgram(mTextureProgram);
+    glUniform1i(mTextureUniformLocation, 0);
+
+    drawQuad(mTextureProgram, "position", 0.5f);
+    EXPECT_GL_NO_ERROR();
+
+    // Verify that magenta was drawn
+    EXPECT_PIXEL_EQ(window->getWidth() / 2, window->getHeight() / 2, 255, 0, 255, 255);
+
+    glDeleteTextures(1, &texture);
+}
+
 // Use this to select which configurations (e.g. which renderer, which GLES major version) these tests should be run against.
 ANGLE_INSTANTIATE_TEST(PbufferTest, ES2_D3D9(), ES2_D3D11(), ES2_OPENGL(), ES2_D3D11_WARP(), ES2_D3D11_REFERENCE());