Vulkan: Fix sampler object lifetime.

Using the same scheme as we do for VkImageViews we can track VkSampler
lifetime using SharedResourceUse. This fixes the race condition that
could occur when samplers are deleted in one Context while being used
in another.

This fixes the last known resource lifetime issue. The multithreading
tests should now pass without validation errors.

Also adds regression tests to angle_end2end_tests.

Bug: angleproject:2464
Change-Id: I9dbed5062a0863b240ddf1a9b5d28560334934de
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/1869548
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Ian Elliott <ianelliott@google.com>
Reviewed-by: Tim Van Patten <timvp@google.com>

src/libANGLE/renderer/vulkan/ContextVk.cpp

@@ -2921,7 +2921,21 @@ angle::Result ContextVk::updateActiveTextures(const gl::Context *context,
         }
 
         TextureVk *textureVk = vk::GetImpl(texture);
-        SamplerVk *samplerVk = (sampler != nullptr) ? vk::GetImpl(sampler) : nullptr;
+
+        SamplerVk *samplerVk;
+        Serial samplerSerial;
+        if (sampler == nullptr)
+        {
+            samplerVk     = nullptr;
+            samplerSerial = kZeroSerial;
+            textureVk->onSamplerGraphAccess(&mCommandGraph);
+        }
+        else
+        {
+            samplerVk     = vk::GetImpl(sampler);
+            samplerSerial = samplerVk->getSerial();
+            samplerVk->onSamplerGraphAccess(&mCommandGraph);
+        }
 
         vk::ImageHelper &image = textureVk->getImage();
 
@@ -2953,8 +2967,7 @@ angle::Result ContextVk::updateActiveTextures(const gl::Context *context,
         mActiveTextures[textureUnit].sampler = samplerVk;
         // Cache serials from sampler and texture, but re-use texture if no sampler bound
         ASSERT(textureVk != nullptr);
-        mActiveTexturesDesc.update(textureUnit, textureVk->getSerial(),
-                                   (samplerVk != nullptr) ? samplerVk->getSerial() : kZeroSerial);
+        mActiveTexturesDesc.update(textureUnit, textureVk->getSerial(), samplerSerial);
     }
 
     return angle::Result::Continue;

src/libANGLE/renderer/vulkan/SamplerVk.cpp

@@ -23,13 +23,7 @@ SamplerVk::~SamplerVk() = default;
 void SamplerVk::onDestroy(const gl::Context *context)
 {
     ContextVk *contextVk = vk::GetImpl(context);
-    contextVk->addGarbage(&mSampler);
-}
-
-const vk::Sampler &SamplerVk::getSampler() const
-{
-    ASSERT(mSampler.valid());
-    return mSampler;
+    mSampler.release(contextVk->getRenderer());
 }
 
 angle::Result SamplerVk::syncState(const gl::Context *context, const bool dirty)
@@ -43,7 +37,7 @@ angle::Result SamplerVk::syncState(const gl::Context *context, const bool dirty)
         {
             return angle::Result::Continue;
         }
-        contextVk->addGarbage(&mSampler);
+        mSampler.release(renderer);
     }
 
     const gl::Extensions &extensions = renderer->getNativeExtensions();
@@ -78,7 +72,7 @@ angle::Result SamplerVk::syncState(const gl::Context *context, const bool dirty)
         samplerInfo.maxLod     = 0.25f;
     }
 
-    ANGLE_VK_TRY(contextVk, mSampler.init(contextVk->getDevice(), samplerInfo));
+    ANGLE_VK_TRY(contextVk, mSampler.get().init(contextVk->getDevice(), samplerInfo));
     // Regenerate the serial on a sampler change.
     mSerial = contextVk->generateTextureSerial();
     return angle::Result::Continue;

src/libANGLE/renderer/vulkan/SamplerVk.h

@@ -25,11 +25,22 @@ class SamplerVk : public SamplerImpl
 
     void onDestroy(const gl::Context *context) override;
     angle::Result syncState(const gl::Context *context, const bool dirty) override;
-    const vk::Sampler &getSampler() const;
+
+    const vk::Sampler &getSampler() const
+    {
+        ASSERT(mSampler.valid());
+        return mSampler.get();
+    }
+
     Serial getSerial() const { return mSerial; }
 
+    void onSamplerGraphAccess(vk::CommandGraph *commandGraph)
+    {
+        mSampler.onGraphAccess(commandGraph);
+    }
+
   private:
-    vk::Sampler mSampler;
+    vk::SamplerHelper mSampler;
     // The serial is used for cache indexing.
     Serial mSerial;
 };

src/libANGLE/renderer/vulkan/TextureVk.cpp

@@ -117,7 +117,7 @@ void TextureVk::onDestroy(const gl::Context *context)
     ContextVk *contextVk = vk::GetImpl(context);
 
     releaseAndDeleteImage(contextVk);
-    contextVk->addGarbage(&mSampler);
+    mSampler.release(contextVk->getRenderer());
 }
 
 angle::Result TextureVk::setImage(const gl::Context *context,
@@ -1348,7 +1348,7 @@ angle::Result TextureVk::syncState(const gl::Context *context,
     RendererVk *renderer = contextVk->getRenderer();
     if (mSampler.valid())
     {
-        contextVk->addGarbage(&mSampler);
+        mSampler.release(renderer);
     }
 
     if (dirtyBits.test(gl::Texture::DIRTY_BIT_SWIZZLE_RED) ||
@@ -1363,7 +1363,7 @@ angle::Result TextureVk::syncState(const gl::Context *context,
             uint32_t layerCount =
                 mState.getType() == gl::TextureType::_2D ? 1 : mImage->getLayerCount();
 
-            mImageViews.release(contextVk->getRenderer());
+            mImageViews.release(renderer);
             const gl::ImageDesc &baseLevelDesc = mState.getBaseLevelDesc();
 
             ANGLE_TRY(initImageViews(contextVk, mImage->getFormat(),
@@ -1417,7 +1417,7 @@ angle::Result TextureVk::syncState(const gl::Context *context,
         samplerInfo.maxLod     = 0.25f;
     }
 
-    ANGLE_VK_TRY(contextVk, mSampler.init(contextVk->getDevice(), samplerInfo));
+    ANGLE_VK_TRY(contextVk, mSampler.get().init(contextVk->getDevice(), samplerInfo));
 
     // Regenerate the serial on a sampler change.
     mSerial = contextVk->generateTextureSerial();
@@ -1515,12 +1515,6 @@ angle::Result TextureVk::getStorageImageView(ContextVk *contextVk,
                                              nativeLayer, imageViewOut);
 }
 
-const vk::Sampler &TextureVk::getSampler() const
-{
-    ASSERT(mSampler.valid());
-    return mSampler;
-}
-
 angle::Result TextureVk::initImage(ContextVk *contextVk,
                                    const vk::Format &format,
                                    const bool sized,

src/libANGLE/renderer/vulkan/TextureVk.h

@@ -157,6 +157,11 @@ class TextureVk : public TextureImpl
         mImageViews.onGraphAccess(commandGraph);
     }
 
+    void onSamplerGraphAccess(vk::CommandGraph *commandGraph)
+    {
+        mSampler.onGraphAccess(commandGraph);
+    }
+
     void releaseOwnershipOfImage(const gl::Context *context);
 
     const vk::ImageView &getReadImageViewAndRecordUse(ContextVk *contextVk) const;
@@ -168,7 +173,12 @@ class TextureVk : public TextureImpl
                                       size_t level,
                                       size_t singleLayer,
                                       const vk::ImageView **imageViewOut);
-    const vk::Sampler &getSampler() const;
+
+    const vk::Sampler &getSampler() const
+    {
+        ASSERT(mSampler.valid());
+        return mSampler.get();
+    }
 
     angle::Result ensureImageInitialized(ContextVk *contextVk);
 
@@ -338,7 +348,7 @@ class TextureVk : public TextureImpl
 
     // |mSampler| contains the relevant Vulkan sampler states reprensenting the OpenGL Texture
     // sampling states for the Texture.
-    vk::Sampler mSampler;
+    vk::SamplerHelper mSampler;
 
     // Render targets stored as vector of vectors
     // Level is first dimension, layer is second

src/libANGLE/renderer/vulkan/vk_helpers.cpp

@@ -3104,7 +3104,23 @@ angle::Result ImageViewHelper::getLevelLayerDrawImageView(ContextVk *contextVk,
                                     imageView, level, 1, layer, 1);
 }
 
-// FramebufferHelper implementation.
+// SamplerHelper implementation.
+SamplerHelper::SamplerHelper()
+{
+    mUse.init();
+}
+
+SamplerHelper::~SamplerHelper()
+{
+    mUse.release();
+}
+
+void SamplerHelper::release(RendererVk *renderer)
+{
+    renderer->collectGarbageAndReinit(&mUse, &mSampler);
+}
+
+// DispatchHelper implementation.
 DispatchHelper::DispatchHelper() : CommandGraphResource(CommandGraphResourceType::Dispatcher) {}
 
 DispatchHelper::~DispatchHelper() = default;

src/libANGLE/renderer/vulkan/vk_helpers.h

@@ -1052,6 +1052,26 @@ class ImageViewHelper : angle::NonCopyable
     LayerLevelImageViewVector mLayerLevelDrawImageViews;
 };
 
+// The SamplerHelper allows a Sampler to be coupled with a resource lifetime.
+class SamplerHelper final : angle::NonCopyable
+{
+  public:
+    SamplerHelper();
+    ~SamplerHelper();
+
+    void release(RendererVk *renderer);
+
+    bool valid() const { return mSampler.valid(); }
+    Sampler &get() { return mSampler; }
+    const Sampler &get() const { return mSampler; }
+
+    void onGraphAccess(CommandGraph *commandGraph) { commandGraph->onResourceUse(mUse); }
+
+  private:
+    SharedResourceUse mUse;
+    Sampler mSampler;
+};
+
 class FramebufferHelper : public CommandGraphResource
 {
   public:

src/tests/egl_tests/EGLContextSharingTest.cpp

@@ -10,6 +10,7 @@
 
 #include "test_utils/ANGLETest.h"
 #include "test_utils/angle_test_configs.h"
+#include "test_utils/gl_raii.h"
 #include "util/EGLWindow.h"
 
 using namespace angle;
@@ -243,6 +244,126 @@ TEST_P(EGLContextSharingTest, DisplayShareGroupReleasedWithLastContext)
     ASSERT_GL_FALSE(glIsTexture(textureFromCtx0));
 }
 
+// Tests that deleting an object on one Context doesn't destroy it ahead-of-time. Mostly focused
+// on the Vulkan back-end where we manage object lifetime manually.
+TEST_P(EGLContextSharingTest, TextureLifetime)
+{
+    EGLWindow *eglWindow = getEGLWindow();
+    EGLConfig config     = getEGLWindow()->getConfig();
+    EGLDisplay display   = getEGLWindow()->getDisplay();
+
+    // Create a pbuffer surface for use with a shared context.
+    EGLSurface surface     = eglWindow->getSurface();
+    EGLContext mainContext = eglWindow->getContext();
+
+    // Initialize a shared context.
+    mContexts[0] = eglCreateContext(display, config, mainContext, nullptr);
+    ASSERT_NE(mContexts[0], EGL_NO_CONTEXT);
+
+    // Create a Texture on the shared context.
+    ASSERT_EGL_TRUE(eglMakeCurrent(display, surface, surface, mContexts[0]));
+
+    constexpr GLsizei kTexSize                  = 2;
+    const GLColor kTexData[kTexSize * kTexSize] = {GLColor::red, GLColor::green, GLColor::blue,
+                                                   GLColor::yellow};
+    GLTexture tex;
+    glBindTexture(GL_TEXTURE_2D, tex);
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, kTexSize, kTexSize, 0, GL_RGBA, GL_UNSIGNED_BYTE,
+                 kTexData);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+
+    // Make the main Context current and draw with the texture.
+    ASSERT_EGL_TRUE(eglMakeCurrent(display, surface, surface, mainContext));
+
+    glBindTexture(GL_TEXTURE_2D, tex);
+    ANGLE_GL_PROGRAM(program, essl1_shaders::vs::Texture2D(), essl1_shaders::fs::Texture2D());
+    glUseProgram(program);
+
+    // No uniform update because the update seems to hide the error on Vulkan.
+
+    // Enqueue the draw call.
+    drawQuad(program, essl1_shaders::PositionAttrib(), 0.5f);
+    EXPECT_GL_NO_ERROR();
+
+    // Delete the texture in the main context to orphan it.
+    // Do not read back the data to keep the commands in the graph.
+    tex.reset();
+
+    // Bind and delete the test context. This should trigger texture garbage collection.
+    ASSERT_EGL_TRUE(eglMakeCurrent(display, surface, surface, mContexts[0]));
+    SafeDestroyContext(display, mContexts[0]);
+
+    // Bind the main context to clean up the test.
+    ASSERT_EGL_TRUE(eglMakeCurrent(display, surface, surface, mainContext));
+}
+
+// Tests that deleting an object on one Context doesn't destroy it ahead-of-time. Mostly focused
+// on the Vulkan back-end where we manage object lifetime manually.
+TEST_P(EGLContextSharingTest, SamplerLifetime)
+{
+    EGLWindow *eglWindow = getEGLWindow();
+    EGLConfig config     = getEGLWindow()->getConfig();
+    EGLDisplay display   = getEGLWindow()->getDisplay();
+
+    ANGLE_SKIP_TEST_IF(getClientMajorVersion() < 3);
+    ANGLE_SKIP_TEST_IF(!IsEGLDisplayExtensionEnabled(display, "EGL_KHR_create_context"));
+
+    // Create a pbuffer surface for use with a shared context.
+    EGLSurface surface     = eglWindow->getSurface();
+    EGLContext mainContext = eglWindow->getContext();
+
+    std::vector<EGLint> contextAttributes;
+    contextAttributes.push_back(EGL_CONTEXT_MAJOR_VERSION_KHR);
+    contextAttributes.push_back(getClientMajorVersion());
+    contextAttributes.push_back(EGL_NONE);
+
+    // Initialize a shared context.
+    mContexts[0] = eglCreateContext(display, config, mainContext, contextAttributes.data());
+    ASSERT_NE(mContexts[0], EGL_NO_CONTEXT);
+
+    // Create a Texture on the shared context. Also create a Sampler object.
+    ASSERT_EGL_TRUE(eglMakeCurrent(display, surface, surface, mContexts[0]));
+
+    constexpr GLsizei kTexSize                  = 2;
+    const GLColor kTexData[kTexSize * kTexSize] = {GLColor::red, GLColor::green, GLColor::blue,
+                                                   GLColor::yellow};
+    GLTexture tex;
+    glBindTexture(GL_TEXTURE_2D, tex);
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, kTexSize, kTexSize, 0, GL_RGBA, GL_UNSIGNED_BYTE,
+                 kTexData);
+
+    GLSampler sampler;
+    glBindSampler(0, sampler);
+    glSamplerParameteri(sampler, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+    glSamplerParameteri(sampler, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+
+    // Make the main Context current and draw with the texture and sampler.
+    ASSERT_EGL_TRUE(eglMakeCurrent(display, surface, surface, mainContext));
+
+    glBindTexture(GL_TEXTURE_2D, tex);
+    glBindSampler(0, sampler);
+    ANGLE_GL_PROGRAM(program, essl1_shaders::vs::Texture2D(), essl1_shaders::fs::Texture2D());
+    glUseProgram(program);
+
+    // No uniform update because the update seems to hide the error on Vulkan.
+
+    // Enqueue the draw call.
+    drawQuad(program, essl1_shaders::PositionAttrib(), 0.5f);
+    EXPECT_GL_NO_ERROR();
+
+    // Delete the texture and sampler in the main context to orphan them.
+    // Do not read back the data to keep the commands in the graph.
+    tex.reset();
+    sampler.reset();
+
+    // Bind and delete the test context. This should trigger texture garbage collection.
+    ASSERT_EGL_TRUE(eglMakeCurrent(display, surface, surface, mContexts[0]));
+    SafeDestroyContext(display, mContexts[0]);
+
+    // Bind the main context to clean up the test.
+    ASSERT_EGL_TRUE(eglMakeCurrent(display, surface, surface, mainContext));
+}
 }  // anonymous namespace
 
 ANGLE_INSTANTIATE_TEST(EGLContextSharingTest,
@@ -251,4 +372,5 @@ ANGLE_INSTANTIATE_TEST(EGLContextSharingTest,
                        ES3_D3D11(),
                        ES2_OPENGL(),
                        ES3_OPENGL(),
-                       ES2_VULKAN());
+                       ES2_VULKAN(),
+                       ES3_VULKAN());

util/shader_utils.cpp

@@ -408,12 +408,12 @@ void main()
 const char *Texture2D()
 {
     return R"(precision mediump float;
-uniform sampler2D u_tex;
+uniform sampler2D u_tex2D;
 varying vec2 v_texCoord;
 
 void main()
 {
-    gl_FragColor = texture2D(u_tex, v_texCoord);
+    gl_FragColor = texture2D(u_tex2D, v_texCoord);
 })";
 }