Vulkan: Fix cube map attachment clears and readpixels.
These were both missing the correct layer offset. Cache the layer
inside the RenderTargetVk for easy access.
Bug: angleproject:2470
Change-Id: I690dbf0702d7ec52f44ba0a9429b6ef0e51baf6b
Reviewed-on: https://chromium-review.googlesource.com/1225910
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Yuly Novikov <ynovikov@chromium.org>
diff --git a/src/libANGLE/renderer/vulkan/FramebufferVk.cpp b/src/libANGLE/renderer/vulkan/FramebufferVk.cpp
index e1ad661..986837c 100644
--- a/src/libANGLE/renderer/vulkan/FramebufferVk.cpp
+++ b/src/libANGLE/renderer/vulkan/FramebufferVk.cpp
@@ -211,8 +211,10 @@
}
// 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 @@
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 @@
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 @@
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;
diff --git a/src/libANGLE/renderer/vulkan/RenderTargetVk.cpp b/src/libANGLE/renderer/vulkan/RenderTargetVk.cpp
index 9a4a0f3..18dd86d 100644
--- a/src/libANGLE/renderer/vulkan/RenderTargetVk.cpp
+++ b/src/libANGLE/renderer/vulkan/RenderTargetVk.cpp
@@ -17,8 +17,9 @@
{
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 &&other)
- : mImage(other.mImage), mImageView(other.mImageView), mResource(other.mResource)
+ : mImage(other.mImage),
+ mImageView(other.mImageView),
+ mResource(other.mResource),
+ mLayerIndex(other.mLayerIndex)
{
}
diff --git a/src/libANGLE/renderer/vulkan/RenderTargetVk.h b/src/libANGLE/renderer/vulkan/RenderTargetVk.h
index f4d1ef3..4eacad5 100644
--- a/src/libANGLE/renderer/vulkan/RenderTargetVk.h
+++ b/src/libANGLE/renderer/vulkan/RenderTargetVk.h
@@ -35,7 +35,8 @@
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 @@
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 @@
vk::ImageHelper *mImage;
vk::ImageView *mImageView;
vk::CommandGraphResource *mResource;
+ size_t mLayerIndex;
};
} // namespace rx
diff --git a/src/libANGLE/renderer/vulkan/RenderbufferVk.cpp b/src/libANGLE/renderer/vulkan/RenderbufferVk.cpp
index eceee09..5d1dfba 100644
--- a/src/libANGLE/renderer/vulkan/RenderbufferVk.cpp
+++ b/src/libANGLE/renderer/vulkan/RenderbufferVk.cpp
@@ -24,7 +24,7 @@
} // anonymous namespace
RenderbufferVk::RenderbufferVk(const gl::RenderbufferState &state)
- : RenderbufferImpl(state), mRenderTarget(&mImage, &mImageView, this)
+ : RenderbufferImpl(state), mRenderTarget(&mImage, &mImageView, this, 0)
{
}
diff --git a/src/libANGLE/renderer/vulkan/SurfaceVk.cpp b/src/libANGLE/renderer/vulkan/SurfaceVk.cpp
index 7a072de..e1a58d0 100644
--- a/src/libANGLE/renderer/vulkan/SurfaceVk.cpp
+++ b/src/libANGLE/renderer/vulkan/SurfaceVk.cpp
@@ -59,7 +59,7 @@
} // namespace
OffscreenSurfaceVk::AttachmentImage::AttachmentImage(vk::CommandGraphResource *commandGraphResource)
- : renderTarget(&image, &imageView, commandGraphResource)
+ : renderTarget(&image, &imageView, commandGraphResource, 0)
{
}
@@ -268,8 +268,8 @@
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)
{
}
diff --git a/src/libANGLE/renderer/vulkan/TextureVk.cpp b/src/libANGLE/renderer/vulkan/TextureVk.cpp
index 58ee7e1..5a874f8 100644
--- a/src/libANGLE/renderer/vulkan/TextureVk.cpp
+++ b/src/libANGLE/renderer/vulkan/TextureVk.cpp
@@ -429,7 +429,9 @@
// 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_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();
}
diff --git a/src/libANGLE/renderer/vulkan/vk_helpers.cpp b/src/libANGLE/renderer/vulkan/vk_helpers.cpp
index 4f0fa99..e375506 100644
--- a/src/libANGLE/renderer/vulkan/vk_helpers.cpp
+++ b/src/libANGLE/renderer/vulkan/vk_helpers.cpp
@@ -790,9 +790,20 @@
}
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 @@
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);
}
diff --git a/src/libANGLE/renderer/vulkan/vk_helpers.h b/src/libANGLE/renderer/vulkan/vk_helpers.h
index ef45095..ed787f2 100644
--- a/src/libANGLE/renderer/vulkan/vk_helpers.h
+++ b/src/libANGLE/renderer/vulkan/vk_helpers.h
@@ -228,10 +228,17 @@
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);
diff --git a/src/tests/gl_tests/MipmapTest.cpp b/src/tests/gl_tests/MipmapTest.cpp
index d6f7c60..c30114a 100644
--- a/src/tests/gl_tests/MipmapTest.cpp
+++ b/src/tests/gl_tests/MipmapTest.cpp
@@ -768,7 +768,7 @@
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);
diff --git a/src/tests/gl_tests/TextureTest.cpp b/src/tests/gl_tests/TextureTest.cpp
index f2a117c..4f1cf76 100644
--- a/src/tests/gl_tests/TextureTest.cpp
+++ b/src/tests/gl_tests/TextureTest.cpp
@@ -1374,8 +1374,7 @@
// 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 @@
mTextureCube, 0);
EXPECT_GLENUM_EQ(GL_FRAMEBUFFER_COMPLETE, glCheckFramebufferStatus(GL_FRAMEBUFFER));
-
- glDeleteFramebuffers(1, &fbo);
-
EXPECT_GL_NO_ERROR();
+
+ // Test clearing the six mip faces individually.
+ std::array<GLColor, 6> faceColors = {{GLColor::red, GLColor::green, GLColor::blue,
+ GLColor::yellow, GLColor::cyan, GLColor::magenta}};
+
+ 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