Vulkan: Fix FramebufferVk cache

src/libANGLE/renderer/vulkan/ContextVk.h

@@ -422,7 +422,10 @@ class ContextVk : public ContextImpl, public vk::Context
     // avoid calling vkAllocateDesctiporSets each texture update.
     Serial generateTextureSerial() { return mTextureSerialFactory.generate(); }
     const vk::TextureDescriptorDesc &getActiveTexturesDesc() const { return mActiveTexturesDesc; }
-    Serial generateAttachmentImageSerial() { return mAttachmentImageSerialFactory.generate(); }
+    Serial generateAttachmentImageViewSerial()
+    {
+        return mAttachmentImageViewSerialFactory.generate();
+    }
 
     angle::Result updateScissor(const gl::State &glState);
 
@@ -965,7 +968,7 @@ class ContextVk : public ContextImpl, public vk::Context
 
     // Generators for texture & framebuffer serials.
     SerialFactory mTextureSerialFactory;
-    SerialFactory mAttachmentImageSerialFactory;
+    SerialFactory mAttachmentImageViewSerialFactory;
 
     gl::State::DirtyBits mPipelineDirtyBitsMask;
 

src/libANGLE/renderer/vulkan/FramebufferVk.cpp

@@ -1094,11 +1094,12 @@ angle::Result FramebufferVk::updateColorAttachment(const gl::Context *context,
 
     if (renderTarget && mState.getEnabledDrawBuffers()[colorIndexGL])
     {
-        mCurrentFramebufferDesc.update(colorIndexGL, renderTarget->getAssignSerial(contextVk));
+        mCurrentFramebufferDesc.update(colorIndexGL,
+                                       renderTarget->getAssignImageViewSerial(contextVk));
     }
     else
     {
-        mCurrentFramebufferDesc.update(colorIndexGL, vk::kZeroAttachmentSerial);
+        mCurrentFramebufferDesc.update(colorIndexGL, kZeroSerial);
     }
 
     return angle::Result::Continue;
@@ -1136,12 +1137,11 @@ void FramebufferVk::updateDepthStencilAttachmentSerial(ContextVk *contextVk)
     if (depthStencilRT != nullptr)
     {
         mCurrentFramebufferDesc.update(vk::kFramebufferDescDepthStencilIndex,
-                                       depthStencilRT->getAssignSerial(contextVk));
+                                       depthStencilRT->getAssignImageViewSerial(contextVk));
     }
     else
     {
-        mCurrentFramebufferDesc.update(vk::kFramebufferDescDepthStencilIndex,
-                                       vk::kZeroAttachmentSerial);
+        mCurrentFramebufferDesc.update(vk::kFramebufferDescDepthStencilIndex, kZeroSerial);
     }
 }
 
@@ -1181,7 +1181,8 @@ angle::Result FramebufferVk::syncState(const gl::Context *context,
                 {
                     mCurrentFramebufferDesc.update(
                         static_cast<uint32_t>(colorIndexGL),
-                        mRenderTargetCache.getColors()[colorIndexGL]->getAssignSerial(contextVk));
+                        mRenderTargetCache.getColors()[colorIndexGL]->getAssignImageViewSerial(
+                            contextVk));
                 }
                 updateDepthStencilAttachmentSerial(contextVk);
                 break;

src/libANGLE/renderer/vulkan/RenderTargetVk.cpp

@@ -58,18 +58,15 @@ void RenderTargetVk::reset()
     mContentDefined = false;
 }
 
-vk::AttachmentSerial RenderTargetVk::getAssignSerial(ContextVk *contextVk)
+Serial RenderTargetVk::getAssignImageViewSerial(ContextVk *contextVk)
 {
-    ASSERT(mImage && mImage->valid());
-    vk::AttachmentSerial attachmentSerial;
+    ASSERT(mImageViews);
     ASSERT(mLayerIndex < std::numeric_limits<uint16_t>::max());
     ASSERT(mLevelIndex < std::numeric_limits<uint16_t>::max());
-    Serial imageSerial = mImage->getAssignSerial(contextVk);
-    ASSERT(imageSerial.getValue() < std::numeric_limits<uint32_t>::max());
-    SetBitField(attachmentSerial.layer, mLayerIndex);
-    SetBitField(attachmentSerial.level, mLevelIndex);
-    SetBitField(attachmentSerial.imageSerial, imageSerial.getValue());
-    return attachmentSerial;
+
+    Serial imageViewSerial = mImageViews->getAssignSerial(contextVk, mLevelIndex, mLayerIndex);
+    ASSERT(imageViewSerial.getValue() < std::numeric_limits<uint32_t>::max());
+    return imageViewSerial;
 }
 
 angle::Result RenderTargetVk::onColorDraw(ContextVk *contextVk)

src/libANGLE/renderer/vulkan/RenderTargetVk.h

@@ -47,8 +47,8 @@ class RenderTargetVk final : public FramebufferAttachmentRenderTarget
               uint32_t levelIndex,
               uint32_t layerIndex);
     void reset();
-    // This returns the serial from underlying ImageHelper, first assigning one if required
-    vk::AttachmentSerial getAssignSerial(ContextVk *contextVk);
+    // This returns the serial from underlying ImageViewHelper, first assigning one if required
+    Serial getAssignImageViewSerial(ContextVk *contextVk);
 
     // Note: RenderTargets should be called in order, with the depth/stencil onRender last.
     angle::Result onColorDraw(ContextVk *contextVk);

src/libANGLE/renderer/vulkan/vk_cache_utils.cpp

@@ -1667,7 +1667,7 @@ FramebufferDesc::~FramebufferDesc()                            = default;
 FramebufferDesc::FramebufferDesc(const FramebufferDesc &other) = default;
 FramebufferDesc &FramebufferDesc::operator=(const FramebufferDesc &other) = default;
 
-void FramebufferDesc::update(uint32_t index, AttachmentSerial serial)
+void FramebufferDesc::update(uint32_t index, Serial serial)
 {
     ASSERT(index < kMaxFramebufferAttachments);
     mSerials[index] = serial;
@@ -1675,13 +1675,12 @@ void FramebufferDesc::update(uint32_t index, AttachmentSerial serial)
 
 size_t FramebufferDesc::hash() const
 {
-    return angle::ComputeGenericHash(&mSerials,
-                                     sizeof(AttachmentSerial) * kMaxFramebufferAttachments);
+    return angle::ComputeGenericHash(&mSerials, sizeof(Serial) * kMaxFramebufferAttachments);
 }
 
 void FramebufferDesc::reset()
 {
-    memset(&mSerials, 0, sizeof(AttachmentSerial) * kMaxFramebufferAttachments);
+    memset(&mSerials, 0, sizeof(Serial) * kMaxFramebufferAttachments);
 }
 
 bool FramebufferDesc::operator==(const FramebufferDesc &other) const
@@ -1692,9 +1691,9 @@ bool FramebufferDesc::operator==(const FramebufferDesc &other) const
 uint32_t FramebufferDesc::attachmentCount() const
 {
     uint32_t count = 0;
-    for (const AttachmentSerial &serial : mSerials)
+    for (const Serial &serial : mSerials)
     {
-        if (serial.imageSerial != 0)
+        if (serial.valid())
         {
             count++;
         }

src/libANGLE/renderer/vulkan/vk_cache_utils.h

@@ -782,15 +782,7 @@ constexpr size_t kMaxFramebufferAttachments = gl::IMPLEMENTATION_MAX_FRAMEBUFFER
 // Color serials are at index [0:gl::IMPLEMENTATION_MAX_DRAW_BUFFERS-1]
 // Depth/stencil index is at gl::IMPLEMENTATION_MAX_DRAW_BUFFERS
 constexpr size_t kFramebufferDescDepthStencilIndex = gl::IMPLEMENTATION_MAX_DRAW_BUFFERS;
-// Struct for AttachmentSerial cache signatures. Includes level/layer for imageView as
-//  well as a unique Serial value for the underlying image
-struct AttachmentSerial
-{
-    uint16_t level;
-    uint16_t layer;
-    uint32_t imageSerial;
-};
-constexpr AttachmentSerial kZeroAttachmentSerial = {0, 0, 0};
+
 class FramebufferDesc
 {
   public:
@@ -800,7 +792,7 @@ class FramebufferDesc
     FramebufferDesc(const FramebufferDesc &other);
     FramebufferDesc &operator=(const FramebufferDesc &other);
 
-    void update(uint32_t index, AttachmentSerial serial);
+    void update(uint32_t index, Serial serial);
     size_t hash() const;
     void reset();
 
@@ -809,7 +801,19 @@ class FramebufferDesc
     uint32_t attachmentCount() const;
 
   private:
-    gl::AttachmentArray<AttachmentSerial> mSerials;
+    gl::AttachmentArray<Serial> mSerials;
+};
+
+// Layer/level pair type used to index into Serial Cache in ImageViewHelper
+struct LayerLevel
+{
+    uint32_t layer;
+    uint32_t level;
+
+    bool operator==(const LayerLevel &other) const
+    {
+        return layer == other.layer && level == other.level;
+    }
 };
 }  // namespace vk
 }  // namespace rx
@@ -864,6 +868,18 @@ struct hash<rx::vk::SamplerDesc>
 {
     size_t operator()(const rx::vk::SamplerDesc &key) const { return key.hash(); }
 };
+
+template <>
+struct hash<rx::vk::LayerLevel>
+{
+    size_t operator()(const rx::vk::LayerLevel &layerLevel) const
+    {
+        // The left-shift by 11 was found to produce unique hash values
+        // in a 256x256 space for layer/level
+        return std::hash<uint32_t>()(layerLevel.layer) ^
+               (std::hash<uint32_t>()(layerLevel.level) << 11);
+    }
+};
 }  // namespace std
 
 namespace rx

src/libANGLE/renderer/vulkan/vk_helpers.cpp

@@ -3210,15 +3210,6 @@ void ImageHelper::clear(VkImageAspectFlags aspectFlags,
     }
 }
 
-Serial ImageHelper::getAssignSerial(ContextVk *contextVk)
-{
-    if (mSerial.getValue() == 0)
-    {
-        mSerial = contextVk->generateAttachmentImageSerial();
-    }
-    return mSerial;
-}
-
 // static
 void ImageHelper::Copy(ImageHelper *srcImage,
                        ImageHelper *dstImage,
@@ -4686,6 +4677,7 @@ ImageViewHelper::ImageViewHelper(ImageViewHelper &&other)
     std::swap(mStencilReadImageView, other.mStencilReadImageView);
     std::swap(mLevelDrawImageViews, other.mLevelDrawImageViews);
     std::swap(mLayerLevelDrawImageViews, other.mLayerLevelDrawImageViews);
+    std::swap(mSerialCache, other.mSerialCache);
 }
 
 ImageViewHelper::~ImageViewHelper()
@@ -4754,6 +4746,8 @@ void ImageViewHelper::release(RendererVk *renderer)
         // Ensure the resource use is always valid.
         mUse.init();
     }
+
+    mSerialCache.clear();
 }
 
 void ImageViewHelper::destroy(VkDevice device)
@@ -4780,6 +4774,8 @@ void ImageViewHelper::destroy(VkDevice device)
         }
     }
     mLayerLevelDrawImageViews.clear();
+
+    mSerialCache.clear();
 }
 
 angle::Result ImageViewHelper::initReadViews(ContextVk *contextVk,
@@ -4965,6 +4961,16 @@ angle::Result ImageViewHelper::getLevelLayerDrawImageView(ContextVk *contextVk,
                                     imageView, level, 1, layer, 1);
 }
 
+Serial ImageViewHelper::getAssignSerial(ContextVk *contextVk, uint32_t level, uint32_t layer)
+{
+    LayerLevel layerLevelPair = {layer, level};
+    if (mSerialCache.find(layerLevelPair) == mSerialCache.end())
+    {
+        mSerialCache[layerLevelPair] = contextVk->generateAttachmentImageViewSerial();
+    }
+    return mSerialCache[layerLevelPair];
+}
+
 // SamplerHelper implementation.
 SamplerHelper::SamplerHelper()
 {

src/libANGLE/renderer/vulkan/vk_helpers.h

@@ -1152,8 +1152,6 @@ class ImageHelper final : public Resource, public angle::Subject
                uint32_t layerCount,
                CommandBuffer *commandBuffer);
 
-    // Return unique Serial for underlying image, first assigning it if it hasn't been set yet
-    Serial getAssignSerial(ContextVk *contextVk);
     void resetSerial() { mSerial = rx::kZeroSerial; }
 
     static void Copy(ImageHelper *srcImage,
@@ -1605,6 +1603,9 @@ class ImageViewHelper : angle::NonCopyable
                                              uint32_t layer,
                                              const ImageView **imageViewOut);
 
+    // Return unique Serial for this imageView, first assigning it if it hasn't yet been set
+    Serial getAssignSerial(ContextVk *contextVk, uint32_t level, uint32_t layer);
+
   private:
     ImageView &getReadImageView()
     {
@@ -1636,6 +1637,9 @@ class ImageViewHelper : angle::NonCopyable
     // Draw views.
     ImageViewVector mLevelDrawImageViews;
     LayerLevelImageViewVector mLayerLevelDrawImageViews;
+
+    // Store Serials per layer/level of imageView
+    std::unordered_map<LayerLevel, Serial> mSerialCache;
 };
 
 // The SamplerHelper allows a Sampler to be coupled with a resource lifetime.

src/tests/gl_tests/StateChangeTest.cpp

@@ -4953,9 +4953,6 @@ void main()
 // Verify that a swizzle on an active sampler is handled appropriately
 TEST_P(ImageRespecificationTest, Swizzle)
 {
-    // TODO: Fix FramebufferVk caching logic (http://anglebug.com/4651)
-    ANGLE_SKIP_TEST_IF(isVulkanRenderer());
-
     GLubyte data[] = {1, 64, 128, 200};
     GLColor expectedData(data[0], data[1], data[2], data[3]);
 
@@ -5006,9 +5003,6 @@ TEST_P(ImageRespecificationTest, Swizzle)
 // the Framebuffer that has the texture as a color attachment is recreated before next use.
 TEST_P(ImageRespecificationTest, ImageTarget2DOESSwitch)
 {
-    // TODO: Fix FramebufferVk caching logic (http://anglebug.com/4651)
-    ANGLE_SKIP_TEST_IF(isVulkanRenderer());
-
     // This is the specific problem on the Vulkan backend and needs some extensions
     ANGLE_SKIP_TEST_IF(
         !IsGLExtensionEnabled("GL_OES_EGL_image_external") ||