Commit 56c8577b by Corentin Wallez Committed by Commit Bot

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: 's avatarJamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarGeoff Lang <geofflang@chromium.org>
parent bfb5df15
...@@ -692,7 +692,10 @@ gl::Error TextureD3D::initializeContents(const gl::Context *context, ...@@ -692,7 +692,10 @@ gl::Error TextureD3D::initializeContents(const gl::Context *context,
} }
// Fast path: can use a render target clear. // 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)); ANGLE_TRY(ensureRenderTarget(context));
ASSERT(mTexStorage); ASSERT(mTexStorage);
...@@ -945,21 +948,13 @@ gl::Error TextureD3D_2D::copyImage(const gl::Context *context, ...@@ -945,21 +948,13 @@ gl::Error TextureD3D_2D::copyImage(const gl::Context *context,
origSourceArea.x + origSourceArea.width > fbSize.width || origSourceArea.x + origSourceArea.width > fbSize.width ||
origSourceArea.y + origSourceArea.height > fbSize.height; origSourceArea.y + origSourceArea.height > fbSize.height;
// In WebGL mode we need to zero the texture outside the framebuffer. // WebGL requires that pixels that would be outside the framebuffer are treated as zero values,
// If we have robust resource init, it was already zeroed by redefineImage() above, otherwise // so clear the mip level to 0 prior to making the copy if any pixel would be sampled outside.
// zero it explicitly. // Same thing for robust resource init.
// TODO(fjhenigman): When robust resource is fully implemented look into making it a
// prerequisite for WebGL and deleting this code.
if (outside && if (outside &&
(context->getExtensions().webglCompatibility || context->isRobustResourceInitEnabled())) (context->getExtensions().webglCompatibility || context->isRobustResourceInitEnabled()))
{ {
angle::MemoryBuffer *zero; ANGLE_TRY(initializeContents(context, index));
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()));
} }
gl::Rectangle sourceArea; gl::Rectangle sourceArea;
...@@ -1704,21 +1699,13 @@ gl::Error TextureD3D_Cube::copyImage(const gl::Context *context, ...@@ -1704,21 +1699,13 @@ gl::Error TextureD3D_Cube::copyImage(const gl::Context *context,
origSourceArea.x + origSourceArea.width > fbSize.width || origSourceArea.x + origSourceArea.width > fbSize.width ||
origSourceArea.y + origSourceArea.height > fbSize.height; origSourceArea.y + origSourceArea.height > fbSize.height;
// In WebGL mode we need to zero the texture outside the framebuffer. // WebGL requires that pixels that would be outside the framebuffer are treated as zero values,
// If we have robust resource init, it was already zeroed by redefineImage() above, otherwise // so clear the mip level to 0 prior to making the copy if any pixel would be sampled outside.
// zero it explicitly. // Same thing for robust resource init.
// TODO(fjhenigman): When robust resource is fully implemented look into making it a if (outside &&
// prerequisite for WebGL and deleting this code. (context->getExtensions().webglCompatibility || context->isRobustResourceInitEnabled()))
if (outside && context->getExtensions().webglCompatibility && {
!context->isRobustResourceInitEnabled()) ANGLE_TRY(initializeContents(context, index));
{
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()));
} }
gl::Rectangle sourceArea; gl::Rectangle sourceArea;
......
...@@ -766,6 +766,62 @@ TEST_P(RobustResourceInitTest, UninitializedPartsOfCopied2DTexturesAreBlack) ...@@ -766,6 +766,62 @@ TEST_P(RobustResourceInitTest, UninitializedPartsOfCopied2DTexturesAreBlack)
} }
// Reading an uninitialized portion of a texture (copyTexImage2D with negative x and y) should // 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. // succeed with all bytes set to 0.
TEST_P(RobustResourceInitTest, ReadingOutOfboundsCopiedTexture) TEST_P(RobustResourceInitTest, ReadingOutOfboundsCopiedTexture)
{ {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment