Vulkan: Fix cube map attachment clears and readpixels.

src/libANGLE/renderer/vulkan/FramebufferVk.cpp

@@ -211,8 +211,10 @@ gl::Error FramebufferVk::clear(const gl::Context *context, GLbitfield mask)
     }
 
     // If we clear the depth OR the stencil but not both, and we have a packed depth stencil
-    // attachment, we need to use clearAttachment instead of clearDepthStencil since Vulkan won't
+    // attachment, we need to use clearAttachments instead of clearDepthStencil since Vulkan won't
     // allow us to clear one or the other separately.
+    // Note: this might be bugged if we emulate single depth or stencil with a packed format.
+    // TODO(jmadill): Investigate emulated packed formats. http://anglebug.com/2815
     bool isSingleClearOnPackedDepthStencilAttachment =
         depthStencilAttachment && (clearDepth != clearStencil);
     if (glState.isScissorTestEnabled() || isSingleClearOnPackedDepthStencilAttachment)
@@ -275,7 +277,10 @@ gl::Error FramebufferVk::clear(const gl::Context *context, GLbitfield mask)
         ASSERT(colorRenderTarget);
         vk::ImageHelper *image = colorRenderTarget->getImageForWrite(this);
         GLint mipLevelToClear  = (attachment->type() == GL_TEXTURE) ? attachment->mipLevel() : 0;
-        image->clearColor(modifiedClearColorValue, mipLevelToClear, 1, commandBuffer);
+
+        // If we're clearing a cube map face ensure we only clear the selected layer.
+        image->clearColorLayer(modifiedClearColorValue, mipLevelToClear, 1,
+                               colorRenderTarget->getLayerIndex(), 1, commandBuffer);
     }
 
     return gl::NoError();
@@ -885,10 +890,7 @@ angle::Result FramebufferVk::clearWithClearAttachments(ContextVk *contextVk,
     vk::RecordingMode mode           = vk::RecordingMode::Start;
     ANGLE_TRY(getCommandBufferForDraw(contextVk, &commandBuffer, &mode));
 
-    // TODO(jmadill): Cube map attachments. http://anglebug.com/2470
-    // We assume for now that we always need to clear only 1 layer starting at the
-    // baseArrayLayer 0, this might need to change depending how we'll implement
-    // cube maps, 3d textures and array textures.
+    // The array layer is offset by the ImageView. So we shouldn't need to set a base array layer.
     VkClearRect clearRect;
     clearRect.baseArrayLayer = 0;
     clearRect.layerCount     = 1;
@@ -1187,7 +1189,7 @@ angle::Result FramebufferVk::readPixelsImpl(ContextVk *contextVk,
     region.imageOffset.y                   = area.y;
     region.imageOffset.z                   = 0;
     region.imageSubresource.aspectMask     = copyAspectFlags;
-    region.imageSubresource.baseArrayLayer = 0;
+    region.imageSubresource.baseArrayLayer = renderTarget->getLayerIndex();
     region.imageSubresource.layerCount     = 1;
     region.imageSubresource.mipLevel       = 0;
 

src/libANGLE/renderer/vulkan/RenderTargetVk.cpp

@@ -17,8 +17,9 @@ namespace rx
 {
 RenderTargetVk::RenderTargetVk(vk::ImageHelper *image,
                                vk::ImageView *imageView,
-                               vk::CommandGraphResource *resource)
-    : mImage(image), mImageView(imageView), mResource(resource)
+                               vk::CommandGraphResource *resource,
+                               size_t layerIndex)
+    : mImage(image), mImageView(imageView), mResource(resource), mLayerIndex(layerIndex)
 {
 }
 
@@ -27,7 +28,10 @@ RenderTargetVk::~RenderTargetVk()
 }
 
 RenderTargetVk::RenderTargetVk(RenderTargetVk &&other)
-    : mImage(other.mImage), mImageView(other.mImageView), mResource(other.mResource)
+    : mImage(other.mImage),
+      mImageView(other.mImageView),
+      mResource(other.mResource),
+      mLayerIndex(other.mLayerIndex)
 {
 }
 

src/libANGLE/renderer/vulkan/RenderTargetVk.h

@@ -35,7 +35,8 @@ class RenderTargetVk final : public FramebufferAttachmentRenderTarget
   public:
     RenderTargetVk(vk::ImageHelper *image,
                    vk::ImageView *imageView,
-                   vk::CommandGraphResource *resource);
+                   vk::CommandGraphResource *resource,
+                   size_t layerIndex);
     ~RenderTargetVk();
 
     // Used in std::vector initialization.
@@ -61,6 +62,7 @@ class RenderTargetVk final : public FramebufferAttachmentRenderTarget
 
     const vk::Format &getImageFormat() const;
     const gl::Extents &getImageExtents() const;
+    size_t getLayerIndex() const { return mLayerIndex; }
 
     // Special mutator for Surface RenderTargets. Allows the Framebuffer to keep a single
     // RenderTargetVk pointer.
@@ -70,6 +72,7 @@ class RenderTargetVk final : public FramebufferAttachmentRenderTarget
     vk::ImageHelper *mImage;
     vk::ImageView *mImageView;
     vk::CommandGraphResource *mResource;
+    size_t mLayerIndex;
 };
 
 }  // namespace rx

src/libANGLE/renderer/vulkan/RenderbufferVk.cpp

@@ -24,7 +24,7 @@ constexpr VkClearColorValue kBlackClearColorValue                 = {{0}};
 }  // anonymous namespace
 
 RenderbufferVk::RenderbufferVk(const gl::RenderbufferState &state)
-    : RenderbufferImpl(state), mRenderTarget(&mImage, &mImageView, this)
+    : RenderbufferImpl(state), mRenderTarget(&mImage, &mImageView, this, 0)
 {
 }
 

src/libANGLE/renderer/vulkan/SurfaceVk.cpp

@@ -59,7 +59,7 @@ constexpr VkImageUsageFlags kSurfaceVKDepthStencilImageUsageFlags =
 }  // namespace
 
 OffscreenSurfaceVk::AttachmentImage::AttachmentImage(vk::CommandGraphResource *commandGraphResource)
-    : renderTarget(&image, &imageView, commandGraphResource)
+    : renderTarget(&image, &imageView, commandGraphResource, 0)
 {
 }
 
@@ -268,8 +268,8 @@ WindowSurfaceVk::WindowSurfaceVk(const egl::SurfaceState &surfaceState,
       mSurface(VK_NULL_HANDLE),
       mInstance(VK_NULL_HANDLE),
       mSwapchain(VK_NULL_HANDLE),
-      mColorRenderTarget(nullptr, nullptr, this),
-      mDepthStencilRenderTarget(&mDepthStencilImage, &mDepthStencilImageView, this),
+      mColorRenderTarget(nullptr, nullptr, this, 0),
+      mDepthStencilRenderTarget(&mDepthStencilImage, &mDepthStencilImageView, this, 0),
       mCurrentSwapchainImageIndex(0)
 {
 }

src/libANGLE/renderer/vulkan/TextureVk.cpp

@@ -429,7 +429,9 @@ PixelBuffer::SubresourceUpdate::SubresourceUpdate(const SubresourceUpdate &other
 
 // TextureVk implementation.
 TextureVk::TextureVk(const gl::TextureState &state, RendererVk *renderer)
-    : TextureImpl(state), mRenderTarget(&mImage, &mBaseLevelImageView, this), mPixelBuffer(renderer)
+    : TextureImpl(state),
+      mRenderTarget(&mImage, &mBaseLevelImageView, this, 0),
+      mPixelBuffer(renderer)
 {
 }
 
@@ -1086,7 +1088,7 @@ angle::Result TextureVk::initCubeMapRenderTargets(ContextVk *contextVk)
         ANGLE_TRY(mImage.initLayerImageView(contextVk, gl::TextureType::CubeMap,
                                             VK_IMAGE_ASPECT_COLOR_BIT, gl::SwizzleState(),
                                             &imageView, 1, cubeMapFaceIndex, 1));
-        mCubeMapRenderTargets.emplace_back(&mImage, &imageView, this);
+        mCubeMapRenderTargets.emplace_back(&mImage, &imageView, this, cubeMapFaceIndex);
     }
     return angle::Result::Continue();
 }

src/libANGLE/renderer/vulkan/vk_helpers.cpp

@@ -790,9 +790,20 @@ void ImageHelper::changeLayoutWithStages(VkImageAspectFlags aspectMask,
 }
 
 void ImageHelper::clearColor(const VkClearColorValue &color,
-                             uint32_t mipLevel,
+                             uint32_t baseMipLevel,
                              uint32_t levelCount,
                              CommandBuffer *commandBuffer)
+
+{
+    clearColorLayer(color, baseMipLevel, levelCount, 0, mLayerCount, commandBuffer);
+}
+
+void ImageHelper::clearColorLayer(const VkClearColorValue &color,
+                                  uint32_t baseMipLevel,
+                                  uint32_t levelCount,
+                                  uint32_t baseArrayLayer,
+                                  uint32_t layerCount,
+                                  CommandBuffer *commandBuffer)
 {
     ASSERT(valid());
 
@@ -802,10 +813,10 @@ void ImageHelper::clearColor(const VkClearColorValue &color,
 
     VkImageSubresourceRange range;
     range.aspectMask     = VK_IMAGE_ASPECT_COLOR_BIT;
-    range.baseMipLevel   = mipLevel;
+    range.baseMipLevel   = baseMipLevel;
     range.levelCount     = levelCount;
-    range.baseArrayLayer = 0;
-    range.layerCount     = mLayerCount;
+    range.baseArrayLayer = baseArrayLayer;
+    range.layerCount     = layerCount;
 
     commandBuffer->clearColorImage(mImage, mCurrentLayout, color, 1, &range);
 }

src/libANGLE/renderer/vulkan/vk_helpers.h

@@ -228,10 +228,17 @@ class ImageHelper final : angle::NonCopyable
                                 CommandBuffer *commandBuffer);
 
     void clearColor(const VkClearColorValue &color,
-                    uint32_t mipLevel,
+                    uint32_t baseMipLevel,
                     uint32_t levelCount,
                     CommandBuffer *commandBuffer);
 
+    void clearColorLayer(const VkClearColorValue &color,
+                         uint32_t baseMipLevel,
+                         uint32_t levelCount,
+                         uint32_t baseArrayLayer,
+                         uint32_t layerCount,
+                         CommandBuffer *commandBuffer);
+
     void clearDepthStencil(VkImageAspectFlags aspectFlags,
                            const VkClearDepthStencilValue &depthStencil,
                            CommandBuffer *commandBuffer);

src/tests/gl_tests/MipmapTest.cpp

@@ -768,7 +768,7 @@ TEST_P(MipmapTest, MipMapGenerationD3D9Bug)
 TEST_P(MipmapTest, TextureCubeGeneralLevelZero)
 {
     // This test seems to fail only on Android Vulkan.
-    // TODO(jmadill): Diagnose and fix. http://anglebug.com/2470
+    // TODO(jmadill): Diagnose and fix. http://anglebug.com/2817
     ANGLE_SKIP_TEST_IF(IsVulkan() && IsAndroid());
 
     glBindTexture(GL_TEXTURE_CUBE_MAP, mTextureCube);

src/tests/gl_tests/TextureTest.cpp

@@ -1374,8 +1374,7 @@ TEST_P(Texture2DTestWithDrawScale, MipmapsTwice)
 // https://code.google.com/p/angleproject/issues/detail?id=849
 TEST_P(TextureCubeTest, CubeMapFBO)
 {
-    GLuint fbo;
-    glGenFramebuffers(1, &fbo);
+    GLFramebuffer fbo;
     glBindFramebuffer(GL_FRAMEBUFFER, fbo);
 
     glBindTexture(GL_TEXTURE_CUBE_MAP, mTextureCube);
@@ -1383,10 +1382,74 @@ TEST_P(TextureCubeTest, CubeMapFBO)
                            mTextureCube, 0);
 
     EXPECT_GLENUM_EQ(GL_FRAMEBUFFER_COMPLETE, glCheckFramebufferStatus(GL_FRAMEBUFFER));
+    EXPECT_GL_NO_ERROR();
 
-    glDeleteFramebuffers(1, &fbo);
+    // Test clearing the six mip faces individually.
+    std::array<GLColor, 6> faceColors = {{GLColor::red, GLColor::green, GLColor::blue,
+                                          GLColor::yellow, GLColor::cyan, GLColor::magenta}};
 
-    EXPECT_GL_NO_ERROR();
+    for (size_t faceIndex = 0; faceIndex < 6; ++faceIndex)
+    {
+        glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
+                               GL_TEXTURE_CUBE_MAP_POSITIVE_X + faceIndex, mTextureCube, 0);
+
+        Vector4 clearColorF = faceColors[faceIndex].toNormalizedVector();
+        glClearColor(clearColorF.x(), clearColorF.y(), clearColorF.z(), clearColorF.w());
+        glClear(GL_COLOR_BUFFER_BIT);
+
+        EXPECT_PIXEL_COLOR_EQ(0, 0, faceColors[faceIndex]);
+    }
+
+    // Iterate the faces again to make sure the colors haven't changed.
+    for (size_t faceIndex = 0; faceIndex < 6; ++faceIndex)
+    {
+        glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
+                               GL_TEXTURE_CUBE_MAP_POSITIVE_X + faceIndex, mTextureCube, 0);
+        EXPECT_PIXEL_COLOR_EQ(0, 0, faceColors[faceIndex])
+            << "face color " << faceIndex << " shouldn't change";
+    }
+}
+
+// Tests clearing a cube map with a scissor enabled.
+TEST_P(TextureCubeTest, CubeMapFBOScissoredClear)
+{
+    // TODO(jie.a.chen): Diagnose and fix. http://anglebug.com/2822
+    ANGLE_SKIP_TEST_IF(IsVulkan() && IsIntel() && IsWindows());
+
+    constexpr size_t kSize = 16;
+
+    GLFramebuffer fbo;
+    glBindFramebuffer(GL_FRAMEBUFFER, fbo);
+    glViewport(0, 0, kSize, kSize);
+
+    GLTexture texcube;
+    glBindTexture(GL_TEXTURE_CUBE_MAP, texcube);
+    for (GLenum face = 0; face < 6; face++)
+    {
+        glTexImage2D(GL_TEXTURE_CUBE_MAP_POSITIVE_X + face, 0, GL_RGBA, kSize, kSize, 0, GL_RGBA,
+                     GL_UNSIGNED_BYTE, nullptr);
+    }
+    ASSERT_GL_NO_ERROR();
+
+    glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_CUBE_MAP_POSITIVE_Y,
+                           texcube, 0);
+
+    EXPECT_GLENUM_EQ(GL_FRAMEBUFFER_COMPLETE, glCheckFramebufferStatus(GL_FRAMEBUFFER));
+    ASSERT_GL_NO_ERROR();
+
+    glClearColor(1.0f, 0.0f, 0.0f, 1.0f);
+    glClear(GL_COLOR_BUFFER_BIT);
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::red);
+
+    glEnable(GL_SCISSOR_TEST);
+    glScissor(kSize / 2, 0, kSize / 2, kSize);
+    glClearColor(0.0f, 1.0f, 0.0f, 1.0f);
+    glClear(GL_COLOR_BUFFER_BIT);
+
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::red);
+    EXPECT_PIXEL_COLOR_EQ(kSize / 2 + 1, 0, GLColor::green);
+
+    ASSERT_GL_NO_ERROR();
 }
 
 // Test that glTexSubImage2D works properly when glTexStorage2DEXT has initialized the image with a