Vulkan: Fix mipmap generation and level redefinition

When generating mipmaps, the non-base levels are redefined to be
compatible.  mRedefinedLevels was not updated to take this into account,
resulting in invalid copies to the image.

Additionally, noted a few spots where ImageDesc is used to respecify the
image, but those are not up-to-date when the backend functions are
called.  Changed those to directly get the necessary information from
the allocated image.

Bug: chromium:1094644
Bug: chromium:1094599
Change-Id: I2afc9e5a53f24ef56836c5d7eec2e3e11df0ef61
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2267423
Reviewed-by: Cody Northrop <cnorthrop@google.com>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>

src/common/bitset_utils.h

@@ -82,6 +82,8 @@ class BitSetT final
         std::size_t mCurrentBit;
     };
 
+    using value_type = BitsT;
+
     BitSetT();
     constexpr explicit BitSetT(BitsT value);
 
@@ -455,8 +457,8 @@ BitSetT<N, BitsT, ParamT>::Iterator::Iterator(const BitSetT &bits) : mBitsCopy(b
 }
 
 template <size_t N, typename BitsT, typename ParamT>
-ANGLE_INLINE typename BitSetT<N, BitsT, ParamT>::Iterator &BitSetT<N, BitsT, ParamT>::Iterator::
-operator++()
+ANGLE_INLINE typename BitSetT<N, BitsT, ParamT>::Iterator &
+BitSetT<N, BitsT, ParamT>::Iterator::operator++()
 {
     ASSERT(mBitsCopy.any());
     mBitsCopy.reset(static_cast<ParamT>(mCurrentBit));

src/libANGLE/renderer/vulkan/TextureVk.cpp

@@ -1396,13 +1396,12 @@ angle::Result TextureVk::generateMipmap(const gl::Context *context)
 }
 
 angle::Result TextureVk::copyAndStageImageSubresource(ContextVk *contextVk,
-                                                      const gl::ImageDesc &desc,
                                                       bool ignoreLayerCount,
                                                       uint32_t currentLayer,
                                                       uint32_t srcLevelVk,
                                                       uint32_t dstLevelGL)
 {
-    const gl::Extents &baseLevelExtents = desc.size;
+    const gl::Extents &baseLevelExtents = mImage->getLevelExtents(srcLevelVk);
 
     VkExtent3D updatedExtents;
     VkOffset3D offset = {};
@@ -1429,13 +1428,15 @@ angle::Result TextureVk::copyAndStageImageSubresource(ContextVk *contextVk,
     ASSERT(stagingBuffer);
     uint32_t bufferRowLength   = updatedExtents.width;
     uint32_t bufferImageHeight = updatedExtents.height;
-    if (desc.format.info->compressed)
+    const gl::InternalFormat &formatInfo =
+        gl::GetSizedInternalFormatInfo(mImage->getFormat().internalFormat);
+    if (formatInfo.compressed)
     {
         // In the case of a compressed texture, bufferRowLength can never be smaller than the
         // compressed format's compressed block width, and bufferImageHeight can never be smaller
         // than the compressed block height.
-        bufferRowLength   = std::max(bufferRowLength, desc.format.info->compressedBlockWidth);
-        bufferImageHeight = std::max(bufferImageHeight, desc.format.info->compressedBlockHeight);
+        bufferRowLength   = std::max(bufferRowLength, formatInfo.compressedBlockWidth);
+        bufferImageHeight = std::max(bufferImageHeight, formatInfo.compressedBlockHeight);
     }
     ANGLE_TRY(mImage->stageSubresourceUpdateFromBuffer(
         contextVk, bufferSize, dstLevelGL, currentLayer, layerCount, bufferRowLength,
@@ -1531,10 +1532,7 @@ angle::Result TextureVk::respecifyImageAttributesAndLevels(ContextVk *contextVk,
             // Pull data from the current image and stage it as an update for the new image
 
             // First we populate the staging buffer with current level data
-            const gl::ImageDesc &desc =
-                mState.getImageDesc(gl::TextureTypeToTarget(mState.getType(), layer), levelGL);
-
-            ANGLE_TRY(copyAndStageImageSubresource(contextVk, desc, true, layer, levelVK, levelGL));
+            ANGLE_TRY(copyAndStageImageSubresource(contextVk, true, layer, levelVK, levelGL));
         }
     }
 
@@ -1705,8 +1703,33 @@ angle::Result TextureVk::syncState(const gl::Context *context,
     bool isGenerateMipmap = source == gl::TextureCommand::GenerateMipmap;
     if (isGenerateMipmap)
     {
-        mImage->removeStagedUpdates(contextVk, mState.getEffectiveBaseLevel() + 1,
-                                    mState.getMipmapMaxLevel());
+        // Remove staged updates to the range that's being respecified (which is all the mips except
+        // mip 0).
+        uint32_t baseLevel = mState.getEffectiveBaseLevel() + 1;
+        uint32_t maxLevel  = mState.getMipmapMaxLevel();
+
+        mImage->removeStagedUpdates(contextVk, baseLevel, maxLevel);
+
+        // These levels are no longer incompatibly defined if they previously were.  The
+        // corresponding bits in mRedefinedLevels should be cleared.  Note that the texture may be
+        // simultaneously rebased, so mImage->getBaseLevel() and getEffectiveBaseLevel() may be
+        // different.
+        static_assert(gl::IMPLEMENTATION_MAX_TEXTURE_LEVELS < 32,
+                      "levels mask assumes 32-bits is enough");
+        gl::TexLevelMask::value_type levelsMask =
+            angle::Bit<uint32_t>(maxLevel + 1 - baseLevel) - 1;
+
+        uint32_t imageBaseLevel = mImage->getBaseLevel();
+        if (imageBaseLevel > baseLevel)
+        {
+            levelsMask >>= imageBaseLevel - baseLevel;
+        }
+        else
+        {
+            levelsMask <<= baseLevel - imageBaseLevel;
+        }
+
+        mRedefinedLevels &= gl::TexLevelMask(~levelsMask);
     }
 
     // Set base and max level before initializing the image
@@ -1735,7 +1758,16 @@ angle::Result TextureVk::syncState(const gl::Context *context,
     if (oldUsageFlags != mImageUsageFlags || oldCreateFlags != mImageCreateFlags ||
         mRedefinedLevels.any() || isMipmapEnabledByMinFilter)
     {
-        ANGLE_TRY(respecifyImageAttributes(contextVk));
+        // If generating mipmap and base level is incompatibly redefined, the image is going to be
+        // recreated.  Don't try to preserve the other mips.
+        if (isGenerateMipmap && mRedefinedLevels.test(0))
+        {
+            releaseImage(contextVk);
+        }
+        else
+        {
+            ANGLE_TRY(respecifyImageAttributes(contextVk));
+        }
     }
 
     // If generating mipmaps and the image is not full-mip already, make sure it's recreated to the

src/libANGLE/renderer/vulkan/TextureVk.h

@@ -345,7 +345,6 @@ class TextureVk : public TextureImpl, public angle::ObserverInterface
     // each subresource of the image and stage it for another subresource.  When all subresources
     // are taken care of, the image is recreated.
     angle::Result copyAndStageImageSubresource(ContextVk *contextVk,
-                                               const gl::ImageDesc &desc,
                                                bool ignoreLayerCount,
                                                uint32_t currentLayer,
                                                uint32_t srcLevelVk,

src/tests/gl_tests/TextureTest.cpp

@@ -2794,6 +2794,191 @@ TEST_P(Texture2DBaseMaxTestES3, RedefineEveryLevelToAnotherFormat)
     }
 }
 
+// Test that generating mipmaps after incompatibly redefining a level works.
+TEST_P(Texture2DBaseMaxTestES3, GenerateMipmapAfterRedefine)
+{
+    initTest();
+
+    // Test that all mips have the expected data initially (this makes sure the texture image is
+    // created already).
+    for (uint32_t lod = 0; lod < kMipCount; ++lod)
+    {
+        setLodUniform(lod);
+        drawQuad(mProgram, essl3_shaders::PositionAttrib(), 0.5f);
+        EXPECT_PIXEL_COLOR_EQ(0, 0, kMipColors[lod]);
+    }
+
+    // Redefine level 1 (any level would do other than 0) to an incompatible size, say the same size
+    // as level 0.
+    const GLColor kNewMipColor = GLColor::yellow;
+    std::array<GLColor, getMipDataSize(kMip0Size, 0)> newMipData;
+    std::fill(newMipData.begin(), newMipData.end(), kNewMipColor);
+
+    glTexImage2D(GL_TEXTURE_2D, 1, GL_RGBA8, kMip0Size, kMip0Size, 0, GL_RGBA, GL_UNSIGNED_BYTE,
+                 newMipData.data());
+
+    // Generate mipmaps.  This should redefine level 1 back to being compatible with level 0.
+    glGenerateMipmap(GL_TEXTURE_2D);
+
+    // Test that the texture looks as expected.
+    const int w = getWindowWidth() - 1;
+    const int h = getWindowHeight() - 1;
+    for (uint32_t lod = 0; lod < kMipCount; ++lod)
+    {
+        setLodUniform(lod);
+        drawQuad(mProgram, essl3_shaders::PositionAttrib(), 0.5f);
+        EXPECT_PIXEL_COLOR_EQ(0, 0, kMipColors[0]);
+        EXPECT_PIXEL_COLOR_EQ(w, 0, kMipColors[0]);
+        EXPECT_PIXEL_COLOR_EQ(0, h, kMipColors[0]);
+        EXPECT_PIXEL_COLOR_EQ(w, h, kMipColors[0]);
+    }
+}
+
+// Test that generating mipmaps after incompatibly redefining a level while simultaneously changing
+// the base level works.
+TEST_P(Texture2DBaseMaxTestES3, GenerateMipmapAfterRedefineAndRebase)
+{
+    ANGLE_SKIP_TEST_IF(IsWindows() && IsAMD() && IsOpenGL());
+
+    initTest();
+
+    // Test that all mips have the expected data initially (this makes sure the texture image is
+    // created already).
+    for (uint32_t lod = 0; lod < kMipCount; ++lod)
+    {
+        setLodUniform(lod);
+        drawQuad(mProgram, essl3_shaders::PositionAttrib(), 0.5f);
+        EXPECT_PIXEL_COLOR_EQ(0, 0, kMipColors[lod]);
+    }
+
+    // Redefine level 2 to an incompatible size, say the same size as level 0.
+    const GLColor kNewMipColor = GLColor::yellow;
+    std::array<GLColor, getMipDataSize(kMip0Size, 0)> newMipData;
+    std::fill(newMipData.begin(), newMipData.end(), kNewMipColor);
+
+    glTexImage2D(GL_TEXTURE_2D, 2, GL_RGBA8, kMip0Size, kMip0Size, 0, GL_RGBA, GL_UNSIGNED_BYTE,
+                 newMipData.data());
+
+    // Set base level of the texture to 1 then generate mipmaps.  Level 2 that's redefined should
+    // go back to being compatibly defined.
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_BASE_LEVEL, 1);
+    glGenerateMipmap(GL_TEXTURE_2D);
+
+    // Test that the texture looks as expected.
+    const int w = getWindowWidth() - 1;
+    const int h = getWindowHeight() - 1;
+    for (uint32_t lod = 0; lod < kMipCount; ++lod)
+    {
+        setLodUniform(lod);
+        drawQuad(mProgram, essl3_shaders::PositionAttrib(), 0.5f);
+        EXPECT_PIXEL_COLOR_EQ(0, 0, kMipColors[1]);
+        EXPECT_PIXEL_COLOR_EQ(w, 0, kMipColors[1]);
+        EXPECT_PIXEL_COLOR_EQ(0, h, kMipColors[1]);
+        EXPECT_PIXEL_COLOR_EQ(w, h, kMipColors[1]);
+    }
+
+    // Redefine level 1 (current base level) to an incompatible size.
+    glTexImage2D(GL_TEXTURE_2D, 1, GL_RGBA8, kMip0Size, kMip0Size, 0, GL_RGBA, GL_UNSIGNED_BYTE,
+                 newMipData.data());
+
+    // Set base level of the texture back to 0 then generate mipmaps.  Level 1 should go back to
+    // being compatibly defined.
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_BASE_LEVEL, 0);
+    glGenerateMipmap(GL_TEXTURE_2D);
+
+    // Test that the texture looks as expected.
+    for (uint32_t lod = 0; lod < kMipCount; ++lod)
+    {
+        setLodUniform(lod);
+        drawQuad(mProgram, essl3_shaders::PositionAttrib(), 0.5f);
+        EXPECT_PIXEL_COLOR_EQ(0, 0, kMipColors[0]);
+        EXPECT_PIXEL_COLOR_EQ(w, 0, kMipColors[0]);
+        EXPECT_PIXEL_COLOR_EQ(0, h, kMipColors[0]);
+        EXPECT_PIXEL_COLOR_EQ(w, h, kMipColors[0]);
+    }
+}
+
+// Test that generating mipmaps after incompatibly redefining the base level of the texture works.
+TEST_P(Texture2DBaseMaxTestES3, GenerateMipmapAfterRedefiningBase)
+{
+    initTest();
+
+    // Test that all mips have the expected data initially (this makes sure the texture image is
+    // created already).
+    for (uint32_t lod = 0; lod < kMipCount; ++lod)
+    {
+        setLodUniform(lod);
+        drawQuad(mProgram, essl3_shaders::PositionAttrib(), 0.5f);
+        EXPECT_PIXEL_COLOR_EQ(0, 0, kMipColors[lod]);
+    }
+
+    // Redefine level 0 to an incompatible size.
+    const GLColor kNewMipColor = GLColor::yellow;
+    std::array<GLColor, getMipDataSize(kMip0Size * 2, 0)> newMipData;
+    std::fill(newMipData.begin(), newMipData.end(), kNewMipColor);
+
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, kMip0Size * 2, kMip0Size * 2, 0, GL_RGBA,
+                 GL_UNSIGNED_BYTE, newMipData.data());
+
+    // Generate mipmaps.
+    glGenerateMipmap(GL_TEXTURE_2D);
+
+    // Test that the texture looks as expected.
+    const int w = getWindowWidth() - 1;
+    const int h = getWindowHeight() - 1;
+    for (uint32_t lod = 0; lod < kMipCount + 1; ++lod)
+    {
+        setLodUniform(lod);
+        drawQuad(mProgram, essl3_shaders::PositionAttrib(), 0.5f);
+        EXPECT_PIXEL_COLOR_EQ(0, 0, kNewMipColor);
+        EXPECT_PIXEL_COLOR_EQ(w, 0, kNewMipColor);
+        EXPECT_PIXEL_COLOR_EQ(0, h, kNewMipColor);
+        EXPECT_PIXEL_COLOR_EQ(w, h, kNewMipColor);
+    }
+}
+
+// Test that generating mipmaps after incompatibly redefining the base level while simultaneously
+// changing MAX_LEVEL works.
+TEST_P(Texture2DBaseMaxTestES3, GenerateMipmapAfterRedefiningBaseAndChangingMax)
+{
+    initTest();
+
+    // Test that all mips have the expected data initially (this makes sure the texture image is
+    // created already).
+    for (uint32_t lod = 0; lod < kMipCount; ++lod)
+    {
+        setLodUniform(lod);
+        drawQuad(mProgram, essl3_shaders::PositionAttrib(), 0.5f);
+        EXPECT_PIXEL_COLOR_EQ(0, 0, kMipColors[lod]);
+    }
+
+    // Redefine level 0 to an incompatible size.
+    const GLColor kNewMipColor = GLColor::yellow;
+    std::array<GLColor, getMipDataSize(kMip0Size * 2, 0)> newMipData;
+    std::fill(newMipData.begin(), newMipData.end(), kNewMipColor);
+
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA8, kMip0Size * 2, kMip0Size * 2, 0, GL_RGBA,
+                 GL_UNSIGNED_BYTE, newMipData.data());
+
+    // Set max level of the texture to 2 then generate mipmaps.
+    constexpr uint32_t kMaxLevel = 2;
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAX_LEVEL, kMaxLevel);
+    glGenerateMipmap(GL_TEXTURE_2D);
+
+    // Test that the texture looks as expected.
+    const int w = getWindowWidth() - 1;
+    const int h = getWindowHeight() - 1;
+    for (uint32_t lod = 0; lod <= kMaxLevel; ++lod)
+    {
+        setLodUniform(lod);
+        drawQuad(mProgram, essl3_shaders::PositionAttrib(), 0.5f);
+        EXPECT_PIXEL_COLOR_EQ(0, 0, kNewMipColor);
+        EXPECT_PIXEL_COLOR_EQ(w, 0, kNewMipColor);
+        EXPECT_PIXEL_COLOR_EQ(0, h, kNewMipColor);
+        EXPECT_PIXEL_COLOR_EQ(w, h, kNewMipColor);
+    }
+}
+
 // Test to check that texture completeness is determined correctly when the texture base level is
 // greater than 0, and also that level 0 is not sampled when base level is greater than 0.
 TEST_P(Texture2DTestES3, DrawWithBaseLevel1)