Commit fb1c2fe8 by shrekshao Committed by Commit Bot

EXT_texture_norm16 readpixels fix

Adding validation logic for RGBA16 readpixels. Change readPixels format from UNSIGNED_BYTE to UNSIGNED_SHORT in the test. FYI: According to https://www.khronos.org/registry/OpenGL/extensions/EXT/EXT_texture_norm16.txt ReadPixels format and type used during CopyTex* are limited to RGBA Bug: 1024387, 1000354, angleproject:1365, angleproject:4089 Change-Id: I70517f255fe335f60e55bdf15f7ebf82e3de0800 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/1914127Reviewed-by: 's avatarGeoff Lang <geofflang@chromium.org> Commit-Queue: Shrek Shao <shrekshao@google.com>
parent f3d35f8f
...@@ -987,6 +987,7 @@ static InternalFormatInfoMap BuildInternalFormatInfoMap() ...@@ -987,6 +987,7 @@ static InternalFormatInfoMap BuildInternalFormatInfoMap()
AddRGBAFormat(&map, GL_RGBA, false, 4, 4, 4, 4, 0, GL_RGBA, GL_UNSIGNED_SHORT_4_4_4_4, GL_UNSIGNED_NORMALIZED, false, AlwaysSupported, AlwaysSupported, RequireES<2, 0>, NeverSupported); AddRGBAFormat(&map, GL_RGBA, false, 4, 4, 4, 4, 0, GL_RGBA, GL_UNSIGNED_SHORT_4_4_4_4, GL_UNSIGNED_NORMALIZED, false, AlwaysSupported, AlwaysSupported, RequireES<2, 0>, NeverSupported);
AddRGBAFormat(&map, GL_RGBA, false, 5, 5, 5, 1, 0, GL_RGBA, GL_UNSIGNED_SHORT_5_5_5_1, GL_UNSIGNED_NORMALIZED, false, AlwaysSupported, AlwaysSupported, RequireES<2, 0>, NeverSupported); AddRGBAFormat(&map, GL_RGBA, false, 5, 5, 5, 1, 0, GL_RGBA, GL_UNSIGNED_SHORT_5_5_5_1, GL_UNSIGNED_NORMALIZED, false, AlwaysSupported, AlwaysSupported, RequireES<2, 0>, NeverSupported);
AddRGBAFormat(&map, GL_RGBA, false, 8, 8, 8, 8, 0, GL_RGBA, GL_UNSIGNED_BYTE, GL_UNSIGNED_NORMALIZED, false, AlwaysSupported, AlwaysSupported, RequireES<2, 0>, NeverSupported); AddRGBAFormat(&map, GL_RGBA, false, 8, 8, 8, 8, 0, GL_RGBA, GL_UNSIGNED_BYTE, GL_UNSIGNED_NORMALIZED, false, AlwaysSupported, AlwaysSupported, RequireES<2, 0>, NeverSupported);
AddRGBAFormat(&map, GL_RGBA, false, 16, 16, 16, 16, 0, GL_RGBA, GL_UNSIGNED_SHORT, GL_UNSIGNED_NORMALIZED, false, RequireExt<&Extensions::textureNorm16>, AlwaysSupported, RequireExt<&Extensions::textureNorm16>, NeverSupported);
AddRGBAFormat(&map, GL_RGBA, false, 10, 10, 10, 2, 0, GL_RGBA, GL_UNSIGNED_INT_2_10_10_10_REV, GL_UNSIGNED_NORMALIZED, false, RequireExt<&Extensions::textureFormat2101010REV>, AlwaysSupported, NeverSupported, NeverSupported); AddRGBAFormat(&map, GL_RGBA, false, 10, 10, 10, 2, 0, GL_RGBA, GL_UNSIGNED_INT_2_10_10_10_REV, GL_UNSIGNED_NORMALIZED, false, RequireExt<&Extensions::textureFormat2101010REV>, AlwaysSupported, NeverSupported, NeverSupported);
AddRGBAFormat(&map, GL_RGBA, false, 8, 8, 8, 8, 0, GL_RGBA, GL_BYTE, GL_SIGNED_NORMALIZED, false, NeverSupported, NeverSupported, NeverSupported, NeverSupported); AddRGBAFormat(&map, GL_RGBA, false, 8, 8, 8, 8, 0, GL_RGBA, GL_BYTE, GL_SIGNED_NORMALIZED, false, NeverSupported, NeverSupported, NeverSupported, NeverSupported);
AddRGBAFormat(&map, GL_SRGB, false, 8, 8, 8, 0, 0, GL_SRGB, GL_UNSIGNED_BYTE, GL_UNSIGNED_NORMALIZED, true, RequireExt<&Extensions::sRGB>, AlwaysSupported, NeverSupported, NeverSupported); AddRGBAFormat(&map, GL_SRGB, false, 8, 8, 8, 0, 0, GL_SRGB, GL_UNSIGNED_BYTE, GL_UNSIGNED_NORMALIZED, true, RequireExt<&Extensions::sRGB>, AlwaysSupported, NeverSupported, NeverSupported);
......
...@@ -157,21 +157,25 @@ bool ValidReadPixelsFormatEnum(Context *context, GLenum format) ...@@ -157,21 +157,25 @@ bool ValidReadPixelsFormatEnum(Context *context, GLenum format)
} }
bool ValidReadPixelsFormatType(Context *context, bool ValidReadPixelsFormatType(Context *context,
GLenum framebufferComponentType, const gl::InternalFormat *info,
GLenum format, GLenum format,
GLenum type) GLenum type)
{ {
switch (framebufferComponentType) switch (info->componentType)
{ {
case GL_UNSIGNED_NORMALIZED: case GL_UNSIGNED_NORMALIZED:
// TODO(geofflang): Don't accept BGRA here. Some chrome internals appear to try to use // TODO(geofflang): Don't accept BGRA here. Some chrome internals appear to try to use
// ReadPixels with BGRA even if the extension is not present // ReadPixels with BGRA even if the extension is not present
return (format == GL_RGBA && type == GL_UNSIGNED_BYTE) || return (format == GL_RGBA && type == GL_UNSIGNED_BYTE && info->pixelBytes >= 1) ||
(context->getExtensions().textureNorm16 && format == GL_RGBA &&
type == GL_UNSIGNED_SHORT && info->pixelBytes >= 2) ||
(context->getExtensions().readFormatBGRA && format == GL_BGRA_EXT && (context->getExtensions().readFormatBGRA && format == GL_BGRA_EXT &&
type == GL_UNSIGNED_BYTE); type == GL_UNSIGNED_BYTE);
case GL_SIGNED_NORMALIZED: case GL_SIGNED_NORMALIZED:
return (format == GL_RGBA && type == GL_UNSIGNED_BYTE); return (format == GL_RGBA && type == GL_BYTE && info->pixelBytes >= 1) ||
(context->getExtensions().textureNorm16 && format == GL_RGBA &&
type == GL_UNSIGNED_SHORT && info->pixelBytes >= 2);
case GL_INT: case GL_INT:
return (format == GL_RGBA_INTEGER && type == GL_INT); return (format == GL_RGBA_INTEGER && type == GL_INT);
...@@ -5617,10 +5621,8 @@ bool ValidateReadPixelsBase(Context *context, ...@@ -5617,10 +5621,8 @@ bool ValidateReadPixelsBase(Context *context,
GLenum currentType = GL_NONE; GLenum currentType = GL_NONE;
ANGLE_VALIDATION_TRY(readFramebuffer->getImplementationColorReadType(context, &currentType)); ANGLE_VALIDATION_TRY(readFramebuffer->getImplementationColorReadType(context, &currentType));
GLenum currentComponentType = readBuffer->getFormat().info->componentType;
bool validFormatTypeCombination = bool validFormatTypeCombination =
ValidReadPixelsFormatType(context, currentComponentType, format, type); ValidReadPixelsFormatType(context, readBuffer->getFormat().info, format, type);
if (!(currentFormat == format && currentType == type) && !validFormatTypeCombination) if (!(currentFormat == format && currentType == type) && !validFormatTypeCombination)
{ {
......
...@@ -48,6 +48,30 @@ GLColor SliceFormatColor(GLenum format, GLColor full) ...@@ -48,6 +48,30 @@ GLColor SliceFormatColor(GLenum format, GLColor full)
} }
} }
GLColor16UI SliceFormatColor16UI(GLenum format, GLColor16UI full)
{
switch (format)
{
case GL_RED:
return GLColor16UI(full.R, 0, 0, 0xFFFF);
case GL_RG:
return GLColor16UI(full.R, full.G, 0, 0xFFFF);
case GL_RGB:
return GLColor16UI(full.R, full.G, full.B, 0xFFFF);
case GL_RGBA:
return full;
case GL_LUMINANCE:
return GLColor16UI(full.R, full.R, full.R, 0xFFFF);
case GL_ALPHA:
return GLColor16UI(0, 0, 0, full.R);
case GL_LUMINANCE_ALPHA:
return GLColor16UI(full.R, full.R, full.R, full.G);
default:
EXPECT_TRUE(false);
return GLColor16UI(0xFFFF, 0xFFFF, 0xFFFF, 0xFFFF);
}
}
// As above, for 32F colors // As above, for 32F colors
GLColor32F SliceFormatColor32F(GLenum format, GLColor32F full) GLColor32F SliceFormatColor32F(GLenum format, GLColor32F full)
{ {
...@@ -4151,10 +4175,9 @@ class Texture2DNorm16TestES3 : public Texture2DTestES3 ...@@ -4151,10 +4175,9 @@ class Texture2DNorm16TestES3 : public Texture2DTestES3
drawQuad(mProgram, "position", 0.5f); drawQuad(mProgram, "position", 0.5f);
GLubyte expectedValue = static_cast<GLubyte>(pixelValue >> 8); EXPECT_PIXEL_16UI_COLOR(0, 0,
EXPECT_PIXEL_COLOR_EQ(0, 0, SliceFormatColor16UI(format, GLColor16UI(pixelValue, pixelValue,
SliceFormatColor(format, GLColor(expectedValue, expectedValue, pixelValue, pixelValue)));
expectedValue, expectedValue)));
glBindRenderbuffer(GL_RENDERBUFFER, mRenderbuffer); glBindRenderbuffer(GL_RENDERBUFFER, mRenderbuffer);
glRenderbufferStorage(GL_RENDERBUFFER, internalformat, 1, 1); glRenderbufferStorage(GL_RENDERBUFFER, internalformat, 1, 1);
...@@ -4166,18 +4189,20 @@ class Texture2DNorm16TestES3 : public Texture2DTestES3 ...@@ -4166,18 +4189,20 @@ class Texture2DNorm16TestES3 : public Texture2DTestES3
glClearColor(1.0f, 1.0f, 1.0f, 1.0f); glClearColor(1.0f, 1.0f, 1.0f, 1.0f);
glClear(GL_COLOR_BUFFER_BIT); glClear(GL_COLOR_BUFFER_BIT);
EXPECT_PIXEL_COLOR_EQ(0, 0, SliceFormatColor(format, GLColor::white)); EXPECT_PIXEL_16UI_COLOR(
0, 0, SliceFormatColor16UI(format, GLColor16UI(0xFFFF, 0xFFFF, 0xFFFF, 0xFFFF)));
glBindTexture(GL_TEXTURE_2D, mTextures[1]); glBindTexture(GL_TEXTURE_2D, mTextures[1]);
glCopyTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, 0, 0, 1, 1); glCopyTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, 0, 0, 1, 1);
glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, mTextures[1], glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, mTextures[1],
0); 0);
EXPECT_PIXEL_COLOR_EQ(0, 0, SliceFormatColor(format, GLColor::white)); EXPECT_PIXEL_16UI_COLOR(
0, 0, SliceFormatColor16UI(format, GLColor16UI(0xFFFF, 0xFFFF, 0xFFFF, 0xFFFF)));
glBindFramebuffer(GL_FRAMEBUFFER, 0);
ASSERT_GL_NO_ERROR(); ASSERT_GL_NO_ERROR();
glBindFramebuffer(GL_FRAMEBUFFER, 0);
} }
GLuint mTextures[3]; GLuint mTextures[3];
...@@ -4205,8 +4230,6 @@ TEST_P(Texture2DNorm16TestES3, TextureNorm16Test) ...@@ -4205,8 +4230,6 @@ TEST_P(Texture2DNorm16TestES3, TextureNorm16Test)
testNorm16Texture(GL_RGB16_SNORM_EXT, GL_RGB, GL_SHORT); testNorm16Texture(GL_RGB16_SNORM_EXT, GL_RGB, GL_SHORT);
testNorm16Texture(GL_RGBA16_SNORM_EXT, GL_RGBA, GL_SHORT); testNorm16Texture(GL_RGBA16_SNORM_EXT, GL_RGBA, GL_SHORT);
testNorm16Render(GL_R16_EXT, GL_RED, GL_UNSIGNED_SHORT);
testNorm16Render(GL_RG16_EXT, GL_RG, GL_UNSIGNED_SHORT);
testNorm16Render(GL_RGBA16_EXT, GL_RGBA, GL_UNSIGNED_SHORT); testNorm16Render(GL_RGBA16_EXT, GL_RGBA, GL_UNSIGNED_SHORT);
} }
......
...@@ -123,6 +123,15 @@ struct GLColor ...@@ -123,6 +123,15 @@ struct GLColor
static const GLColor magenta; static const GLColor magenta;
}; };
struct GLColor16UI
{
constexpr GLColor16UI() : GLColor16UI(0, 0, 0, 0) {}
constexpr GLColor16UI(GLushort r, GLushort g, GLushort b, GLushort a) : R(r), G(g), B(b), A(a)
{}
GLushort R, G, B, A;
};
struct GLColor32F struct GLColor32F
{ {
constexpr GLColor32F() : GLColor32F(0.0f, 0.0f, 0.0f, 0.0f) {} constexpr GLColor32F() : GLColor32F(0.0f, 0.0f, 0.0f, 0.0f) {}
...@@ -251,6 +260,12 @@ constexpr std::array<GLenum, 6> kCubeFaces = { ...@@ -251,6 +260,12 @@ constexpr std::array<GLenum, 6> kCubeFaces = {
#define EXPECT_PIXEL_8UI(x, y, r, g, b, a) \ #define EXPECT_PIXEL_8UI(x, y, r, g, b, a) \
EXPECT_PIXEL_EQ_HELPER(x, y, r, g, b, a, GLubyte, GL_RGBA_INTEGER, GL_UNSIGNED_BYTE) EXPECT_PIXEL_EQ_HELPER(x, y, r, g, b, a, GLubyte, GL_RGBA_INTEGER, GL_UNSIGNED_BYTE)
#define EXPECT_PIXEL_16UI(x, y, r, g, b, a) \
EXPECT_PIXEL_EQ_HELPER(x, y, r, g, b, a, GLushort, GL_RGBA, GL_UNSIGNED_SHORT)
#define EXPECT_PIXEL_16UI_COLOR(x, y, color) \
EXPECT_PIXEL_16UI(x, y, color.R, color.G, color.B, color.A)
#define EXPECT_PIXEL_RGB_EQUAL(x, y, r, g, b) \ #define EXPECT_PIXEL_RGB_EQUAL(x, y, r, g, b) \
EXPECT_PIXEL_RGB_EQ_HELPER(x, y, r, g, b, GLubyte, GL_RGBA, GL_UNSIGNED_BYTE) EXPECT_PIXEL_RGB_EQ_HELPER(x, y, r, g, b, GLubyte, GL_RGBA, GL_UNSIGNED_BYTE)
......
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