TextureD3D_2D::CopyImage clear using initializeContents
When using glCopyTexImage2D clearing of the mip level needs to happen
when running in WebGL or robust resource init mode and any pixel would
be sampled outside of the framebuffer. Previously the code was using
"setImage" for this purpose, causing issues when a PIXEL_UNPACK_BUFFER
was bound.
Also add a regression test.
BUG=chromium:827667
Change-Id: I03be20d8272730ab30afdab2f8919be853e729b6
Reviewed-on: https://chromium-review.googlesource.com/1000182
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>
diff --git a/src/libANGLE/renderer/d3d/TextureD3D.cpp b/src/libANGLE/renderer/d3d/TextureD3D.cpp
index 2963fa4..ccb9515 100644
--- a/src/libANGLE/renderer/d3d/TextureD3D.cpp
+++ b/src/libANGLE/renderer/d3d/TextureD3D.cpp
@@ -692,7 +692,10 @@
}
// Fast path: can use a render target clear.
- if (canCreateRenderTargetForImage(imageIndex))
+ // We don't use the fast path with the zero max lod workaround because it would introduce a race
+ // between the rendertarget and the staging images.
+ if (canCreateRenderTargetForImage(imageIndex) &&
+ !mRenderer->getWorkarounds().zeroMaxLodWorkaround)
{
ANGLE_TRY(ensureRenderTarget(context));
ASSERT(mTexStorage);
@@ -945,21 +948,13 @@
origSourceArea.x + origSourceArea.width > fbSize.width ||
origSourceArea.y + origSourceArea.height > fbSize.height;
- // In WebGL mode we need to zero the texture outside the framebuffer.
- // If we have robust resource init, it was already zeroed by redefineImage() above, otherwise
- // zero it explicitly.
- // TODO(fjhenigman): When robust resource is fully implemented look into making it a
- // prerequisite for WebGL and deleting this code.
+ // WebGL requires that pixels that would be outside the framebuffer are treated as zero values,
+ // so clear the mip level to 0 prior to making the copy if any pixel would be sampled outside.
+ // Same thing for robust resource init.
if (outside &&
(context->getExtensions().webglCompatibility || context->isRobustResourceInitEnabled()))
{
- angle::MemoryBuffer *zero;
- ANGLE_TRY(context->getZeroFilledBuffer(
- origSourceArea.width * origSourceArea.height * internalFormatInfo.pixelBytes, &zero));
- gl::PixelUnpackState unpack;
- unpack.alignment = 1;
- ANGLE_TRY(setImage(context, index, internalFormat, sourceExtents, internalFormatInfo.format,
- internalFormatInfo.type, unpack, zero->data()));
+ ANGLE_TRY(initializeContents(context, index));
}
gl::Rectangle sourceArea;
@@ -1704,21 +1699,13 @@
origSourceArea.x + origSourceArea.width > fbSize.width ||
origSourceArea.y + origSourceArea.height > fbSize.height;
- // In WebGL mode we need to zero the texture outside the framebuffer.
- // If we have robust resource init, it was already zeroed by redefineImage() above, otherwise
- // zero it explicitly.
- // TODO(fjhenigman): When robust resource is fully implemented look into making it a
- // prerequisite for WebGL and deleting this code.
- if (outside && context->getExtensions().webglCompatibility &&
- !context->isRobustResourceInitEnabled())
+ // WebGL requires that pixels that would be outside the framebuffer are treated as zero values,
+ // so clear the mip level to 0 prior to making the copy if any pixel would be sampled outside.
+ // Same thing for robust resource init.
+ if (outside &&
+ (context->getExtensions().webglCompatibility || context->isRobustResourceInitEnabled()))
{
- angle::MemoryBuffer *zero;
- ANGLE_TRY(context->getZeroFilledBuffer(
- origSourceArea.width * origSourceArea.height * internalFormatInfo.pixelBytes, &zero));
- gl::PixelUnpackState unpack;
- unpack.alignment = 1;
- ANGLE_TRY(setImage(context, index, internalFormat, size, internalFormatInfo.format,
- internalFormatInfo.type, unpack, zero->data()));
+ ANGLE_TRY(initializeContents(context, index));
}
gl::Rectangle sourceArea;
diff --git a/src/tests/gl_tests/RobustResourceInitTest.cpp b/src/tests/gl_tests/RobustResourceInitTest.cpp
index fca4285..ece1f5a 100644
--- a/src/tests/gl_tests/RobustResourceInitTest.cpp
+++ b/src/tests/gl_tests/RobustResourceInitTest.cpp
@@ -766,6 +766,62 @@
}
// Reading an uninitialized portion of a texture (copyTexImage2D with negative x and y) should
+// succeed with all bytes set to 0. Regression test for a bug where the zeroing out of the
+// texture was done via the same code path as glTexImage2D, causing the PIXEL_UNPACK_BUFFER
+// to be used.
+TEST_P(RobustResourceInitTestES3, ReadingOutOfboundsCopiedTextureWithUnpackBuffer)
+{
+ ANGLE_SKIP_TEST_IF(!hasGLExtension());
+ // TODO(geofflang@chromium.org): CopyTexImage from GL_RGBA4444 to GL_ALPHA fails when looking
+ // up which resulting format the texture should have.
+ ANGLE_SKIP_TEST_IF(IsOpenGL());
+
+ // GL_ALPHA texture can't be read with glReadPixels, for convenience this test uses
+ // glCopyTextureCHROMIUM to copy GL_ALPHA into GL_RGBA
+ ANGLE_SKIP_TEST_IF(!extensionEnabled("GL_CHROMIUM_copy_texture"));
+ PFNGLCOPYTEXTURECHROMIUMPROC glCopyTextureCHROMIUM =
+ reinterpret_cast<PFNGLCOPYTEXTURECHROMIUMPROC>(eglGetProcAddress("glCopyTextureCHROMIUM"));
+
+ GLFramebuffer fbo;
+ glBindFramebuffer(GL_FRAMEBUFFER, fbo);
+ GLRenderbuffer rbo;
+ glBindRenderbuffer(GL_RENDERBUFFER, rbo);
+ constexpr int fboWidth = 16;
+ constexpr int fboHeight = 16;
+ glRenderbufferStorage(GL_RENDERBUFFER, GL_RGBA4, fboWidth, fboHeight);
+ glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_RENDERBUFFER, rbo);
+ EXPECT_GLENUM_EQ(GL_FRAMEBUFFER_COMPLETE, glCheckFramebufferStatus(GL_FRAMEBUFFER));
+ glClearColor(1.0, 0.0, 0.0, 1.0);
+ glClear(GL_COLOR_BUFFER_BIT);
+ EXPECT_GL_NO_ERROR();
+ constexpr int x = -8;
+ constexpr int y = -8;
+
+ GLBuffer buffer;
+ glBindBuffer(GL_PIXEL_UNPACK_BUFFER, buffer);
+ std::vector<GLColor> bunchOfGreen(fboWidth * fboHeight, GLColor::green);
+ glBufferData(GL_PIXEL_UNPACK_BUFFER, sizeof(bunchOfGreen), bunchOfGreen.data(), GL_STATIC_DRAW);
+ EXPECT_GL_NO_ERROR();
+
+ // Use GL_ALPHA to force a CPU readback in the D3D11 backend
+ GLTexture texAlpha;
+ glBindTexture(GL_TEXTURE_2D, texAlpha);
+ glCopyTexImage2D(GL_TEXTURE_2D, 0, GL_ALPHA, x, y, kWidth, kHeight, 0);
+ EXPECT_GL_NO_ERROR();
+
+ // GL_ALPHA cannot be glReadPixels, so copy into a GL_RGBA texture
+ GLTexture texRGBA;
+ glBindBuffer(GL_PIXEL_UNPACK_BUFFER, 0);
+ setupTexture(&texRGBA);
+ glCopyTextureCHROMIUM(texAlpha, 0, GL_TEXTURE_2D, texRGBA, 0, GL_RGBA, GL_UNSIGNED_BYTE,
+ GL_FALSE, GL_FALSE, GL_FALSE);
+ EXPECT_GL_NO_ERROR();
+
+ checkNonZeroPixels(&texRGBA, -x, -y, fboWidth, fboHeight, GLColor(0, 0, 0, 255));
+ EXPECT_GL_NO_ERROR();
+}
+
+// Reading an uninitialized portion of a texture (copyTexImage2D with negative x and y) should
// succeed with all bytes set to 0.
TEST_P(RobustResourceInitTest, ReadingOutOfboundsCopiedTexture)
{
