Add destroy handler to SurfaceImpl.

src/libANGLE/Context.cpp

@@ -381,7 +381,7 @@ Context::Context(rx::EGLImplFactory *implFactory,
     handleError(mImplementation->initialize());
 }
 
-Context::~Context()
+void Context::destroy(egl::Display *display)
 {
     mGLState.reset();
 
@@ -419,12 +419,16 @@ Context::~Context()
 
     SafeDelete(mSurfacelessFramebuffer);
 
-    releaseSurface();
+    releaseSurface(display);
 
     SafeDelete(mCompiler);
 }
 
-void Context::makeCurrent(egl::Surface *surface)
+Context::~Context()
+{
+}
+
+void Context::makeCurrent(egl::Display *display, egl::Surface *surface)
 {
     if (!mHasBeenCurrent)
     {
@@ -449,12 +453,12 @@ void Context::makeCurrent(egl::Surface *surface)
     // TODO(jmadill): Rework this when we support ContextImpl
     mGLState.setAllDirtyBits();
 
-    releaseSurface();
+    releaseSurface(display);
 
     Framebuffer *newDefault = nullptr;
     if (surface != nullptr)
     {
-        surface->setIsCurrent(true);
+        surface->setIsCurrent(display, true);
         mCurrentSurface = surface;
         newDefault      = surface->getDefaultFramebuffer();
     }
@@ -486,7 +490,7 @@ void Context::makeCurrent(egl::Surface *surface)
     mImplementation->onMakeCurrent(mState);
 }
 
-void Context::releaseSurface()
+void Context::releaseSurface(egl::Display *display)
 {
     // Remove the default framebuffer
     Framebuffer *currentDefault = nullptr;
@@ -511,7 +515,7 @@ void Context::releaseSurface()
 
     if (mCurrentSurface)
     {
-        mCurrentSurface->setIsCurrent(false);
+        mCurrentSurface->setIsCurrent(display, false);
         mCurrentSurface = nullptr;
     }
 }

src/libANGLE/Context.h

@@ -64,10 +64,11 @@ class Context final : public ValidationContext
             const egl::AttributeMap &attribs,
             const egl::DisplayExtensions &displayExtensions);
 
-    virtual ~Context();
+    void destroy(egl::Display *display);
+    ~Context() override;
 
-    void makeCurrent(egl::Surface *surface);
-    void releaseSurface();
+    void makeCurrent(egl::Display *display, egl::Surface *surface);
+    void releaseSurface(egl::Display *display);
 
     // These create  and destroy methods are merely pass-throughs to
     // ResourceManager, which owns these object types

src/libANGLE/Display.cpp

@@ -691,7 +691,7 @@ Error Display::makeCurrent(egl::Surface *drawSurface, egl::Surface *readSurface,
     if (context != nullptr)
     {
         ASSERT(readSurface == drawSurface);
-        context->makeCurrent(drawSurface);
+        context->makeCurrent(this, drawSurface);
     }
 
     return egl::Error(EGL_SUCCESS);
@@ -733,7 +733,7 @@ void Display::destroySurface(Surface *surface)
     }
 
     mState.surfaceSet.erase(surface);
-    surface->onDestroy();
+    surface->onDestroy(this);
 }
 
 void Display::destroyImage(egl::Image *image)
@@ -752,6 +752,7 @@ void Display::destroyStream(egl::Stream *stream)
 
 void Display::destroyContext(gl::Context *context)
 {
+    context->destroy(this);
     mContextSet.erase(context);
     SafeDelete(context);
 }

src/libANGLE/Surface.cpp

@@ -104,28 +104,35 @@ Error Surface::initialize(const Display &display)
     return Error(EGL_SUCCESS);
 }
 
-void Surface::setIsCurrent(bool isCurrent)
+void Surface::setIsCurrent(Display *display, bool isCurrent)
 {
     if (isCurrent)
     {
         mCurrentCount++;
+        return;
     }
-    else
+
+    ASSERT(mCurrentCount > 0);
+    mCurrentCount--;
+    if (mCurrentCount == 0 && mDestroyed)
     {
-        ASSERT(mCurrentCount > 0);
-        mCurrentCount--;
-        if (mCurrentCount == 0 && mDestroyed)
+        if (mImplementation)
         {
-            delete this;
+            mImplementation->destroy(rx::SafeGetImpl(display));
         }
+        delete this;
     }
 }
 
-void Surface::onDestroy()
+void Surface::onDestroy(Display *display)
 {
     mDestroyed = true;
     if (mCurrentCount == 0)
     {
+        if (mImplementation)
+        {
+            mImplementation->destroy(rx::SafeGetImpl(display));
+        }
         delete this;
     }
 }

src/libANGLE/Surface.h

@@ -65,8 +65,8 @@ class Surface : public gl::FramebufferAttachmentObject
     EGLint isPostSubBufferSupported() const;
 
     void setSwapInterval(EGLint interval);
-    void setIsCurrent(bool isCurrent);
-    void onDestroy();
+    void setIsCurrent(Display *display, bool isCurrent);
+    void onDestroy(Display *display);
 
     const Config *getConfig() const;
 

src/libANGLE/Surface_unittest.cpp

@@ -26,8 +26,9 @@ class MockSurfaceImpl : public rx::SurfaceImpl
 {
   public:
     MockSurfaceImpl() : SurfaceImpl(mockState), mockState(nullptr) {}
-    virtual ~MockSurfaceImpl() { destroy(); }
+    virtual ~MockSurfaceImpl() { destructor(); }
 
+    MOCK_METHOD1(destroy, void(const DisplayImpl *));
     MOCK_METHOD1(initialize, egl::Error(const DisplayImpl *));
     MOCK_METHOD1(createDefaultFramebuffer, rx::FramebufferImpl *(const gl::FramebufferState &data));
     MOCK_METHOD1(swap, egl::Error(const DisplayImpl *));
@@ -43,7 +44,7 @@ class MockSurfaceImpl : public rx::SurfaceImpl
     MOCK_CONST_METHOD0(getSwapBehavior, EGLint(void));
     MOCK_METHOD2(getAttachmentRenderTarget, gl::Error(const gl::FramebufferAttachment::Target &, rx::FramebufferAttachmentRenderTarget **));
 
-    MOCK_METHOD0(destroy, void());
+    MOCK_METHOD0(destructor, void());
 
     egl::SurfaceState mockState;
 };
@@ -59,9 +60,10 @@ TEST(SurfaceTest, DestructionDeletesImpl)
     egl::Surface *surface = new egl::WindowSurface(
         &factory, &config, static_cast<EGLNativeWindowType>(0), egl::AttributeMap());
 
-    EXPECT_CALL(*impl, destroy()).Times(1).RetiresOnSaturation();
+    EXPECT_CALL(*impl, destroy(_)).Times(1).RetiresOnSaturation();
+    EXPECT_CALL(*impl, destructor()).Times(1).RetiresOnSaturation();
 
-    surface->onDestroy();
+    surface->onDestroy(nullptr);
 
     // Only needed because the mock is leaked if bugs are present,
     // which logs an error, but does not cause the test to fail.

src/libANGLE/renderer/DisplayImpl.cpp

@@ -55,4 +55,4 @@ const egl::Caps &DisplayImpl::getCaps() const
     return mCaps;
 }
 
-}
+}  // namespace rx

src/libANGLE/renderer/SurfaceImpl.h

@@ -36,6 +36,7 @@ class SurfaceImpl : public FramebufferAttachmentObjectImpl
   public:
     SurfaceImpl(const egl::SurfaceState &surfaceState);
     virtual ~SurfaceImpl();
+    virtual void destroy(const DisplayImpl *displayImpl) {}
 
     virtual egl::Error initialize(const DisplayImpl *displayImpl)                        = 0;
     virtual FramebufferImpl *createDefaultFramebuffer(const gl::FramebufferState &state) = 0;

src/libANGLE/renderer/vulkan/FramebufferVk.cpp

@@ -300,6 +300,7 @@ gl::Error FramebufferVk::readPixels(ContextImpl *context,
 
     PackPixels(params, angleFormat, inputPitch, mapPointer, reinterpret_cast<uint8_t *>(pixels));
 
+    stagingImage.getImage().destroy(renderer->getDevice());
     stagingImage.getDeviceMemory().unmap();
 
     return vk::NoError();

src/libANGLE/renderer/vulkan/SurfaceVk.cpp

@@ -121,8 +121,6 @@ WindowSurfaceVk::WindowSurfaceVk(const egl::SurfaceState &surfaceState,
       mNativeWindowType(window),
       mSurface(VK_NULL_HANDLE),
       mSwapchain(VK_NULL_HANDLE),
-      mDevice(VK_NULL_HANDLE),
-      mInstance(VK_NULL_HANDLE),
       mRenderTarget(),
       mCurrentSwapchainImageIndex(0)
 {
@@ -133,17 +131,43 @@ WindowSurfaceVk::WindowSurfaceVk(const egl::SurfaceState &surfaceState,
 
 WindowSurfaceVk::~WindowSurfaceVk()
 {
+    ASSERT(mSurface == VK_NULL_HANDLE);
+    ASSERT(mSwapchain == VK_NULL_HANDLE);
+    ASSERT(mSwapchainImages.empty());
+    ASSERT(mSwapchainImageViews.empty());
+}
+
+void WindowSurfaceVk::destroy(const DisplayImpl *displayImpl)
+{
+    const DisplayVk *displayVk = GetAs<DisplayVk>(displayImpl);
+    RendererVk *rendererVk     = displayVk->getRenderer();
+    VkDevice device            = rendererVk->getDevice();
+    VkInstance instance        = rendererVk->getInstance();
+
+    for (auto &imageView : mSwapchainImageViews)
+    {
+        imageView.destroy(device);
+    }
+
+    mSwapchainImageViews.clear();
+
+    // Although we don't own the swapchain image handles, we need to keep our shutdown clean.
+    for (auto &image : mSwapchainImages)
+    {
+        image.reset();
+    }
+
     mSwapchainImages.clear();
 
     if (mSwapchain)
     {
-        vkDestroySwapchainKHR(mDevice, mSwapchain, nullptr);
+        vkDestroySwapchainKHR(device, mSwapchain, nullptr);
         mSwapchain = VK_NULL_HANDLE;
     }
 
     if (mSurface)
     {
-        vkDestroySurfaceKHR(mInstance, mSurface, nullptr);
+        vkDestroySurfaceKHR(instance, mSurface, nullptr);
         mSurface = VK_NULL_HANDLE;
     }
 }
@@ -156,11 +180,6 @@ egl::Error WindowSurfaceVk::initialize(const DisplayImpl *displayImpl)
 
 vk::Error WindowSurfaceVk::initializeImpl(RendererVk *renderer)
 {
-    // These are needed for resource deallocation.
-    // TODO(jmadill): Don't cache these.
-    mDevice   = renderer->getDevice();
-    mInstance = renderer->getInstance();
-
     // TODO(jmadill): Make this platform-specific.
     VkWin32SurfaceCreateInfoKHR createInfo;
 

src/libANGLE/renderer/vulkan/SurfaceVk.h

@@ -59,6 +59,8 @@ class WindowSurfaceVk : public SurfaceImpl
                     EGLint height);
     ~WindowSurfaceVk() override;
 
+    void destroy(const DisplayImpl *contextImpl) override;
+
     egl::Error initialize(const DisplayImpl *displayImpl) override;
     FramebufferImpl *createDefaultFramebuffer(const gl::FramebufferState &state) override;
     egl::Error swap(const DisplayImpl *displayImpl) override;
@@ -91,10 +93,6 @@ class WindowSurfaceVk : public SurfaceImpl
     EGLNativeWindowType mNativeWindowType;
     VkSurfaceKHR mSurface;
     VkSwapchainKHR mSwapchain;
-    // These are needed for resource deallocation.
-    // TODO(jmadill): Don't store these here.
-    VkDevice mDevice;
-    VkInstance mInstance;
 
     RenderTargetVk mRenderTarget;
     vk::Semaphore mPresentCompleteSemaphore;

src/libANGLE/renderer/vulkan/renderervk_utils.cpp

@@ -419,10 +419,20 @@ Image &Image::operator=(Image &&other)
 
 Image::~Image()
 {
-    // If the device handle is null, we aren't managing the handle.
+    ASSERT(mHandle == VK_NULL_HANDLE);
+}
+
+void Image::reset()
+{
+    mHandle = VK_NULL_HANDLE;
+}
+
+void Image::destroy(VkDevice device)
+{
     if (valid())
     {
-        vkDestroyImage(mDevice, mHandle, nullptr);
+        vkDestroyImage(device, mHandle, nullptr);
+        mHandle = VK_NULL_HANDLE;
     }
 }
 
@@ -532,10 +542,15 @@ ImageView &ImageView::operator=(ImageView &&other)
 
 ImageView::~ImageView()
 {
-    if (mHandle != VK_NULL_HANDLE)
+    ASSERT(mHandle == VK_NULL_HANDLE);
+}
+
+void ImageView::destroy(VkDevice device)
+{
+    if (valid())
     {
-        ASSERT(validDevice());
-        vkDestroyImageView(mDevice, mHandle, nullptr);
+        vkDestroyImageView(device, mHandle, nullptr);
+        mHandle = VK_NULL_HANDLE;
     }
 }
 

src/libANGLE/renderer/vulkan/renderervk_utils.h

@@ -166,6 +166,12 @@ class Image final : public WrappedObject<VkImage>
     explicit Image(VkImage image);
     Image(Image &&other);
 
+    // Called on shutdown when the helper class *doesn't* own the handle to the image resource.
+    void reset();
+
+    // Called on shutdown when the helper class *does* own the handle to the image resource.
+    void destroy(VkDevice device);
+
     Image &operator=(Image &&other);
 
     ~Image() override;
@@ -199,6 +205,8 @@ class ImageView final : public WrappedObject<VkImageView>
     explicit ImageView(VkDevice device);
     ImageView(ImageView &&other);
 
+    void destroy(VkDevice device);
+
     ImageView &operator=(ImageView &&other);
 
     ~ImageView() override;

src/libGLESv2/entry_points_egl.cpp

@@ -571,7 +571,7 @@ EGLBoolean EGLAPIENTRY MakeCurrent(EGLDisplay dpy, EGLSurface draw, EGLSurface r
     // destroyed surfaces to delete themselves.
     if (previousContext != nullptr && context != previousContext)
     {
-        previousContext->releaseSurface();
+        previousContext->releaseSurface(display);
     }
 
     thread->setError(Error(EGL_SUCCESS));