Vulkan: Fool-proof usage of GL and VK level indices

Using boxed types, this change allows the compiler to catch errors when
a level index in one space (e.g. GL) is mistakenly used in another space
(e.g. VK).

This change uncovered a number of bugs due to such mistakes which are
fixed.

Mistakes are still possible when the index is explicitly extracted, for
example to be given to a Vulkan command, or when it's created, for
example when retrieved from gl::ImageIndex::getLevelIndex.  Future work
can include using gl::LevelIndex in gl::ImageIndex directly to alleviate
the latter at least.

Bug: angleproject:4880
Change-Id: I6427c68c3bc096f771402f51c8554d8171758aa9
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2380232
Commit-Queue: Shahbaz Youssefi <[email protected]>
Reviewed-by: Tim Van Patten <[email protected]>
Reviewed-by: Jamie Madill <[email protected]>
Reviewed-by: Charlie Lao <[email protected]>

Author: ShabbyX

src/libANGLE/angletypes.h

@@ -815,6 +815,63 @@ using TextureBarrierVector = BarrierVector<TextureAndLayout>;
 // necessary. Returns 0 if no buffer is bound or if integer overflow occurs.
 GLsizeiptr GetBoundBufferAvailableSize(const OffsetBindingPointer<Buffer> &binding);
 
+// A texture level index.
+template <typename T>
+class LevelIndexWrapper
+{
+  public:
+    LevelIndexWrapper() = default;
+    explicit constexpr LevelIndexWrapper(T levelIndex) : mLevelIndex(levelIndex) {}
+    constexpr LevelIndexWrapper(const LevelIndexWrapper &other) = default;
+    constexpr LevelIndexWrapper &operator=(const LevelIndexWrapper &other) = default;
+
+    constexpr T get() const { return mLevelIndex; }
+
+    LevelIndexWrapper &operator++()
+    {
+        ++mLevelIndex;
+        return *this;
+    }
+    constexpr bool operator<(const LevelIndexWrapper &other) const
+    {
+        return mLevelIndex < other.mLevelIndex;
+    }
+    constexpr bool operator<=(const LevelIndexWrapper &other) const
+    {
+        return mLevelIndex <= other.mLevelIndex;
+    }
+    constexpr bool operator>(const LevelIndexWrapper &other) const
+    {
+        return mLevelIndex > other.mLevelIndex;
+    }
+    constexpr bool operator>=(const LevelIndexWrapper &other) const
+    {
+        return mLevelIndex >= other.mLevelIndex;
+    }
+    constexpr bool operator==(const LevelIndexWrapper &other) const
+    {
+        return mLevelIndex == other.mLevelIndex;
+    }
+    constexpr bool operator!=(const LevelIndexWrapper &other) const
+    {
+        return mLevelIndex != other.mLevelIndex;
+    }
+    constexpr LevelIndexWrapper operator+(T other) const
+    {
+        return LevelIndexWrapper(mLevelIndex + other);
+    }
+    constexpr LevelIndexWrapper operator-(T other) const
+    {
+        return LevelIndexWrapper(mLevelIndex - other);
+    }
+    constexpr T operator-(LevelIndexWrapper other) const { return mLevelIndex - other.mLevelIndex; }
+
+  private:
+    T mLevelIndex;
+};
+
+// A GL texture level index.
+using LevelIndex = LevelIndexWrapper<GLint>;
 }  // namespace gl
 
 namespace rx

src/libANGLE/renderer/vulkan/FramebufferVk.cpp

@@ -779,15 +779,15 @@ angle::Result FramebufferVk::blitWithCommand(ContextVk *contextVk,
     ANGLE_TRY(contextVk->onImageTransferWrite(imageAspectMask, dstImage));
     vk::CommandBuffer &commandBuffer = contextVk->getOutsideRenderPassCommandBuffer();
 
-    VkImageBlit blit                   = {};
-    blit.srcSubresource.aspectMask     = blitAspectMask;
-    blit.srcSubresource.mipLevel       = readRenderTarget->getLevelIndex();
+    VkImageBlit blit               = {};
+    blit.srcSubresource.aspectMask = blitAspectMask;
+    blit.srcSubresource.mipLevel   = srcImage->toVKLevel(readRenderTarget->getLevelIndex()).get();
     blit.srcSubresource.baseArrayLayer = readRenderTarget->getLayerIndex();
     blit.srcSubresource.layerCount     = 1;
     blit.srcOffsets[0]                 = {sourceArea.x0(), sourceArea.y0(), 0};
     blit.srcOffsets[1]                 = {sourceArea.x1(), sourceArea.y1(), 1};
     blit.dstSubresource.aspectMask     = blitAspectMask;
-    blit.dstSubresource.mipLevel       = drawRenderTarget->getLevelIndex();
+    blit.dstSubresource.mipLevel = dstImage->toVKLevel(drawRenderTarget->getLevelIndex()).get();
     blit.dstSubresource.baseArrayLayer = drawRenderTarget->getLayerIndex();
     blit.dstSubresource.layerCount     = 1;
     blit.dstOffsets[0]                 = {destArea.x0(), destArea.y0(), 0};
@@ -1028,7 +1028,7 @@ angle::Result FramebufferVk::blit(const gl::Context *context,
         params.srcLayer                       = readRenderTarget->getLayerIndex();
 
         // Multisampled images are not allowed to have mips.
-        ASSERT(!isColorResolve || readRenderTarget->getLevelIndex() == 0);
+        ASSERT(!isColorResolve || readRenderTarget->getLevelIndex() == gl::LevelIndex(0));
 
         // If there was no clipping and the format capabilities allow us, use Vulkan's builtin blit.
         // The reason clipping is prohibited in this path is that due to rounding errors, it would
@@ -1127,7 +1127,7 @@ angle::Result FramebufferVk::blit(const gl::Context *context,
         params.srcLayer                       = readRenderTarget->getLayerIndex();
 
         // Multisampled images are not allowed to have mips.
-        ASSERT(!isDepthStencilResolve || readRenderTarget->getLevelIndex() == 0);
+        ASSERT(!isDepthStencilResolve || readRenderTarget->getLevelIndex() == gl::LevelIndex(0));
 
         // Similarly, only blit if there's been no clipping or rotating.
         bool canBlitWithCommand = !isDepthStencilResolve && noClip &&
@@ -1158,8 +1158,9 @@ angle::Result FramebufferVk::blit(const gl::Context *context,
             vk::DeviceScoped<vk::ImageView> stencilView(contextVk->getDevice());
 
             vk::ImageHelper *depthStencilImage = &readRenderTarget->getImageForCopy();
-            uint32_t levelIndex                = readRenderTarget->getLevelIndex();
-            uint32_t layerIndex                = readRenderTarget->getLayerIndex();
+            vk::LevelIndex levelIndex =
+                depthStencilImage->toVKLevel(readRenderTarget->getLevelIndex());
+            uint32_t layerIndex         = readRenderTarget->getLayerIndex();
             gl::TextureType textureType = vk::Get2DTextureType(depthStencilImage->getLayerCount(),
                                                                depthStencilImage->getSamples());
 
@@ -1302,8 +1303,8 @@ angle::Result FramebufferVk::resolveColorWithCommand(ContextVk *contextVk,
         vk::CommandBuffer &commandBuffer = contextVk->getOutsideRenderPassCommandBuffer();
 
         vk::ImageHelper &dstImage = drawRenderTarget->getImageForWrite();
-        uint32_t levelVK          = drawRenderTarget->getLevelIndex() - dstImage.getBaseLevel();
-        resolveRegion.dstSubresource.mipLevel       = levelVK;
+        vk::LevelIndex levelVK    = dstImage.toVKLevel(drawRenderTarget->getLevelIndex());
+        resolveRegion.dstSubresource.mipLevel       = levelVK.get();
         resolveRegion.dstSubresource.baseArrayLayer = drawRenderTarget->getLayerIndex();
 
         srcImage->resolve(&dstImage, resolveRegion, &commandBuffer);
@@ -1331,9 +1332,9 @@ angle::Result FramebufferVk::copyResolveToMultisampedAttachment(ContextVk *conte
     // Note: neither vkCmdCopyImage nor vkCmdBlitImage allow the destination to be multisampled.
     // There's no choice but to use a draw-based path to perform this copy.
 
-    gl::Extents extents = colorRenderTarget->getExtents();
-    uint32_t levelVK    = colorRenderTarget->getLevelIndex() - src->getBaseLevel();
-    uint32_t layer      = colorRenderTarget->getLayerIndex();
+    gl::Extents extents    = colorRenderTarget->getExtents();
+    vk::LevelIndex levelVK = src->toVKLevel(colorRenderTarget->getLevelIndex());
+    uint32_t layer         = colorRenderTarget->getLayerIndex();
 
     UtilsVk::CopyImageParameters params;
     params.srcOffset[0]        = 0;
@@ -1342,7 +1343,7 @@ angle::Result FramebufferVk::copyResolveToMultisampedAttachment(ContextVk *conte
     params.srcExtents[1]       = extents.height;
     params.destOffset[0]       = 0;
     params.destOffset[1]       = 0;
-    params.srcMip              = levelVK;
+    params.srcMip              = levelVK.get();
     params.srcLayer            = layer;
     params.srcHeight           = extents.height;
     params.srcPremultiplyAlpha = false;
@@ -2395,8 +2396,8 @@ angle::Result FramebufferVk::readPixelsImpl(ContextVk *contextVk,
                                             void *pixels)
 {
     ANGLE_TRACE_EVENT0("gpu.angle", "FramebufferVk::readPixelsImpl");
-    uint32_t levelGL = renderTarget->getLevelIndex();
-    uint32_t layer   = renderTarget->getLayerIndex();
+    gl::LevelIndex levelGL = renderTarget->getLevelIndex();
+    uint32_t layer         = renderTarget->getLayerIndex();
     return renderTarget->getImageForCopy().readPixels(contextVk, area, packPixelsParams,
                                                       copyAspectFlags, levelGL, layer, pixels,
                                                       &mReadPixelBuffer);

src/libANGLE/renderer/vulkan/ImageVk.cpp

@@ -67,7 +67,7 @@ egl::Error ImageVk::initialize(const egl::Display *display)
         mOwnsImage = false;
 
         mImageTextureType = mState.imageIndex.getType();
-        mImageLevel       = mState.imageIndex.getLevelIndex();
+        mImageLevel       = gl::LevelIndex(mState.imageIndex.getLevelIndex());
         mImageLayer       = mState.imageIndex.hasLayer() ? mState.imageIndex.getLayerIndex() : 0;
     }
     else
@@ -108,7 +108,7 @@ egl::Error ImageVk::initialize(const egl::Display *display)
         mOwnsImage = false;
 
         mImageTextureType = gl::TextureType::_2D;
-        mImageLevel       = 0;
+        mImageLevel       = gl::LevelIndex(0);
         mImageLayer       = 0;
     }
 

src/libANGLE/renderer/vulkan/ImageVk.h

@@ -40,12 +40,12 @@ class ImageVk : public ImageImpl
 
     vk::ImageHelper *getImage() const { return mImage; }
     gl::TextureType getImageTextureType() const { return mImageTextureType; }
-    uint32_t getImageLevel() const { return mImageLevel; }
+    gl::LevelIndex getImageLevel() const { return mImageLevel; }
     uint32_t getImageLayer() const { return mImageLayer; }
 
   private:
     gl::TextureType mImageTextureType;
-    uint32_t mImageLevel;
+    gl::LevelIndex mImageLevel;
     uint32_t mImageLayer;
 
     bool mOwnsImage;

src/libANGLE/renderer/vulkan/MemoryObjectVk.cpp

@@ -204,10 +204,11 @@ angle::Result MemoryObjectVk::createImage(ContextVk *contextVk,
     // ANGLE_external_objects_flags allows create flags to be specified by the application instead
     // of getting defaulted to zero.  Note that the GL enum values constituting the bits of
     // |createFlags| are identical to their corresponding Vulkan value.
-    ANGLE_TRY(image->initExternal(
-        contextVk, type, vkExtents, vkFormat, 1, imageUsageFlags, createFlags,
-        vk::ImageLayout::Undefined, &externalMemoryImageCreateInfo, 0,
-        static_cast<uint32_t>(levels) - 1, static_cast<uint32_t>(levels), layerCount));
+    ANGLE_TRY(image->initExternal(contextVk, type, vkExtents, vkFormat, 1, imageUsageFlags,
+                                  createFlags, vk::ImageLayout::Undefined,
+                                  &externalMemoryImageCreateInfo, gl::LevelIndex(0),
+                                  gl::LevelIndex(static_cast<uint32_t>(levels) - 1),
+                                  static_cast<uint32_t>(levels), layerCount));
 
     VkMemoryRequirements externalMemoryRequirements;
     image->getImage().getMemoryRequirements(renderer->getDevice(), &externalMemoryRequirements);

src/libANGLE/renderer/vulkan/OverlayVk.cpp

@@ -110,13 +110,13 @@ angle::Result OverlayVk::createFont(ContextVk *contextVk)
         mFontImage.init(contextVk, gl::TextureType::_2D,
                         VkExtent3D{gl::overlay::kFontImageWidth, gl::overlay::kFontImageHeight, 1},
                         renderer->getFormat(angle::FormatID::R8_UNORM), 1,
-                        VK_IMAGE_USAGE_TRANSFER_DST_BIT | VK_IMAGE_USAGE_SAMPLED_BIT, 0, 0, 1,
-                        gl::overlay::kFontCount));
+                        VK_IMAGE_USAGE_TRANSFER_DST_BIT | VK_IMAGE_USAGE_SAMPLED_BIT,
+                        gl::LevelIndex(0), gl::LevelIndex(0), 1, gl::overlay::kFontCount));
     ANGLE_TRY(mFontImage.initMemory(contextVk, renderer->getMemoryProperties(),
                                     VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT));
     ANGLE_TRY(mFontImage.initImageView(contextVk, gl::TextureType::_2DArray,
                                        VK_IMAGE_ASPECT_COLOR_BIT, gl::SwizzleState(),
-                                       &mFontImageView, 0, 1));
+                                       &mFontImageView, vk::LevelIndex(0), 1));
 
     // Copy font data from staging buffer.
     ANGLE_TRY(contextVk->onBufferTransferRead(&fontDataBuffer.get()));
@@ -177,13 +177,13 @@ angle::Result OverlayVk::cullWidgets(ContextVk *contextVk)
 
     ANGLE_TRY(mCulledWidgets.init(contextVk, gl::TextureType::_2D, culledWidgetsExtent,
                                   renderer->getFormat(angle::FormatID::R32G32_UINT), 1,
-                                  VK_IMAGE_USAGE_STORAGE_BIT | VK_IMAGE_USAGE_SAMPLED_BIT, 0, 0, 1,
-                                  1));
+                                  VK_IMAGE_USAGE_STORAGE_BIT | VK_IMAGE_USAGE_SAMPLED_BIT,
+                                  gl::LevelIndex(0), gl::LevelIndex(0), 1, 1));
     ANGLE_TRY(mCulledWidgets.initMemory(contextVk, renderer->getMemoryProperties(),
                                         VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT));
     ANGLE_TRY(mCulledWidgets.initImageView(contextVk, gl::TextureType::_2D,
                                            VK_IMAGE_ASPECT_COLOR_BIT, gl::SwizzleState(),
-                                           &mCulledWidgetsView, 0, 1));
+                                           &mCulledWidgetsView, vk::LevelIndex(0), 1));
 
     UtilsVk::OverlayCullParameters params;
     params.subgroupSize[0]            = mSubgroupSize[0];

src/libANGLE/renderer/vulkan/RenderTargetVk.cpp

@@ -41,7 +41,7 @@ void RenderTargetVk::init(vk::ImageHelper *image,
                           vk::ImageViewHelper *imageViews,
                           vk::ImageHelper *resolveImage,
                           vk::ImageViewHelper *resolveImageViews,
-                          uint32_t levelIndexGL,
+                          gl::LevelIndex levelIndexGL,
                           uint32_t layerIndex,
                           bool isImageTransient)
 {
@@ -64,7 +64,7 @@ void RenderTargetVk::reset()
     mImageViews        = nullptr;
     mResolveImage      = nullptr;
     mResolveImageViews = nullptr;
-    mLevelIndexGL      = 0;
+    mLevelIndexGL      = gl::LevelIndex(0);
     mLayerIndex        = 0;
     mContentDefined    = false;
 }
@@ -74,7 +74,7 @@ vk::ImageViewSubresourceSerial RenderTargetVk::getSubresourceSerialImpl(
 {
     ASSERT(imageViews);
     ASSERT(mLayerIndex < std::numeric_limits<uint16_t>::max());
-    ASSERT(mLevelIndexGL < std::numeric_limits<uint16_t>::max());
+    ASSERT(mLevelIndexGL.get() < std::numeric_limits<uint16_t>::max());
 
     vk::ImageViewSubresourceSerial imageViewSerial =
         imageViews->getSubresourceSerial(mLevelIndexGL, 1, mLayerIndex, vk::LayerMode::Single);
@@ -165,7 +165,7 @@ angle::Result RenderTargetVk::getImageViewImpl(ContextVk *contextVk,
                                                const vk::ImageView **imageViewOut) const
 {
     ASSERT(image.valid() && imageViews);
-    int32_t levelVK = mLevelIndexGL - mImage->getBaseLevel();
+    vk::LevelIndex levelVK = mImage->toVKLevel(mLevelIndexGL);
     return imageViews->getLevelLayerDrawImageView(contextVk, image, levelVK, mLayerIndex,
                                                   imageViewOut);
 }
@@ -224,7 +224,7 @@ const vk::Format &RenderTargetVk::getImageFormat() const
 gl::Extents RenderTargetVk::getExtents() const
 {
     ASSERT(mImage && mImage->valid());
-    uint32_t levelVK = mLevelIndexGL - mImage->getBaseLevel();
+    vk::LevelIndex levelVK = mImage->toVKLevel(mLevelIndexGL);
     return mImage->getLevelExtents2D(levelVK);
 }
 
@@ -306,16 +306,16 @@ gl::ImageIndex RenderTargetVk::getImageIndex() const
     // Determine the GL type from the Vk Image properties.
     if (mImage->getType() == VK_IMAGE_TYPE_3D)
     {
-        return gl::ImageIndex::Make3D(mLevelIndexGL, mLayerIndex);
+        return gl::ImageIndex::Make3D(mLevelIndexGL.get(), mLayerIndex);
     }
 
     // We don't need to distinguish 2D array and cube.
     if (mImage->getLayerCount() > 1)
     {
-        return gl::ImageIndex::Make2DArray(mLevelIndexGL, mLayerIndex);
+        return gl::ImageIndex::Make2DArray(mLevelIndexGL.get(), mLayerIndex);
     }
 
     ASSERT(mLayerIndex == 0);
-    return gl::ImageIndex::Make2D(mLevelIndexGL);
+    return gl::ImageIndex::Make2D(mLevelIndexGL.get());
 }
 }  // namespace rx

src/libANGLE/renderer/vulkan/RenderTargetVk.h

@@ -46,7 +46,7 @@ class RenderTargetVk final : public FramebufferAttachmentRenderTarget
               vk::ImageViewHelper *imageViews,
               vk::ImageHelper *resolveImage,
               vk::ImageViewHelper *resolveImageViews,
-              uint32_t levelIndexGL,
+              gl::LevelIndex levelIndexGL,
               uint32_t layerIndex,
               bool isImageTransient);
     void reset();
@@ -79,7 +79,7 @@ class RenderTargetVk final : public FramebufferAttachmentRenderTarget
 
     const vk::Format &getImageFormat() const;
     gl::Extents getExtents() const;
-    uint32_t getLevelIndex() const { return mLevelIndexGL; }
+    gl::LevelIndex getLevelIndex() const { return mLevelIndexGL; }
     uint32_t getLayerIndex() const { return mLayerIndex; }
 
     gl::ImageIndex getImageIndex() const;
@@ -133,7 +133,7 @@ class RenderTargetVk final : public FramebufferAttachmentRenderTarget
     vk::ImageViewHelper *mResolveImageViews;
 
     // Which subresource of the image is used as render target.
-    uint32_t mLevelIndexGL;
+    gl::LevelIndex mLevelIndexGL;
     uint32_t mLayerIndex;
 
     // Whether the render target has been invalidated.  If so, DONT_CARE is used instead of LOAD for

src/libANGLE/renderer/vulkan/RenderbufferVk.cpp

@@ -88,12 +88,13 @@ angle::Result RenderbufferVk::setStorageImpl(const gl::Context *context,
 
         VkExtent3D extents = {static_cast<uint32_t>(width), static_cast<uint32_t>(height), 1u};
         ANGLE_TRY(mImage->init(contextVk, gl::TextureType::_2D, extents, vkFormat,
-                               static_cast<uint32_t>(samples), usage, 0, 0, 1, 1));
+                               static_cast<uint32_t>(samples), usage, gl::LevelIndex(0),
+                               gl::LevelIndex(0), 1, 1));
 
         VkMemoryPropertyFlags flags = VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT;
         ANGLE_TRY(mImage->initMemory(contextVk, renderer->getMemoryProperties(), flags));
 
-        mRenderTarget.init(mImage, &mImageViews, nullptr, nullptr, 0, 0, false);
+        mRenderTarget.init(mImage, &mImageViews, nullptr, nullptr, gl::LevelIndex(0), 0, false);
     }
 
     return angle::Result::Continue;
@@ -245,8 +246,8 @@ angle::Result RenderbufferVk::getRenderbufferImage(const gl::Context *context,
     gl::MaybeOverrideLuminance(format, type, getColorReadFormat(context),
                                getColorReadType(context));
 
-    return mImage->readPixelsForGetImage(contextVk, packState, packBuffer, 0, 0, format, type,
-                                         pixels);
+    return mImage->readPixelsForGetImage(contextVk, packState, packBuffer, gl::LevelIndex(0), 0,
+                                         format, type, pixels);
 }
 
 void RenderbufferVk::onSubjectStateChange(angle::SubjectIndex index, angle::SubjectMessage message)