Commit c2fc8bd8 by Geoff Lang Committed by Commit Bot

GL: Fix support for glCopyTextureCHROMIUM with source rectangle textures.

Shader generation didn't use the correct sampler type and discarded pixels outside the [0, 1] texcoord which is almost always true when sampling rectangle textures. Added end2end test coverage. BUG=990368 Change-Id: Ifaa73dfa83467d5bfa32e3795b52033254436a77 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/1834582 Commit-Queue: Geoff Lang <geofflang@chromium.org> Reviewed-by: 's avatarCorentin Wallez <cwallez@chromium.org>
parent 5a42da56
...@@ -1089,8 +1089,7 @@ angle::Result BlitGL::getBlitProgram(const gl::Context *context, ...@@ -1089,8 +1089,7 @@ angle::Result BlitGL::getBlitProgram(const gl::Context *context,
std::string fsOutputVariableQualifier; std::string fsOutputVariableQualifier;
std::string sampleFunction; std::string sampleFunction;
if (sourceComponentType != GL_UNSIGNED_INT && destComponentType != GL_UNSIGNED_INT && if (sourceComponentType != GL_UNSIGNED_INT && destComponentType != GL_UNSIGNED_INT &&
(sourceTextureType == gl::TextureType::_2D || sourceTextureType != gl::TextureType::Rectangle)
sourceTextureType == gl::TextureType::External))
{ {
// Simple case, float-to-float with 2D or external textures. Only needs ESSL/GLSL 100 // Simple case, float-to-float with 2D or external textures. Only needs ESSL/GLSL 100
version = "100"; version = "100";
...@@ -1169,6 +1168,11 @@ angle::Result BlitGL::getBlitProgram(const gl::Context *context, ...@@ -1169,6 +1168,11 @@ angle::Result BlitGL::getBlitProgram(const gl::Context *context,
samplerType = "samplerExternalOES"; samplerType = "samplerExternalOES";
break; break;
case gl::TextureType::Rectangle:
ASSERT(sourceComponentType != GL_UNSIGNED_INT);
samplerType = "sampler2DRect";
break;
default: default:
UNREACHABLE(); UNREACHABLE();
break; break;
...@@ -1193,6 +1197,17 @@ angle::Result BlitGL::getBlitProgram(const gl::Context *context, ...@@ -1193,6 +1197,17 @@ angle::Result BlitGL::getBlitProgram(const gl::Context *context,
extensionRequirements = "#extension GL_OES_EGL_image_external : require"; extensionRequirements = "#extension GL_OES_EGL_image_external : require";
break; break;
case gl::TextureType::Rectangle:
if (mFunctions->hasGLExtension("GL_ARB_texture_rectangle"))
{
extensionRequirements = "#extension GL_ARB_texture_rectangle : require";
}
else
{
ASSERT(mFunctions->isAtLeastGL(gl::Version(3, 1)));
}
break;
default: default:
break; break;
} }
...@@ -1210,10 +1225,18 @@ angle::Result BlitGL::getBlitProgram(const gl::Context *context, ...@@ -1210,10 +1225,18 @@ angle::Result BlitGL::getBlitProgram(const gl::Context *context,
break; break;
default: // float type default: // float type
ASSERT(version == "100"); if (version == "100")
outputType = ""; {
outputVariableName = "gl_FragColor"; outputType = "";
outputMultiplier = "1.0"; outputVariableName = "gl_FragColor";
outputMultiplier = "1.0";
}
else
{
outputType = "vec4";
outputVariableName = "outputFloat";
outputMultiplier = "1.0";
}
break; break;
} }
...@@ -1239,9 +1262,24 @@ angle::Result BlitGL::getBlitProgram(const gl::Context *context, ...@@ -1239,9 +1262,24 @@ angle::Result BlitGL::getBlitProgram(const gl::Context *context,
fsSourceStream << "void main()\n"; fsSourceStream << "void main()\n";
fsSourceStream << "{\n"; fsSourceStream << "{\n";
// discard if the texcoord is outside (0, 1)^2 so the blitframebuffer workaround std::string maxTexcoord;
// doesn't write when the point sampled is outside of the source framebuffer. switch (sourceTextureType)
fsSourceStream << " if (clamp(v_texcoord, vec2(0.0), vec2(1.0)) != v_texcoord)\n"; {
case gl::TextureType::Rectangle:
// Valid texcoords are within source texture size
maxTexcoord = "vec2(textureSize(u_source_texture))";
break;
default:
// Valid texcoords are in [0, 1]
maxTexcoord = "vec2(1.0)";
break;
}
// discard if the texcoord is invalid so the blitframebuffer workaround doesn't
// write when the point sampled is outside of the source framebuffer.
fsSourceStream << " if (clamp(v_texcoord, vec2(0.0), " << maxTexcoord
<< ") != v_texcoord)\n";
fsSourceStream << " {\n"; fsSourceStream << " {\n";
fsSourceStream << " discard;\n"; fsSourceStream << " discard;\n";
fsSourceStream << " }\n"; fsSourceStream << " }\n";
......
...@@ -893,10 +893,7 @@ angle::Result TextureGL::copySubTextureHelper(const gl::Context *context, ...@@ -893,10 +893,7 @@ angle::Result TextureGL::copySubTextureHelper(const gl::Context *context,
// Behavior for now is to fallback to CPU readback implementation if the destination texture // Behavior for now is to fallback to CPU readback implementation if the destination texture
// is a luminance format. The correct solution is to handle both source and destination in the // is a luminance format. The correct solution is to handle both source and destination in the
// luma workaround. // luma workaround.
// Temporarily disable GPU copies from rectangle textures until sampling issues are resolved in if (!destSRGB && !destLevelInfo.lumaWorkaround.enabled &&
// http://crbug.com/990368
if (!destSRGB && source->getType() != gl::TextureType::Rectangle &&
!destLevelInfo.lumaWorkaround.enabled &&
nativegl::SupportsNativeRendering(functions, getType(), destLevelInfo.nativeInternalFormat)) nativegl::SupportsNativeRendering(functions, getType(), destLevelInfo.nativeInternalFormat))
{ {
bool copySucceeded = false; bool copySucceeded = false;
......
...@@ -380,7 +380,8 @@ class CopyTextureVariationsTest : public ANGLETestWithParam<CopyTextureVariation ...@@ -380,7 +380,8 @@ class CopyTextureVariationsTest : public ANGLETestWithParam<CopyTextureVariation
} }
} }
void initializeSourceTexture(GLenum sourceFormat, void initializeSourceTexture(GLenum target,
GLenum sourceFormat,
const uint8_t *srcColors, const uint8_t *srcColors,
uint8_t componentCount) uint8_t componentCount)
{ {
...@@ -398,12 +399,13 @@ class CopyTextureVariationsTest : public ANGLETestWithParam<CopyTextureVariation ...@@ -398,12 +399,13 @@ class CopyTextureVariationsTest : public ANGLETestWithParam<CopyTextureVariation
srcRowPitch - inputRowPitch); srcRowPitch - inputRowPitch);
} }
glBindTexture(GL_TEXTURE_2D, mTextures[0]); glBindTexture(target, mTextures[0]);
glTexImage2D(GL_TEXTURE_2D, 0, sourceFormat, 2, 2, 0, sourceFormat, GL_UNSIGNED_BYTE, glTexImage2D(target, 0, sourceFormat, 2, 2, 0, sourceFormat, GL_UNSIGNED_BYTE,
srcColorsPadded); srcColorsPadded);
} }
void testCopyTexture(GLenum sourceFormat, void testCopyTexture(GLenum sourceTarget,
GLenum sourceFormat,
GLenum destFormat, GLenum destFormat,
bool flipY, bool flipY,
bool premultiplyAlpha, bool premultiplyAlpha,
...@@ -427,7 +429,8 @@ class CopyTextureVariationsTest : public ANGLETestWithParam<CopyTextureVariation ...@@ -427,7 +429,8 @@ class CopyTextureVariationsTest : public ANGLETestWithParam<CopyTextureVariation
for (size_t i = 0; i < colorCount - 3; ++i) for (size_t i = 0; i < colorCount - 3; ++i)
{ {
initializeSourceTexture(sourceFormat, &srcColors[i * componentCount], componentCount); initializeSourceTexture(sourceTarget, sourceFormat, &srcColors[i * componentCount],
componentCount);
glCopyTextureCHROMIUM(mTextures[0], 0, GL_TEXTURE_2D, mTextures[1], 0, destFormat, glCopyTextureCHROMIUM(mTextures[0], 0, GL_TEXTURE_2D, mTextures[1], 0, destFormat,
GL_UNSIGNED_BYTE, flipY, premultiplyAlpha, unmultiplyAlpha); GL_UNSIGNED_BYTE, flipY, premultiplyAlpha, unmultiplyAlpha);
...@@ -456,7 +459,8 @@ class CopyTextureVariationsTest : public ANGLETestWithParam<CopyTextureVariation ...@@ -456,7 +459,8 @@ class CopyTextureVariationsTest : public ANGLETestWithParam<CopyTextureVariation
} }
} }
void testCopySubTexture(GLenum sourceFormat, void testCopySubTexture(GLenum sourceTarget,
GLenum sourceFormat,
GLenum destFormat, GLenum destFormat,
bool flipY, bool flipY,
bool premultiplyAlpha, bool premultiplyAlpha,
...@@ -480,7 +484,8 @@ class CopyTextureVariationsTest : public ANGLETestWithParam<CopyTextureVariation ...@@ -480,7 +484,8 @@ class CopyTextureVariationsTest : public ANGLETestWithParam<CopyTextureVariation
for (size_t i = 0; i < colorCount - 3; ++i) for (size_t i = 0; i < colorCount - 3; ++i)
{ {
initializeSourceTexture(sourceFormat, &srcColors[i * componentCount], componentCount); initializeSourceTexture(sourceTarget, sourceFormat, &srcColors[i * componentCount],
componentCount);
glBindTexture(GL_TEXTURE_2D, mTextures[1]); glBindTexture(GL_TEXTURE_2D, mTextures[1]);
glTexImage2D(GL_TEXTURE_2D, 0, destFormat, 2, 2, 0, destFormat, GL_UNSIGNED_BYTE, glTexImage2D(GL_TEXTURE_2D, 0, destFormat, 2, 2, 0, destFormat, GL_UNSIGNED_BYTE,
...@@ -862,14 +867,30 @@ constexpr GLenum kCopyTextureVariationsDstFormats[] = {GL_RGB, GL_RGBA, GL_BGRA_ ...@@ -862,14 +867,30 @@ constexpr GLenum kCopyTextureVariationsDstFormats[] = {GL_RGB, GL_RGBA, GL_BGRA_
TEST_P(CopyTextureVariationsTest, CopyTexture) TEST_P(CopyTextureVariationsTest, CopyTexture)
{ {
testCopyTexture(std::get<1>(GetParam()), std::get<2>(GetParam()), std::get<3>(GetParam()), testCopyTexture(GL_TEXTURE_2D, std::get<1>(GetParam()), std::get<2>(GetParam()),
std::get<4>(GetParam()), std::get<5>(GetParam())); std::get<3>(GetParam()), std::get<4>(GetParam()), std::get<5>(GetParam()));
} }
TEST_P(CopyTextureVariationsTest, CopySubTexture) TEST_P(CopyTextureVariationsTest, CopySubTexture)
{ {
testCopySubTexture(std::get<1>(GetParam()), std::get<2>(GetParam()), std::get<3>(GetParam()), testCopySubTexture(GL_TEXTURE_2D, std::get<1>(GetParam()), std::get<2>(GetParam()),
std::get<4>(GetParam()), std::get<5>(GetParam())); std::get<3>(GetParam()), std::get<4>(GetParam()), std::get<5>(GetParam()));
}
TEST_P(CopyTextureVariationsTest, CopyTextureRectangle)
{
ANGLE_SKIP_TEST_IF(!EnsureGLExtensionEnabled("GL_ANGLE_texture_rectangle"));
testCopyTexture(GL_TEXTURE_RECTANGLE_ANGLE, std::get<1>(GetParam()), std::get<2>(GetParam()),
std::get<3>(GetParam()), std::get<4>(GetParam()), std::get<5>(GetParam()));
}
TEST_P(CopyTextureVariationsTest, CopySubTextureRectangle)
{
ANGLE_SKIP_TEST_IF(!EnsureGLExtensionEnabled("GL_ANGLE_texture_rectangle"));
testCopySubTexture(GL_TEXTURE_RECTANGLE_ANGLE, std::get<1>(GetParam()), std::get<2>(GetParam()),
std::get<3>(GetParam()), std::get<4>(GetParam()), std::get<5>(GetParam()));
} }
// Test that copying to cube maps works // Test that copying to cube maps works
......
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