Fix cubemap robust resource init for GenerateMipmap

src/libANGLE/ImageIndex.cpp

@@ -163,6 +163,19 @@ ImageIndexIterator ImageIndexIterator::Make2DMultisample()
                               nullptr);
 }
 
+ImageIndexIterator ImageIndexIterator::MakeGeneric(TextureType type, GLint minMip, GLint maxMip)
+{
+    if (type == TextureType::CubeMap)
+    {
+        return MakeCube(minMip, maxMip);
+    }
+
+    TextureTarget target = NonCubeTextureTypeToTarget(type);
+    return ImageIndexIterator(type, target, target, Range<GLint>(minMip, maxMip),
+                              Range<GLint>(ImageIndex::ENTIRE_LEVEL, ImageIndex::ENTIRE_LEVEL),
+                              nullptr);
+}
+
 ImageIndexIterator::ImageIndexIterator(TextureType type,
                                        angle::EnumIterator<TextureTarget> targetLow,
                                        angle::EnumIterator<TextureTarget> targetHigh,

src/libANGLE/ImageIndex.h

@@ -64,6 +64,13 @@ bool operator<(const ImageIndex &a, const ImageIndex &b);
 bool operator==(const ImageIndex &a, const ImageIndex &b);
 bool operator!=(const ImageIndex &a, const ImageIndex &b);
 
+// To be used like this:
+//
+// ImageIndexIterator it = ...;
+// while (it.hasNext())
+// {
+//     ImageIndex current = it.next();
+// }
 class ImageIndexIterator
 {
   public:
@@ -75,6 +82,7 @@ class ImageIndexIterator
     static ImageIndexIterator Make3D(GLint minMip, GLint maxMip, GLint minLayer, GLint maxLayer);
     static ImageIndexIterator Make2DArray(GLint minMip, GLint maxMip, const GLsizei *layerCounts);
     static ImageIndexIterator Make2DMultisample();
+    static ImageIndexIterator MakeGeneric(TextureType type, GLint minMip, GLint maxMip);
 
     ImageIndex next();
     ImageIndex current() const;

src/libANGLE/Texture.cpp

@@ -38,19 +38,6 @@ size_t GetImageDescIndex(TextureTarget target, size_t level)
                : level;
 }
 
-ImageIndex GetImageIndexFromDescIndex(TextureType type, size_t descIndex)
-{
-    if (type == TextureType::CubeMap)
-    {
-        size_t faceIndex = descIndex % 6;
-        size_t mipIndex  = descIndex / 6;
-        return ImageIndex::MakeCube(CubeFaceIndexToTextureTarget(faceIndex),
-                                    static_cast<GLint>(mipIndex));
-    }
-
-    return ImageIndex::MakeGeneric(NonCubeTextureTypeToTarget(type), static_cast<GLint>(descIndex));
-}
-
 InitState DetermineInitState(const Context *context, const uint8_t *pixels)
 {
     // Can happen in tests.
@@ -1228,26 +1215,36 @@ Error Texture::generateMipmap(const Context *context)
     const GLuint baseLevel = mState.getEffectiveBaseLevel();
     const GLuint maxLevel  = mState.getMipmapMaxLevel();
 
-    if (maxLevel > baseLevel)
+    if (maxLevel <= baseLevel)
+    {
+        return NoError();
+    }
+    syncState();
+
+    // Clear the base image(s) immediately if needed
+    if (context->isRobustResourceInitEnabled())
     {
-        syncState();
-        const ImageDesc &baseImageInfo =
-            mState.getImageDesc(mState.getBaseImageTarget(), baseLevel);
-
-        // TODO(cwallez@chromium.org): This doesn't handle cubemaps correctly.
-        // Clear the base image immediately if necessary.
-        if (context->isRobustResourceInitEnabled() &&
-            baseImageInfo.initState == InitState::MayNeedInit)
+        ImageIndexIterator it =
+            ImageIndexIterator::MakeGeneric(mState.mType, baseLevel, baseLevel + 1);
+        while (it.hasNext())
         {
-            ANGLE_TRY(
-                initializeContents(context, GetImageIndexFromDescIndex(mState.mType, baseLevel)));
+            const ImageIndex index = it.next();
+            const ImageDesc &desc  = mState.getImageDesc(index.target, index.mipIndex);
+
+            if (desc.initState == InitState::MayNeedInit)
+            {
+                ANGLE_TRY(initializeContents(context, index));
+            }
         }
+    }
 
-        ANGLE_TRY(mTexture->generateMipmap(context));
+    ANGLE_TRY(mTexture->generateMipmap(context));
 
-        mState.setImageDescChain(baseLevel, maxLevel, baseImageInfo.size, baseImageInfo.format,
-                                 InitState::Initialized);
-    }
+    // Propagate the format and size of the bsae 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,
+                             InitState::Initialized);
 
     signalDirty(context, InitState::Initialized);
 
@@ -1468,16 +1465,18 @@ Error Texture::ensureInitialized(const Context *context)
 
     bool anyDirty = false;
 
-    for (size_t descIndex = 0; descIndex < mState.mImageDescs.size(); ++descIndex)
+    ImageIndexIterator it =
+        ImageIndexIterator::MakeGeneric(mState.mType, 0, IMPLEMENTATION_MAX_TEXTURE_LEVELS + 1);
+    while (it.hasNext())
     {
-        auto &imageDesc = mState.mImageDescs[descIndex];
-        if (imageDesc.initState == InitState::MayNeedInit)
+        const ImageIndex index = it.next();
+        ImageDesc &desc = mState.mImageDescs[GetImageDescIndex(index.target, index.mipIndex)];
+        if (desc.initState == InitState::MayNeedInit)
         {
             ASSERT(mState.mInitState == InitState::MayNeedInit);
-            const auto &imageIndex = GetImageIndexFromDescIndex(mState.mType, descIndex);
-            ANGLE_TRY(initializeContents(context, imageIndex));
-            imageDesc.initState = InitState::Initialized;
-            anyDirty            = true;
+            ANGLE_TRY(initializeContents(context, index));
+            desc.initState = InitState::Initialized;
+            anyDirty       = true;
         }
     }
     if (anyDirty)

src/tests/gl_tests/RobustResourceInitTest.cpp

@@ -981,6 +981,43 @@ TEST_P(RobustResourceInitTestES3, GenerateMipmap)
     EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::transparentBlack);
 }
 
+// Tests creating mipmaps for cube maps with robust resource init.
+TEST_P(RobustResourceInitTestES3, GenerateMipmapCubeMap)
+{
+    ANGLE_SKIP_TEST_IF(!hasGLExtension());
+
+    constexpr GLint kTextureSize   = 16;
+    constexpr GLint kTextureLevels = 5;
+
+    // Initialize a 16x16 RGBA8 texture with no data.
+    GLTexture tex;
+    glBindTexture(GL_TEXTURE_CUBE_MAP, tex);
+    for (GLenum target = GL_TEXTURE_CUBE_MAP_POSITIVE_X; target <= GL_TEXTURE_CUBE_MAP_NEGATIVE_Z;
+         ++target)
+    {
+        glTexImage2D(target, 0, GL_RGBA, kTextureSize, kTextureSize, 0, GL_RGBA, GL_UNSIGNED_BYTE,
+                     nullptr);
+    }
+    glTexParameteri(GL_TEXTURE_CUBE_MAP, GL_TEXTURE_MIN_FILTER, GL_NEAREST_MIPMAP_NEAREST);
+    glTexParameteri(GL_TEXTURE_CUBE_MAP, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+
+    // Generate mipmaps and verify all the mips.
+    glGenerateMipmap(GL_TEXTURE_CUBE_MAP);
+    ASSERT_GL_NO_ERROR();
+
+    GLFramebuffer fbo;
+    glBindFramebuffer(GL_FRAMEBUFFER, fbo);
+    for (GLenum target = GL_TEXTURE_CUBE_MAP_POSITIVE_X; target <= GL_TEXTURE_CUBE_MAP_NEGATIVE_Z;
+         ++target)
+    {
+        for (GLint level = 0; level < kTextureLevels; ++level)
+        {
+            glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, target, tex, level);
+            EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::transparentBlack);
+        }
+    }
+}
+
 // Test blitting a framebuffer out-of-bounds. Multiple iterations.
 TEST_P(RobustResourceInitTestES3, BlitFramebufferOutOfBounds)
 {