Vulkan: Optimize respecifying an image

src/libANGLE/renderer/vulkan/TextureVk.cpp

@@ -1937,6 +1937,10 @@ angle::Result TextureVk::copyAndStageImageData(ContextVk *contextVk,
     // levels outside the range of the Vulkan image will remain intact.
     RendererVk *renderer = contextVk->getRenderer();
 
+    // This path is only called when switching from !owned to owned, in which case if any level was
+    // redefined it's already released and deleted by TextureVk::redefineLevel().
+    ASSERT(!mRedefinedLevels.any());
+
     // Create a temp copy of srcImage for staging.
     std::unique_ptr<vk::RefCounted<vk::ImageHelper>> stagingImage;
     stagingImage = std::make_unique<vk::RefCounted<vk::ImageHelper>>();
@@ -1967,15 +1971,6 @@ angle::Result TextureVk::copyAndStageImageData(ContextVk *contextVk,
 
     for (vk::LevelIndex levelVk(0); levelVk < vk::LevelIndex(levelCount); ++levelVk)
     {
-        if (mRedefinedLevels.test(levelVk.get()))
-        {
-            // Note: if this level is incompatibly redefined, there will necessarily be a
-            // staged update, and the contents of the image are to be thrown away.
-            ASSERT(srcImage->hasStagedUpdatesForSubresource(
-                vk_gl::GetLevelIndex(levelVk, previousFirstAllocateLevel), 0, layerCount));
-            continue;
-        }
-
         gl::Extents levelExtents = srcImage->getLevelExtents(levelVk);
 
         copyRegion.srcSubresource.mipLevel = levelVk.get();
@@ -1988,8 +1983,8 @@ angle::Result TextureVk::copyAndStageImageData(ContextVk *contextVk,
     }
 
     // Stage the staging image in the destination
-    dstImage->stageSubresourceUpdatesFromAllImageLevels(
-        renderer, stagingImage.release(), previousFirstAllocateLevel, mRedefinedLevels);
+    dstImage->stageSubresourceUpdatesFromAllImageLevels(stagingImage.release(),
+                                                        previousFirstAllocateLevel);
 
     return angle::Result::Continue;
 }
@@ -2065,14 +2060,11 @@ angle::Result TextureVk::respecifyImageStorageAndLevels(ContextVk *contextVk,
     }
     else
     {
-        // TODO: Make the image stage itself as an update to its levels.  http://anglebug.com/4835
-
-        // Make a copy of the current image and stage that as an update to the new image.
-        ANGLE_TRY(copyAndStageImageData(contextVk, previousFirstAllocateLevel, mImage, mImage));
+        // Make the image stage itself as updates to its levels.
+        mImage->stageSelfAsSubresourceUpdates(contextVk, mImage->getLevelCount(), mRedefinedLevels);
 
-        // Now that we've staged all the updates, release the current image so that it will be
-        // recreated with the correct number of mip levels, base level, and max level.
-        // Do this iff we owned the image and didn't create a new one.
+        // Release the current image so that it will be recreated with the correct number of mip
+        // levels, base level, and max level.
         releaseImage(contextVk);
 
         if (!mState.getImmutableFormat())
@@ -2423,8 +2415,8 @@ angle::Result TextureVk::syncState(const gl::Context *context,
     // For immutable texture, base level does not affect allocation. Only usage flags are. If usage
     // flag changed, we respecify image storage early on. This makes the code more reliable and also
     // better performance wise. Otherwise, we will try to preserve base level by calling
-    // stageSelfForBaseLevel and then later on find out the mImageUsageFlags changed and the whole
-    // thing has to be respecified.
+    // stageSelfAsSubresourceUpdates and then later on find out the mImageUsageFlags changed and the
+    // whole thing has to be respecified.
     if (mState.getImmutableFormat() &&
         (oldUsageFlags != mImageUsageFlags || oldCreateFlags != mImageCreateFlags))
     {
@@ -2463,14 +2455,14 @@ angle::Result TextureVk::syncState(const gl::Context *context,
     {
         ASSERT(mOwnsImage);
         // Immutable texture is not expected to reach here. The usage flag change should have
-        // handled earlier and level count change should not need to reallocate
+        // been handled earlier and level count change should not need to reallocate
         ASSERT(!mState.getImmutableFormat());
 
         // Flush staged updates to the base level of the image.  Note that updates to the rest of
         // the levels have already been discarded through the |removeStagedUpdates| call above.
         ANGLE_TRY(flushImageStagedUpdates(contextVk));
 
-        mImage->stageSelfForBaseLevel(contextVk);
+        mImage->stageSelfAsSubresourceUpdates(contextVk, 1, {});
 
         // Release views and render targets created for the old image.
         releaseImage(contextVk);

src/libANGLE/renderer/vulkan/vk_helpers.cpp

@@ -5764,21 +5764,11 @@ void ImageHelper::stageSubresourceUpdateFromImage(RefCounted<ImageHelper> *image
     appendSubresourceUpdate(updateLevelGL, SubresourceUpdate(image, copyToImage));
 }
 
-void ImageHelper::stageSubresourceUpdatesFromAllImageLevels(RendererVk *renderer,
-                                                            RefCounted<ImageHelper> *image,
-                                                            gl::LevelIndex baseLevel,
-                                                            gl::TexLevelMask skipLevelsMask)
+void ImageHelper::stageSubresourceUpdatesFromAllImageLevels(RefCounted<ImageHelper> *image,
+                                                            gl::LevelIndex baseLevel)
 {
-    uint32_t levelsStaged = 0;
-
     for (LevelIndex levelVk(0); levelVk < LevelIndex(image->get().getLevelCount()); ++levelVk)
     {
-        if (skipLevelsMask.test(levelVk.get()))
-        {
-            continue;
-        }
-        ++levelsStaged;
-
         const gl::LevelIndex levelGL = vk_gl::GetLevelIndex(levelVk, baseLevel);
         const gl::ImageIndex index =
             gl::ImageIndex::Make2DArrayRange(levelGL.get(), 0, image->get().getLayerCount());
@@ -5787,15 +5777,6 @@ void ImageHelper::stageSubresourceUpdatesFromAllImageLevels(RendererVk *renderer
                                         image->get().getLevelExtents(levelVk),
                                         image->get().getType());
     }
-
-    // TODO: remove skipLevelsMask and this code after optimizing image-respecify to stage itself
-    // (instead of a copy).  http://anglebug.com/4835
-    if (levelsStaged == 0)
-    {
-        image->get().releaseImage(renderer);
-        image->get().releaseStagingBuffer(renderer);
-        SafeDelete(image);
-    }
 }
 
 void ImageHelper::stageClear(const gl::ImageIndex &index,
@@ -5905,8 +5886,17 @@ void ImageHelper::stageClearIfEmulatedFormat(bool isRobustResourceInitEnabled)
     }
 }
 
-void ImageHelper::stageSelfForBaseLevel(ContextVk *contextVk)
+void ImageHelper::stageSelfAsSubresourceUpdates(ContextVk *contextVk,
+                                                uint32_t levelCount,
+                                                gl::TexLevelMask skipLevelsMask)
+
 {
+    // Nothing to do if every level must be skipped
+    if ((~skipLevelsMask & gl::TexLevelMask(angle::Bit<uint32_t>(levelCount) - 1)).none())
+    {
+        return;
+    }
+
     // Because we are cloning this object to another object, we must finalize the layout if it is
     // being used by current renderpass as attachment. Otherwise we are copying the incorrect layout
     // since it is determined at endRenderPass time.
@@ -5922,14 +5912,14 @@ void ImageHelper::stageSelfForBaseLevel(ContextVk *contextVk)
     prevImage->get().mImage        = std::move(mImage);
     prevImage->get().mDeviceMemory = std::move(mDeviceMemory);
 
-    // Barrier information.  Note: mLevelCount is set to 1 so that only the base level is
-    // transitioned when flushing the update.
+    // Barrier information.  Note: mLevelCount is set to levelCount so that only the necessary
+    // levels are transitioned when flushing the update.
     prevImage->get().mFormat                      = mFormat;
     prevImage->get().mCurrentLayout               = mCurrentLayout;
     prevImage->get().mCurrentQueueFamilyIndex     = mCurrentQueueFamilyIndex;
     prevImage->get().mLastNonShaderReadOnlyLayout = mLastNonShaderReadOnlyLayout;
     prevImage->get().mCurrentShaderReadStageMask  = mCurrentShaderReadStageMask;
-    prevImage->get().mLevelCount                  = 1;
+    prevImage->get().mLevelCount                  = levelCount;
     prevImage->get().mLayerCount                  = mLayerCount;
     prevImage->get().mImageSerial                 = mImageSerial;
 
@@ -5942,11 +5932,23 @@ void ImageHelper::stageSelfForBaseLevel(ContextVk *contextVk)
 
     setEntireContentUndefined();
 
-    // Stage an update from the previous image.
-    const gl::ImageIndex firstAllocateLevelIndex =
-        gl::ImageIndex::Make2DArrayRange(mFirstAllocatedLevel.get(), 0, mLayerCount);
-    stageSubresourceUpdateFromImage(prevImage.release(), firstAllocateLevelIndex, LevelIndex(0),
-                                    gl::kOffsetZero, getLevelExtents(LevelIndex(0)), mImageType);
+    // Stage updates from the previous image.
+    for (LevelIndex levelVk(0); levelVk < LevelIndex(levelCount); ++levelVk)
+    {
+        if (skipLevelsMask.test(levelVk.get()))
+        {
+            continue;
+        }
+
+        const gl::ImageIndex index =
+            gl::ImageIndex::Make2DArrayRange(toGLLevel(levelVk).get(), 0, mLayerCount);
+
+        stageSubresourceUpdateFromImage(prevImage.get(), index, levelVk, gl::kOffsetZero,
+                                        getLevelExtents(levelVk), mImageType);
+    }
+
+    ASSERT(levelCount > 0);
+    prevImage.release();
 }
 
 angle::Result ImageHelper::flushSingleSubresourceStagedUpdates(ContextVk *contextVk,

src/libANGLE/renderer/vulkan/vk_helpers.h

@@ -1696,10 +1696,8 @@ class ImageHelper final : public Resource, public angle::Subject
 
     // Takes an image and stages a subresource update for each level of it, including its full
     // extent and all its layers, at the specified GL level.
-    void stageSubresourceUpdatesFromAllImageLevels(RendererVk *renderer,
-                                                   RefCounted<ImageHelper> *image,
-                                                   gl::LevelIndex baseLevel,
-                                                   gl::TexLevelMask skipLevelsMask);
+    void stageSubresourceUpdatesFromAllImageLevels(RefCounted<ImageHelper> *image,
+                                                   gl::LevelIndex baseLevel);
 
     // Stage a clear to an arbitrary value.
     void stageClear(const gl::ImageIndex &index,
@@ -1713,9 +1711,14 @@ class ImageHelper final : public Resource, public angle::Subject
                                                      const Format &format);
     void stageRobustResourceClear(const gl::ImageIndex &index);
 
-    // Stage the currently allocated image as an update to base level, making this !valid().  This
-    // is used for mipmap generation.
-    void stageSelfForBaseLevel(ContextVk *contextVk);
+    // Stage the currently allocated image as updates to base level and on, making this !valid().
+    // This is used for:
+    //
+    // - Mipmap generation, where levelCount is 1 so only the base level is retained
+    // - Image respecification, where every level (other than those explicitly skipped) is staged
+    void stageSelfAsSubresourceUpdates(ContextVk *contextVk,
+                                       uint32_t levelCount,
+                                       gl::TexLevelMask skipLevelsMask);
 
     // Flush staged updates for a single subresource. Can optionally take a parameter to defer
     // clears to a subsequent RenderPass load op.