Don't store a ref from an EGL image to its source.

src/libANGLE/Display.cpp

@@ -729,8 +729,8 @@ Error Display::createImage(const gl::Context *context,
     }
     ASSERT(sibling != nullptr);
 
-    angle::UniqueObjectPointer<Image, gl::Context> imagePtr(
-        new Image(mImplementation, context, target, sibling, attribs), context);
+    angle::UniqueObjectPointer<Image, Display> imagePtr(
+        new Image(mImplementation, context, target, sibling, attribs), this);
     ANGLE_TRY(imagePtr->initialize(this));
 
     Image *image = imagePtr.release();
@@ -876,7 +876,7 @@ void Display::destroyImage(egl::Image *image)
 {
     auto iter = mImageSet.find(image);
     ASSERT(iter != mImageSet.end());
-    (*iter)->release(mProxyContext.get());
+    (*iter)->release(this);
     mImageSet.erase(iter);
 }
 

src/libANGLE/Image.cpp

@@ -10,6 +10,7 @@
 
 #include "common/debug.h"
 #include "common/utilities.h"
+#include "libANGLE/Context.h"
 #include "libANGLE/Renderbuffer.h"
 #include "libANGLE/Texture.h"
 #include "libANGLE/angletypes.h"
@@ -43,10 +44,15 @@ gl::ImageIndex GetImageIndex(EGLenum eglTarget, const egl::AttributeMap &attribs
         return gl::ImageIndex::MakeFromTarget(target, mip);
     }
 }
+
+const Display *DisplayFromContext(const gl::Context *context)
+{
+    return (context ? context->getCurrentDisplay() : nullptr);
+}
+
 }  // anonymous namespace
 
-ImageSibling::ImageSibling(GLuint id)
-    : RefCountObject(id), FramebufferAttachmentObject(), mSourcesOf(), mTargetOf()
+ImageSibling::ImageSibling() : FramebufferAttachmentObject(), mSourcesOf(), mTargetOf()
 {
 }
 
@@ -62,7 +68,7 @@ ImageSibling::~ImageSibling()
 void ImageSibling::setTargetImage(const gl::Context *context, egl::Image *imageTarget)
 {
     ASSERT(imageTarget != nullptr);
-    mTargetOf.set(context, imageTarget);
+    mTargetOf.set(DisplayFromContext(context), imageTarget);
     imageTarget->addTargetSibling(this);
 }
 
@@ -74,11 +80,11 @@ gl::Error ImageSibling::orphanImages(const gl::Context *context)
         ASSERT(mSourcesOf.empty());
 
         ANGLE_TRY(mTargetOf->orphanSibling(context, this));
-        mTargetOf.set(context, nullptr);
+        mTargetOf.set(DisplayFromContext(context), nullptr);
     }
     else
     {
-        for (egl::Image *sourceImage : mSourcesOf)
+        for (Image *sourceImage : mSourcesOf)
         {
             ANGLE_TRY(sourceImage->orphanSibling(context, this));
         }
@@ -137,8 +143,7 @@ Image::Image(rx::EGLImplFactory *factory,
              EGLenum target,
              ImageSibling *buffer,
              const AttributeMap &attribs)
-    : RefCountObject(0),
-      mState(target, buffer, attribs),
+    : mState(target, buffer, attribs),
       mImplementation(factory->createImage(mState, context, target, attribs)),
       mOrphanedAndNeedsInit(false)
 {
@@ -148,19 +153,20 @@ Image::Image(rx::EGLImplFactory *factory,
     mState.source->addImageSource(this);
 }
 
-gl::Error Image::onDestroy(const gl::Context *context)
+Error Image::onDestroy(const Display *display)
 {
     // All targets should hold a ref to the egl image and it should not be deleted until there are
     // no siblings left.
     ASSERT(mState.targets.empty());
 
     // Tell the source that it is no longer used by this image
-    if (mState.source.get() != nullptr)
+    if (mState.source != nullptr)
     {
         mState.source->removeImageSource(this);
-        mState.source.set(context, nullptr);
+        mState.source = nullptr;
     }
-    return gl::NoError();
+
+    return NoError();
 }
 
 Image::~Image()
@@ -185,14 +191,16 @@ void Image::addTargetSibling(ImageSibling *sibling)
 
 gl::Error Image::orphanSibling(const gl::Context *context, ImageSibling *sibling)
 {
+    ASSERT(sibling != nullptr);
+
     // notify impl
     ANGLE_TRY(mImplementation->orphan(context, sibling));
 
-    if (mState.source.get() == sibling)
+    if (mState.source == sibling)
     {
         // If the sibling is the source, it cannot be a target.
         ASSERT(mState.targets.find(sibling) == mState.targets.end());
-        mState.source.set(context, nullptr);
+        mState.source = nullptr;
         mOrphanedAndNeedsInit =
             (sibling->initState(mState.imageIndex) == gl::InitState::MayNeedInit);
     }
@@ -236,7 +244,7 @@ Error Image::initialize(const Display *display)
 
 bool Image::orphaned() const
 {
-    return (mState.source.get() == nullptr);
+    return (mState.source == nullptr);
 }
 
 gl::InitState Image::sourceInitState() const

src/libANGLE/Image.h

@@ -33,10 +33,10 @@ class Display;
 // Only currently Renderbuffers and Textures can be bound with images. This makes the relationship
 // explicit, and also ensures that an image sibling can determine if it's been initialized or not,
 // which is important for the robust resource init extension with Textures and EGLImages.
-class ImageSibling : public gl::RefCountObject, public gl::FramebufferAttachmentObject
+class ImageSibling : public gl::FramebufferAttachmentObject
 {
   public:
-    ImageSibling(GLuint id);
+    ImageSibling();
     ~ImageSibling() override;
 
     bool isEGLImageTarget() const;
@@ -60,7 +60,7 @@ class ImageSibling : public gl::RefCountObject, public gl::FramebufferAttachment
     void removeImageSource(egl::Image *imageSource);
 
     std::set<Image *> mSourcesOf;
-    gl::BindingPointer<Image> mTargetOf;
+    BindingPointer<Image> mTargetOf;
 };
 
 struct ImageState : private angle::NonCopyable
@@ -70,7 +70,7 @@ struct ImageState : private angle::NonCopyable
 
     EGLLabelKHR label;
     gl::ImageIndex imageIndex;
-    gl::BindingPointer<ImageSibling> source;
+    ImageSibling *source;
     std::set<ImageSibling *> targets;
 
     gl::Format format;
@@ -78,7 +78,7 @@ struct ImageState : private angle::NonCopyable
     size_t samples;
 };
 
-class Image final : public gl::RefCountObject, public LabeledObject
+class Image final : public RefCountObject, public LabeledObject
 {
   public:
     Image(rx::EGLImplFactory *factory,
@@ -87,7 +87,7 @@ class Image final : public gl::RefCountObject, public LabeledObject
           ImageSibling *buffer,
           const AttributeMap &attribs);
 
-    gl::Error onDestroy(const gl::Context *context) override;
+    Error onDestroy(const Display *display) override;
     ~Image() override;
 
     void setLabel(EGLLabelKHR label) override;

src/libANGLE/Image_unittest.cpp

@@ -45,11 +45,12 @@ TEST(ImageTest, RefCounting)
 
     egl::Image *image =
         new egl::Image(&mockEGLFactory, nullptr, EGL_GL_TEXTURE_2D, texture, egl::AttributeMap());
+    rx::MockImageImpl *imageImpl = static_cast<rx::MockImageImpl *>(image->getImplementation());
     image->addRef();
 
-    // Verify that the image added a ref to the texture and the texture has not added a ref to the
-    // image
-    EXPECT_EQ(2u, texture->getRefCount());
+    // Verify that the image does not add a ref to its source so that the source may still be
+    // deleted
+    EXPECT_EQ(1u, texture->getRefCount());
     EXPECT_EQ(1u, image->getRefCount());
 
     // Create a renderbuffer and set it as a target of the EGL image
@@ -65,30 +66,27 @@ TEST(ImageTest, RefCounting)
 
     // Verify that the renderbuffer added a ref to the image and the image did not add a ref to
     // the renderbuffer
-    EXPECT_EQ(2u, texture->getRefCount());
+    EXPECT_EQ(1u, texture->getRefCount());
     EXPECT_EQ(2u, image->getRefCount());
     EXPECT_EQ(1u, renderbuffer->getRefCount());
 
-    // Simulate deletion of the texture and verify that it still exists because the image holds a
-    // ref
+    // Simulate deletion of the texture and verify that it is deleted but the image still exists
+    EXPECT_CALL(*imageImpl, orphan(_, _)).WillOnce(Return(gl::NoError())).RetiresOnSaturation();
+    EXPECT_CALL(*textureImpl, destructor()).Times(1).RetiresOnSaturation();
     texture->release(nullptr);
-    EXPECT_EQ(1u, texture->getRefCount());
     EXPECT_EQ(2u, image->getRefCount());
     EXPECT_EQ(1u, renderbuffer->getRefCount());
 
     // Simulate deletion of the image and verify that it still exists because the renderbuffer holds
     // a ref
     image->release(nullptr);
-    EXPECT_EQ(1u, texture->getRefCount());
     EXPECT_EQ(1u, image->getRefCount());
     EXPECT_EQ(1u, renderbuffer->getRefCount());
 
     // Simulate deletion of the renderbuffer and verify that the deletion cascades to all objects
-    rx::MockImageImpl *imageImpl = static_cast<rx::MockImageImpl *>(image->getImplementation());
     EXPECT_CALL(*imageImpl, destructor()).Times(1).RetiresOnSaturation();
     EXPECT_CALL(*imageImpl, orphan(_, _)).WillOnce(Return(gl::NoError())).RetiresOnSaturation();
 
-    EXPECT_CALL(*textureImpl, destructor()).Times(1).RetiresOnSaturation();
     EXPECT_CALL(*renderbufferImpl, destructor()).Times(1).RetiresOnSaturation();
 
     renderbuffer->release(nullptr);
@@ -124,12 +122,11 @@ TEST(ImageTest, RespecificationReleasesReferences)
         new egl::Image(&mockEGLFactory, nullptr, EGL_GL_TEXTURE_2D, texture, egl::AttributeMap());
     image->addRef();
 
-    // Verify that the image added a ref to the texture and the texture has not added a ref to the
-    // image
-    EXPECT_EQ(2u, texture->getRefCount());
+    // Verify that the image did not add a ref to it's source.
+    EXPECT_EQ(1u, texture->getRefCount());
     EXPECT_EQ(1u, image->getRefCount());
 
-    // Respecify the texture and verify that the image releases its reference
+    // Respecify the texture and verify that the image is orpahaned
     rx::MockImageImpl *imageImpl = static_cast<rx::MockImageImpl *>(image->getImplementation());
     EXPECT_CALL(*imageImpl, orphan(_, _)).WillOnce(Return(gl::NoError())).RetiresOnSaturation();
     EXPECT_CALL(*textureImpl, setImage(_, _, _, _, _, _, _, _))

src/libANGLE/RefCountObject.h

@@ -207,4 +207,15 @@ class OffsetBindingPointer : public gl::BindingPointer<ObjectType>
 };
 }  // namespace gl
 
+namespace egl
+{
+class Display;
+
+using RefCountObject = angle::RefCountObject<Display, Error>;
+
+template <class ObjectType>
+using BindingPointer = angle::BindingPointer<ObjectType, Display, Error>;
+
+}  // namespace egl
+
 #endif   // LIBANGLE_REFCOUNTOBJECT_H_

src/libANGLE/Renderbuffer.cpp

@@ -66,7 +66,7 @@ void RenderbufferState::update(GLsizei width,
 
 // Renderbuffer implementation.
 Renderbuffer::Renderbuffer(rx::GLImplFactory *implFactory, GLuint id)
-    : egl::ImageSibling(id),
+    : RefCountObject(id),
       mState(),
       mImplementation(implFactory->createRenderbuffer(mState)),
       mLabel()

src/libANGLE/Renderbuffer.h

@@ -61,8 +61,7 @@ class RenderbufferState final : angle::NonCopyable
     InitState mInitState;
 };
 
-class Renderbuffer final : public egl::ImageSibling,
-                           public LabeledObject
+class Renderbuffer final : public RefCountObject, public egl::ImageSibling, public LabeledObject
 {
   public:
     Renderbuffer(rx::GLImplFactory *implFactory, GLuint id);

src/libANGLE/Texture.cpp

@@ -580,7 +580,7 @@ void TextureState::clearImageDescs()
 }
 
 Texture::Texture(rx::GLImplFactory *factory, GLuint id, TextureType type)
-    : egl::ImageSibling(id),
+    : RefCountObject(id),
       mState(type),
       mTexture(factory->createTexture(mState)),
       mLabel(),

src/libANGLE/Texture.h

@@ -187,7 +187,7 @@ struct TextureState final : private angle::NonCopyable
 bool operator==(const TextureState &a, const TextureState &b);
 bool operator!=(const TextureState &a, const TextureState &b);
 
-class Texture final : public egl::ImageSibling, public LabeledObject
+class Texture final : public RefCountObject, public egl::ImageSibling, public LabeledObject
 {
   public:
     Texture(rx::GLImplFactory *factory, GLuint id, TextureType type);

src/libANGLE/renderer/d3d/EGLImageD3D.cpp

@@ -44,7 +44,7 @@ egl::Error EGLImageD3D::initialize(const egl::Display *display)
 
 gl::Error EGLImageD3D::orphan(const gl::Context *context, egl::ImageSibling *sibling)
 {
-    if (sibling == mState.source.get())
+    if (sibling == mState.source)
     {
         ANGLE_TRY(copyToLocalRendertarget(context));
     }
@@ -54,7 +54,7 @@ gl::Error EGLImageD3D::orphan(const gl::Context *context, egl::ImageSibling *sib
 
 gl::Error EGLImageD3D::getRenderTarget(const gl::Context *context, RenderTargetD3D **outRT) const
 {
-    if (mState.source.get())
+    if (mState.source != nullptr)
     {
         ASSERT(!mRenderTarget);
         FramebufferAttachmentRenderTarget *rt = nullptr;
@@ -73,7 +73,7 @@ gl::Error EGLImageD3D::getRenderTarget(const gl::Context *context, RenderTargetD
 
 gl::Error EGLImageD3D::copyToLocalRendertarget(const gl::Context *context)
 {
-    ASSERT(mState.source.get() != nullptr);
+    ASSERT(mState.source != nullptr);
     ASSERT(mRenderTarget == nullptr);
 
     RenderTargetD3D *curRenderTarget = nullptr;

src/libANGLE/renderer/gl/egl/ImageEGL.cpp

@@ -60,14 +60,14 @@ egl::Error ImageEGL::initialize(const egl::Display *display)
             attributes.push_back(mState.imageIndex.getLayerIndex());
         }
 
-        const TextureGL *textureGL = GetImplAs<TextureGL>(GetAs<gl::Texture>(mState.source.get()));
+        const TextureGL *textureGL = GetImplAs<TextureGL>(GetAs<gl::Texture>(mState.source));
         buffer                = gl_egl::GLObjectHandleToEGLClientBuffer(textureGL->getTextureID());
         mNativeInternalFormat = textureGL->getNativeInternalFormat(mState.imageIndex);
     }
     else if (egl::IsRenderbufferTarget(mTarget))
     {
         const RenderbufferGL *renderbufferGL =
-            GetImplAs<RenderbufferGL>(GetAs<gl::Renderbuffer>(mState.source.get()));
+            GetImplAs<RenderbufferGL>(GetAs<gl::Renderbuffer>(mState.source));
         buffer = gl_egl::GLObjectHandleToEGLClientBuffer(renderbufferGL->getRenderbufferID());
         mNativeInternalFormat = renderbufferGL->getNativeInternalFormat();
     }