Vulkan: Make texture syncState aware of upcoming generateMipmap

src/libANGLE/State.cpp

@@ -3132,7 +3132,7 @@ angle::Result State::syncTextures(const Context *context)
         Texture *texture = mActiveTexturesCache[textureIndex];
         if (texture && texture->hasAnyDirtyBit())
         {
-            ANGLE_TRY(texture->syncState(context));
+            ANGLE_TRY(texture->syncState(context, TextureCommand::Other));
         }
     }
 
@@ -3150,7 +3150,7 @@ angle::Result State::syncImages(const Context *context)
         Texture *texture = mImageUnits[imageUnitIndex].texture.get();
         if (texture && texture->hasAnyDirtyBit())
         {
-            ANGLE_TRY(texture->syncState(context));
+            ANGLE_TRY(texture->syncState(context, TextureCommand::Other));
         }
     }
 
@@ -3306,7 +3306,7 @@ angle::Result State::onProgramExecutableChange(const Context *context, Program *
 
         if (image->hasAnyDirtyBit())
         {
-            ANGLE_TRY(image->syncState(context));
+            ANGLE_TRY(image->syncState(context, TextureCommand::Other));
         }
 
         if (mRobustResourceInit && image->initState() == InitState::MayNeedInit)
@@ -3351,7 +3351,7 @@ angle::Result State::onProgramPipelineExecutableChange(const Context *context,
 
         if (image->hasAnyDirtyBit())
         {
-            ANGLE_TRY(image->syncState(context));
+            ANGLE_TRY(image->syncState(context, TextureCommand::Other));
         }
 
         if (mRobustResourceInit && image->initState() == InitState::MayNeedInit)

src/libANGLE/Texture.cpp

@@ -1501,10 +1501,7 @@ angle::Result Texture::generateMipmap(Context *context)
         return angle::Result::Continue;
     }
 
-    if (hasAnyDirtyBit())
-    {
-        ANGLE_TRY(syncState(context));
-    }
+    ANGLE_TRY(syncState(context, TextureCommand::GenerateMipmap));
 
     // Clear the base image(s) immediately if needed
     if (context->isRobustResourceInitEnabled())
@@ -1526,7 +1523,7 @@ angle::Result Texture::generateMipmap(Context *context)
 
     ANGLE_TRY(mTexture->generateMipmap(context));
 
-    // Propagate the format and size of the bsae mip to the smaller ones. Cube maps are guaranteed
+    // Propagate the format and size of the base mip to the smaller ones. Cube maps are guaranteed
     // to have faces of the same size and format so any faces can be picked.
     const ImageDesc &baseImageInfo = mState.getImageDesc(mState.getBaseImageTarget(), baseLevel);
     mState.setImageDescChain(baseLevel, maxLevel, baseImageInfo.size, baseImageInfo.format,
@@ -1787,10 +1784,10 @@ GLuint Texture::getNativeID() const
     return mTexture->getNativeID();
 }
 
-angle::Result Texture::syncState(const Context *context)
+angle::Result Texture::syncState(const Context *context, TextureCommand source)
 {
-    ASSERT(hasAnyDirtyBit());
-    ANGLE_TRY(mTexture->syncState(context, mDirtyBits));
+    ASSERT(hasAnyDirtyBit() || source == TextureCommand::GenerateMipmap);
+    ANGLE_TRY(mTexture->syncState(context, mDirtyBits, source));
     mDirtyBits.reset();
     return angle::Result::Continue;
 }
@@ -2006,7 +2003,7 @@ angle::Result Texture::getTexImage(const Context *context,
 {
     if (hasAnyDirtyBit())
     {
-        ANGLE_TRY(syncState(context));
+        ANGLE_TRY(syncState(context, TextureCommand::Other));
     }
 
     return mTexture->getTexImage(context, packState, packBuffer, target, level, format, type,

src/libANGLE/Texture.h

@@ -99,6 +99,14 @@ struct ContextBindingCount
     uint32_t imageBindingCount;
 };
 
+// The source of the syncState call.  Knowing why syncState is being called can help the back-end
+// make more-informed decisions.
+enum class TextureCommand
+{
+    GenerateMipmap,
+    Other,
+};
+
 // State from Table 6.9 (state per texture object) in the OpenGL ES 3.0.2 spec.
 class TextureState final : private angle::NonCopyable
 {
@@ -580,7 +588,7 @@ class Texture final : public RefCountObject<TextureID>,
     };
     using DirtyBits = angle::BitSet<DIRTY_BIT_COUNT>;
 
-    angle::Result syncState(const Context *context);
+    angle::Result syncState(const Context *context, TextureCommand source);
     bool hasAnyDirtyBit() const { return mDirtyBits.any(); }
 
     // ObserverInterface implementation.

src/libANGLE/renderer/TextureImpl.h

@@ -187,7 +187,8 @@ class TextureImpl : public FramebufferAttachmentObjectImpl
     virtual GLint getNativeID() const;
 
     virtual angle::Result syncState(const gl::Context *context,
-                                    const gl::Texture::DirtyBits &dirtyBits) = 0;
+                                    const gl::Texture::DirtyBits &dirtyBits,
+                                    gl::TextureCommand source) = 0;
 
     virtual GLenum getColorReadFormat(const gl::Context *context);
     virtual GLenum getColorReadType(const gl::Context *context);

src/libANGLE/renderer/TextureImpl_mock.h

@@ -129,7 +129,10 @@ class MockTextureImpl : public TextureImpl
 
     MOCK_METHOD2(setBaseLevel, angle::Result(const gl::Context *, GLuint));
 
-    MOCK_METHOD2(syncState, angle::Result(const gl::Context *, const gl::Texture::DirtyBits &));
+    MOCK_METHOD3(syncState,
+                 angle::Result(const gl::Context *,
+                               const gl::Texture::DirtyBits &,
+                               gl::TextureCommand source));
 
     MOCK_METHOD0(destructor, void());
 

src/libANGLE/renderer/d3d/TextureD3D.cpp

@@ -681,7 +681,8 @@ angle::Result TextureD3D::setBaseLevel(const gl::Context *context, GLuint baseLe
 }
 
 angle::Result TextureD3D::syncState(const gl::Context *context,
-                                    const gl::Texture::DirtyBits &dirtyBits)
+                                    const gl::Texture::DirtyBits &dirtyBits,
+                                    gl::TextureCommand source)
 {
     // This could be improved using dirty bits.
     return angle::Result::Continue;

src/libANGLE/renderer/d3d/TextureD3D.h

@@ -107,7 +107,8 @@ class TextureD3D : public TextureImpl, public angle::ObserverInterface
     angle::Result setBaseLevel(const gl::Context *context, GLuint baseLevel) override;
 
     angle::Result syncState(const gl::Context *context,
-                            const gl::Texture::DirtyBits &dirtyBits) override;
+                            const gl::Texture::DirtyBits &dirtyBits,
+                            gl::TextureCommand source) override;
 
     angle::Result initializeContents(const gl::Context *context,
                                      const gl::ImageIndex &imageIndex) override;

src/libANGLE/renderer/gl/TextureGL.cpp

@@ -1382,7 +1382,8 @@ GLint TextureGL::getNativeID() const
 }
 
 angle::Result TextureGL::syncState(const gl::Context *context,
-                                   const gl::Texture::DirtyBits &dirtyBits)
+                                   const gl::Texture::DirtyBits &dirtyBits,
+                                   gl::TextureCommand source)
 {
     if (dirtyBits.none() && mLocalDirtyBits.none())
     {

src/libANGLE/renderer/gl/TextureGL.h

@@ -190,7 +190,8 @@ class TextureGL : public TextureImpl
     gl::TextureType getType() const;
 
     angle::Result syncState(const gl::Context *context,
-                            const gl::Texture::DirtyBits &dirtyBits) override;
+                            const gl::Texture::DirtyBits &dirtyBits,
+                            gl::TextureCommand source) override;
     bool hasAnyDirtyBit() const;
 
     angle::Result setBaseLevel(const gl::Context *context, GLuint baseLevel) override;

src/libANGLE/renderer/metal/TextureMtl.h

@@ -131,7 +131,8 @@ class TextureMtl : public TextureImpl
                                             FramebufferAttachmentRenderTarget **rtOut) override;
 
     angle::Result syncState(const gl::Context *context,
-                            const gl::Texture::DirtyBits &dirtyBits) override;
+                            const gl::Texture::DirtyBits &dirtyBits,
+                            gl::TextureCommand source) override;
 
     angle::Result setStorageMultisample(const gl::Context *context,
                                         gl::TextureType type,

src/libANGLE/renderer/metal/TextureMtl.mm

@@ -813,7 +813,8 @@ angle::Result TextureMtl::getAttachmentRenderTarget(const gl::Context *context,
 }
 
 angle::Result TextureMtl::syncState(const gl::Context *context,
-                                    const gl::Texture::DirtyBits &dirtyBits)
+                                    const gl::Texture::DirtyBits &dirtyBits,
+                                    gl::TextureCommand source)
 {
     if (dirtyBits.any())
     {

src/libANGLE/renderer/null/TextureNULL.cpp

@@ -173,7 +173,8 @@ angle::Result TextureNULL::releaseTexImage(const gl::Context *context)
 }
 
 angle::Result TextureNULL::syncState(const gl::Context *context,
-                                     const gl::Texture::DirtyBits &dirtyBits)
+                                     const gl::Texture::DirtyBits &dirtyBits,
+                                     gl::TextureCommand source)
 {
     return angle::Result::Continue;
 }

src/libANGLE/renderer/null/TextureNULL.h

@@ -118,7 +118,8 @@ class TextureNULL : public TextureImpl
     angle::Result releaseTexImage(const gl::Context *context) override;
 
     angle::Result syncState(const gl::Context *context,
-                            const gl::Texture::DirtyBits &dirtyBits) override;
+                            const gl::Texture::DirtyBits &dirtyBits,
+                            gl::TextureCommand source) override;
 
     angle::Result setStorageMultisample(const gl::Context *context,
                                         gl::TextureType type,

src/libANGLE/renderer/renderer_utils.cpp

@@ -621,7 +621,7 @@ angle::Result IncompleteTextureSet::getIncompleteTexture(
                                  GL_UNSIGNED_BYTE, color));
     }
 
-    ANGLE_TRY(t->syncState(context));
+    ANGLE_TRY(t->syncState(context, gl::TextureCommand::Other));
 
     mIncompleteTextures[type].set(context, t.release());
     *textureOut = mIncompleteTextures[type].get();

src/libANGLE/renderer/vulkan/TextureVk.cpp

@@ -1156,7 +1156,7 @@ angle::Result TextureVk::redefineLevel(const gl::Context *context,
         // override them with this call.
         uint32_t levelIndexGL = index.getLevelIndex();
         uint32_t layerIndex   = index.hasLayer() ? index.getLayerIndex() : 0;
-        mImage->removeStagedUpdates(contextVk, levelIndexGL, layerIndex);
+        mImage->removeSingleSubresourceStagedUpdates(contextVk, levelIndexGL, layerIndex);
 
         if (mImage->valid())
         {
@@ -1338,11 +1338,8 @@ angle::Result TextureVk::generateMipmapsWithCPU(const gl::Context *context)
 
 angle::Result TextureVk::generateMipmap(const gl::Context *context)
 {
-    ContextVk *contextVk   = vk::GetImpl(context);
-    RendererVk *renderer   = contextVk->getRenderer();
-    bool needRedefineImage = true;
-
-    const gl::ImageDesc &baseLevelDesc = mState.getBaseLevelDesc();
+    ContextVk *contextVk = vk::GetImpl(context);
+    RendererVk *renderer = contextVk->getRenderer();
 
     // The image should already be allocated by a prior syncState.
     ASSERT(mImage->valid());
@@ -1350,29 +1347,8 @@ angle::Result TextureVk::generateMipmap(const gl::Context *context)
     // If base level has changed, the front-end should have called syncState already.
     ASSERT(mImage->getBaseLevel() == mState.getEffectiveBaseLevel());
 
-    // Check whether the image is already full mipmap
-    if (mImage->getLevelCount() == getMipLevelCount(ImageMipLevels::FullMipChain))
-    {
-        needRedefineImage = false;
-    }
-
-    // Only staged update here can be the robust resource init.
-    ANGLE_TRY(ensureImageInitialized(contextVk, ImageMipLevels::EnabledLevels));
-
-    if (needRedefineImage)
-    {
-        // Redefine the images with mipmaps.
-        // Copy image to the staging buffer and stage an update to the new one.
-        ANGLE_TRY(copyAndStageImageSubresource(contextVk, baseLevelDesc, false,
-                                               getNativeImageLayer(0), 0, mImage->getBaseLevel()));
-
-        // Release the original image and recreate it with new mipmap counts.
-        releaseImage(contextVk);
-
-        mImage->retain(&contextVk->getResourceUseList());
-
-        ANGLE_TRY(ensureImageInitialized(contextVk, ImageMipLevels::FullMipChain));
-    }
+    // Only staged update here is the robust resource init if any.
+    ANGLE_TRY(ensureImageInitialized(contextVk, ImageMipLevels::FullMipChain));
 
     // Check if the image supports blit. If it does, we can do the mipmap generation on the gpu
     // only.
@@ -1668,7 +1644,8 @@ angle::Result TextureVk::initRenderTargets(ContextVk *contextVk,
 }
 
 angle::Result TextureVk::syncState(const gl::Context *context,
-                                   const gl::Texture::DirtyBits &dirtyBits)
+                                   const gl::Texture::DirtyBits &dirtyBits,
+                                   gl::TextureCommand source)
 {
     ContextVk *contextVk = vk::GetImpl(context);
 
@@ -1679,23 +1656,27 @@ angle::Result TextureVk::syncState(const gl::Context *context,
     if (dirtyBits.test(gl::Texture::DIRTY_BIT_BOUND_AS_IMAGE))
     {
         mImageCreateFlags |= VK_IMAGE_CREATE_MUTABLE_FORMAT_BIT;
-
-        // Recreate the image to include storage bit if needed.
-        if (!(mImageUsageFlags & VK_IMAGE_USAGE_STORAGE_BIT))
-        {
-            mImageUsageFlags |= VK_IMAGE_USAGE_STORAGE_BIT;
-        }
+        mImageUsageFlags |= VK_IMAGE_USAGE_STORAGE_BIT;
     }
 
     if (dirtyBits.test(gl::Texture::DIRTY_BIT_SRGB_OVERRIDE))
     {
-        if (!(mImageCreateFlags & VK_IMAGE_CREATE_MUTABLE_FORMAT_BIT) &&
-            mState.getSRGBOverride() != gl::SrgbOverride::Default)
+        if (mState.getSRGBOverride() != gl::SrgbOverride::Default)
         {
             mImageCreateFlags |= VK_IMAGE_CREATE_MUTABLE_FORMAT_BIT;
         }
     }
 
+    // Before redefining the image for any reason, check to see if it's about to go through mipmap
+    // generation.  In that case, drop every staged change for the subsequent mips after base, and
+    // make sure the image is created with the complete mipchain.
+    bool isGenerateMipmap = source == gl::TextureCommand::GenerateMipmap;
+    if (isGenerateMipmap)
+    {
+        mImage->removeStagedUpdates(contextVk, mState.getEffectiveBaseLevel() + 1,
+                                    mState.getEffectiveMaxLevel());
+    }
+
     // Set base and max level before initializing the image
     if (dirtyBits.test(gl::Texture::DIRTY_BIT_MAX_LEVEL) ||
         dirtyBits.test(gl::Texture::DIRTY_BIT_BASE_LEVEL))
@@ -1715,12 +1696,34 @@ angle::Result TextureVk::syncState(const gl::Context *context,
         ANGLE_TRY(respecifyImageAttributes(contextVk));
     }
 
+    // If generating mipmaps and the image is not full-mip already, make sure it's recreated to the
+    // correct levels.
+    if (isGenerateMipmap && mImage->valid() &&
+        mImage->getLevelCount() != getMipLevelCount(ImageMipLevels::FullMipChain))
+    {
+        // Redefine the image with mipmaps.
+        const gl::ImageDesc &baseLevelDesc = mState.getBaseLevelDesc();
+
+        // Copy image to the staging buffer and stage an update to the new one.
+        ANGLE_TRY(ensureImageInitialized(contextVk, ImageMipLevels::EnabledLevels));
+        ANGLE_TRY(copyAndStageImageSubresource(contextVk, baseLevelDesc, false,
+                                               getNativeImageLayer(0), 0, mImage->getBaseLevel()));
+
+        // Release the original image so it can be recreated with the new mipmap counts.
+        releaseImage(contextVk);
+
+        mImage->retain(&contextVk->getResourceUseList());
+    }
+
     // Initialize the image storage and flush the pixel buffer.
-    ANGLE_TRY(ensureImageInitialized(contextVk, ImageMipLevels::EnabledLevels));
+    ANGLE_TRY(ensureImageInitialized(contextVk, isGenerateMipmap ? ImageMipLevels::FullMipChain
+                                                                 : ImageMipLevels::EnabledLevels));
 
     // Mask out the IMPLEMENTATION dirty bit to avoid unnecessary syncs.
     gl::Texture::DirtyBits localBits = dirtyBits;
     localBits.reset(gl::Texture::DIRTY_BIT_IMPLEMENTATION);
+    localBits.reset(gl::Texture::DIRTY_BIT_BASE_LEVEL);
+    localBits.reset(gl::Texture::DIRTY_BIT_MAX_LEVEL);
 
     if (localBits.none() && mSampler.valid())
     {
@@ -1739,20 +1742,16 @@ angle::Result TextureVk::syncState(const gl::Context *context,
         localBits.test(gl::Texture::DIRTY_BIT_SWIZZLE_ALPHA) ||
         localBits.test(gl::Texture::DIRTY_BIT_SRGB_OVERRIDE))
     {
-        if (mImage && mImage->valid())
-        {
-            // We use a special layer count here to handle EGLImages. They might only be
-            // looking at one layer of a cube or 2D array texture.
-            uint32_t layerCount =
-                mState.getType() == gl::TextureType::_2D ? 1 : mImage->getLayerCount();
+        // We use a special layer count here to handle EGLImages. They might only be
+        // looking at one layer of a cube or 2D array texture.
+        uint32_t layerCount =
+            mState.getType() == gl::TextureType::_2D ? 1 : mImage->getLayerCount();
 
-            mImageViews.release(renderer);
-            const gl::ImageDesc &baseLevelDesc = mState.getBaseLevelDesc();
+        mImageViews.release(renderer);
+        const gl::ImageDesc &baseLevelDesc = mState.getBaseLevelDesc();
 
-            ANGLE_TRY(initImageViews(contextVk, mImage->getFormat(),
-                                     baseLevelDesc.format.info->sized, mImage->getLevelCount(),
-                                     layerCount));
-        }
+        ANGLE_TRY(initImageViews(contextVk, mImage->getFormat(), baseLevelDesc.format.info->sized,
+                                 mImage->getLevelCount(), layerCount));
     }
 
     vk::SamplerDesc samplerDesc(mState.getSamplerState(), mState.isStencilMode());

src/libANGLE/renderer/vulkan/TextureVk.h

@@ -140,7 +140,8 @@ class TextureVk : public TextureImpl, public angle::ObserverInterface
                                             FramebufferAttachmentRenderTarget **rtOut) override;
 
     angle::Result syncState(const gl::Context *context,
-                            const gl::Texture::DirtyBits &dirtyBits) override;
+                            const gl::Texture::DirtyBits &dirtyBits,
+                            gl::TextureCommand source) override;
 
     angle::Result setStorageMultisample(const gl::Context *context,
                                         gl::TextureType type,

src/libANGLE/renderer/vulkan/vk_helpers.cpp

@@ -3333,9 +3333,9 @@ void ImageHelper::resolve(ImageHelper *dest,
                                 VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, 1, &region);
 }
 
-void ImageHelper::removeStagedUpdates(ContextVk *contextVk,
-                                      uint32_t levelIndexGL,
-                                      uint32_t layerIndex)
+void ImageHelper::removeSingleSubresourceStagedUpdates(ContextVk *contextVk,
+                                                       uint32_t levelIndexGL,
+                                                       uint32_t layerIndex)
 {
     // Find any staged updates for this index and removes them from the pending list.
     for (size_t index = 0; index < mSubresourceUpdates.size();)
@@ -3353,6 +3353,30 @@ void ImageHelper::removeStagedUpdates(ContextVk *contextVk,
     }
 }
 
+void ImageHelper::removeStagedUpdates(ContextVk *contextVk,
+                                      uint32_t levelGLStart,
+                                      uint32_t levelGLEnd)
+{
+    // Remove all updates to levels [start, end].
+    for (size_t index = 0; index < mSubresourceUpdates.size();)
+    {
+        auto update               = mSubresourceUpdates.begin() + index;
+        uint32_t updateMipLevelGL = update->updateSource == UpdateSource::Clear
+                                        ? update->clear.levelIndex
+                                        : update->dstSubresource().mipLevel;
+
+        if (updateMipLevelGL >= levelGLStart && updateMipLevelGL <= levelGLEnd)
+        {
+            update->release(contextVk->getRenderer());
+            mSubresourceUpdates.erase(update);
+        }
+        else
+        {
+            index++;
+        }
+    }
+}
+
 angle::Result ImageHelper::stageSubresourceUpdateImpl(ContextVk *contextVk,
                                                       const gl::ImageIndex &index,
                                                       const gl::Extents &glExtents,
@@ -3976,7 +4000,7 @@ angle::Result ImageHelper::flushSingleSubresourceStagedUpdates(ContextVk *contex
             deferredClears->store(deferredClearIndex, update.aspectFlags, update.value);
 
             // We process the updates again to erase any clears for this level.
-            removeStagedUpdates(contextVk, levelGL, layer);
+            removeSingleSubresourceStagedUpdates(contextVk, levelGL, layer);
             return angle::Result::Continue;
         }
 

src/libANGLE/renderer/vulkan/vk_helpers.h

@@ -1170,7 +1170,10 @@ class ImageHelper final : public Resource, public angle::Subject
     void resolve(ImageHelper *dest, const VkImageResolve &region, CommandBuffer *commandBuffer);
 
     // Data staging
-    void removeStagedUpdates(ContextVk *contextVk, uint32_t levelIndexGL, uint32_t layerIndex);
+    void removeSingleSubresourceStagedUpdates(ContextVk *contextVk,
+                                              uint32_t levelIndexGL,
+                                              uint32_t layerIndex);
+    void removeStagedUpdates(ContextVk *contextVk, uint32_t levelGLStart, uint32_t levelGLEnd);
 
     angle::Result stageSubresourceUpdateImpl(ContextVk *contextVk,
                                              const gl::ImageIndex &index,

src/tests/gl_tests/MipmapTest.cpp

@@ -576,6 +576,35 @@ TEST_P(MipmapTest, GenerateMipmapFromInitDataThenRender)
     EXPECT_PIXEL_COLOR_EQ(getWindowWidth() / 8, getWindowHeight() / 8, GLColor::blue);
 }
 
+// Test that generating mipmap after the image is already created for a single level works.
+TEST_P(MipmapTest, GenerateMipmapAfterSingleLevelDraw)
+{
+    uint32_t width  = getWindowWidth();
+    uint32_t height = getWindowHeight();
+
+    const std::vector<GLColor> kInitData(width * height, GLColor::blue);
+
+    // Pass in initial data so the texture is blue.
+    glBindTexture(GL_TEXTURE_2D, mTexture2D);
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, width, height, 0, GL_RGBA, GL_UNSIGNED_BYTE,
+                 kInitData.data());
+
+    // Make sure the texture image is created.
+    clearAndDrawQuad(m2DProgram, getWindowWidth(), getWindowHeight());
+    EXPECT_PIXEL_COLOR_EQ(getWindowWidth() / 2, getWindowHeight() / 2, kInitData[0]);
+
+    // Then generate the mips.
+    glGenerateMipmap(GL_TEXTURE_2D);
+    ASSERT_GL_NO_ERROR();
+
+    // Enable mipmaps.
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR_MIPMAP_NEAREST);
+
+    // Draw and make sure the second mip is blue.
+    clearAndDrawQuad(m2DProgram, getWindowWidth() / 2, getWindowHeight() / 2);
+    EXPECT_PIXEL_COLOR_EQ(getWindowWidth() / 4, getWindowHeight() / 4, kInitData[0]);
+}
+
 // Test that generating mipmaps, then modifying the base level and generating mipmaps again works.
 TEST_P(MipmapTest, GenerateMipmapAfterModifyingBaseLevel)
 {