Metal Fix norm16 subimage uploads w/ signed types
The supported texture format tables would have entries for
a signed or unsigned internal format and matching type.
The texSubImage variants do not communicate the sized internal format,
rather unsized format type. Data parameters are validated based on
format, not internal format. For these, the signed variants were
missing.
Add texture format table entries for signed types that match the format
parameters.
Add a precautionary validation case so that future such bugs would not
pass the control flow forward, into the copying phase.
Fixed: angleproject:8014
Change-Id: I5b00cca8672691fdb953f62093d62feaa724db99
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4238881
Commit-Queue: Kimmo Kinnunen <kkinnunen@apple.com>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Auto-Submit: Kimmo Kinnunen <kkinnunen@apple.com>
diff --git a/src/libANGLE/ErrorStrings.h b/src/libANGLE/ErrorStrings.h
index a0e2e21..9a14f8a 100644
--- a/src/libANGLE/ErrorStrings.h
+++ b/src/libANGLE/ErrorStrings.h
@@ -220,6 +220,7 @@
MSG kInsufficientParams = "More parameters are required than were provided.";
MSG kInsufficientVertexBufferSize = "Vertex buffer is not big enough for the draw call";
MSG kIntegerOverflow = "Integer overflow.";
+MSG kInternalErrorFormatNotFound = "Internal error: unknown internal format.";
MSG kInternalFormatRequiresTexture2D = "internalformat is an ETC1 or PVRTC1 format.";
MSG kInternalFormatRequiresTexture2DArray = "internalformat is an ETC2/EAC format and target is not GL_TEXTURE_2D_ARRAY.";
MSG kInternalFormatRequiresTexture2DArrayS3TC = "internalformat is an S3TC format and target is not GL_TEXTURE_2D_ARRAY.";
diff --git a/src/libANGLE/formatutils.cpp b/src/libANGLE/formatutils.cpp
index becd277..fc41b9d 100644
--- a/src/libANGLE/formatutils.cpp
+++ b/src/libANGLE/formatutils.cpp
@@ -1359,17 +1359,22 @@
AddRGBAXFormat(&map, GL_RED, false, FB< 8, 0, 0, 0, 0, 0>(), GL_RED, GL_UNSIGNED_BYTE, GL_UNSIGNED_NORMALIZED, false, RequireExt<&Extensions::textureRgEXT>, AlwaysSupported, RequireExt<&Extensions::textureRgEXT>, NeverSupported, NeverSupported);
AddRGBAXFormat(&map, GL_RED, false, FB< 8, 0, 0, 0, 0, 0>(), GL_RED, GL_BYTE, GL_SIGNED_NORMALIZED, false, NeverSupported, NeverSupported, NeverSupported, NeverSupported, NeverSupported);
AddRGBAXFormat(&map, GL_RED, false, FB<16, 0, 0, 0, 0, 0>(), GL_RED, GL_UNSIGNED_SHORT, GL_UNSIGNED_NORMALIZED, false, RequireExt<&Extensions::textureNorm16EXT>, AlwaysSupported, RequireExt<&Extensions::textureNorm16EXT>, NeverSupported, NeverSupported);
+ AddRGBAXFormat(&map, GL_RED, false, FB<16, 0, 0, 0, 0, 0>(), GL_RED, GL_SHORT, GL_SIGNED_NORMALIZED, false, NeverSupported, AlwaysSupported, NeverSupported, NeverSupported, NeverSupported);
AddRGBAXFormat(&map, GL_RG, false, FB< 8, 8, 0, 0, 0, 0>(), GL_RG, GL_UNSIGNED_BYTE, GL_UNSIGNED_NORMALIZED, false, RequireExt<&Extensions::textureRgEXT>, AlwaysSupported, RequireExt<&Extensions::textureRgEXT>, NeverSupported, NeverSupported);
AddRGBAXFormat(&map, GL_RG, false, FB< 8, 8, 0, 0, 0, 0>(), GL_RG, GL_BYTE, GL_SIGNED_NORMALIZED, false, NeverSupported, NeverSupported, NeverSupported, NeverSupported, NeverSupported);
AddRGBAXFormat(&map, GL_RG, false, FB<16, 16, 0, 0, 0, 0>(), GL_RG, GL_UNSIGNED_SHORT, GL_UNSIGNED_NORMALIZED, false, RequireExt<&Extensions::textureNorm16EXT>, AlwaysSupported, RequireExt<&Extensions::textureNorm16EXT>, NeverSupported, NeverSupported);
+ AddRGBAXFormat(&map, GL_RG, false, FB<16, 16, 0, 0, 0, 0>(), GL_RG, GL_SHORT, GL_SIGNED_NORMALIZED, false, NeverSupported, AlwaysSupported, NeverSupported, NeverSupported, NeverSupported);
AddRGBAXFormat(&map, GL_RGB, false, FB< 8, 8, 8, 0, 0, 0>(), GL_RGB, GL_UNSIGNED_BYTE, GL_UNSIGNED_NORMALIZED, false, AlwaysSupported, AlwaysSupported, RequireESOrExt<2, 0, &Extensions::framebufferObjectOES>, NeverSupported, NeverSupported);
AddRGBAXFormat(&map, GL_RGB, false, FB< 5, 6, 5, 0, 0, 0>(), GL_RGB, GL_UNSIGNED_SHORT_5_6_5, GL_UNSIGNED_NORMALIZED, false, AlwaysSupported, AlwaysSupported, RequireESOrExt<2, 0, &Extensions::framebufferObjectOES>, NeverSupported, NeverSupported);
AddRGBAXFormat(&map, GL_RGB, false, FB< 8, 8, 8, 0, 0, 0>(), GL_RGB, GL_BYTE, GL_SIGNED_NORMALIZED, false, NeverSupported, NeverSupported, NeverSupported, NeverSupported, NeverSupported);
AddRGBAXFormat(&map, GL_RGB, false, FB<10, 10, 10, 0, 0, 2>(), GL_RGB, GL_UNSIGNED_INT_2_10_10_10_REV, GL_UNSIGNED_NORMALIZED, false, RequireExt<&Extensions::textureType2101010REVEXT>, AlwaysSupported, NeverSupported, NeverSupported, NeverSupported);
+ AddRGBAXFormat(&map, GL_RGB, false, FB<16, 16, 16, 0, 0, 0>(), GL_RGB, GL_UNSIGNED_SHORT, GL_UNSIGNED_NORMALIZED, false, RequireExt<&Extensions::textureNorm16EXT>, AlwaysSupported, RequireExt<&Extensions::textureNorm16EXT>, NeverSupported, NeverSupported);
+ AddRGBAXFormat(&map, GL_RGB, false, FB<16, 16, 16, 0, 0, 0>(), GL_RGB, GL_SHORT, GL_SIGNED_NORMALIZED, false, NeverSupported, AlwaysSupported, NeverSupported, NeverSupported, NeverSupported);
AddRGBAXFormat(&map, GL_RGBA, false, FB< 4, 4, 4, 4, 0, 0>(), GL_RGBA, GL_UNSIGNED_SHORT_4_4_4_4, GL_UNSIGNED_NORMALIZED, false, AlwaysSupported, AlwaysSupported, RequireESOrExt<2, 0, &Extensions::framebufferObjectOES>, NeverSupported, NeverSupported);
AddRGBAXFormat(&map, GL_RGBA, false, FB< 5, 5, 5, 1, 0, 0>(), GL_RGBA, GL_UNSIGNED_SHORT_5_5_5_1, GL_UNSIGNED_NORMALIZED, false, AlwaysSupported, AlwaysSupported, RequireESOrExt<2, 0, &Extensions::framebufferObjectOES>, NeverSupported, NeverSupported);
AddRGBAXFormat(&map, GL_RGBA, false, FB< 8, 8, 8, 8, 0, 0>(), GL_RGBA, GL_UNSIGNED_BYTE, GL_UNSIGNED_NORMALIZED, false, AlwaysSupported, AlwaysSupported, RequireESOrExt<2, 0, &Extensions::framebufferObjectOES>, NeverSupported, NeverSupported);
AddRGBAXFormat(&map, GL_RGBA, false, FB<16, 16, 16, 16, 0, 0>(), GL_RGBA, GL_UNSIGNED_SHORT, GL_UNSIGNED_NORMALIZED, false, RequireExt<&Extensions::textureNorm16EXT>, AlwaysSupported, RequireExt<&Extensions::textureNorm16EXT>, NeverSupported, NeverSupported);
+ AddRGBAXFormat(&map, GL_RGBA, false, FB<16, 16, 16, 16, 0, 0>(), GL_RGBA, GL_SHORT, GL_SIGNED_NORMALIZED, false, NeverSupported, AlwaysSupported, NeverSupported, NeverSupported, NeverSupported);
AddRGBAXFormat(&map, GL_RGBA, false, FB<10, 10, 10, 2, 0, 0>(), GL_RGBA, GL_UNSIGNED_INT_2_10_10_10_REV, GL_UNSIGNED_NORMALIZED, false, RequireExt<&Extensions::textureType2101010REVEXT>, AlwaysSupported, NeverSupported, NeverSupported, NeverSupported);
AddRGBAXFormat(&map, GL_RGBA, false, FB< 8, 8, 8, 8, 0, 0>(), GL_RGBA, GL_BYTE, GL_SIGNED_NORMALIZED, false, NeverSupported, NeverSupported, NeverSupported, NeverSupported, NeverSupported);
AddRGBAXFormat(&map, GL_SRGB, false, FB< 8, 8, 8, 0, 0, 0>(), GL_SRGB, GL_UNSIGNED_BYTE, GL_UNSIGNED_NORMALIZED, true, RequireExt<&Extensions::sRGBEXT>, AlwaysSupported, NeverSupported, NeverSupported, NeverSupported);
diff --git a/src/libANGLE/validationES.cpp b/src/libANGLE/validationES.cpp
index a682f15..c497c2a 100644
--- a/src/libANGLE/validationES.cpp
+++ b/src/libANGLE/validationES.cpp
@@ -1251,7 +1251,12 @@
// ...the data would be unpacked from the buffer object such that the memory reads required
// would exceed the data store size.
const InternalFormat &formatInfo = GetInternalFormatInfo(format, type);
- ASSERT(formatInfo.internalFormat != GL_NONE);
+ if (formatInfo.internalFormat == GL_NONE)
+ {
+ UNREACHABLE();
+ context->validationError(entryPoint, GL_INVALID_OPERATION, kInternalErrorFormatNotFound);
+ return false;
+ }
const Extents size(width, height, depth);
const auto &unpack = context->getState().getUnpackState();
diff --git a/src/tests/gl_tests/TextureTest.cpp b/src/tests/gl_tests/TextureTest.cpp
index afc7875..e128d23 100644
--- a/src/tests/gl_tests/TextureTest.cpp
+++ b/src/tests/gl_tests/TextureTest.cpp
@@ -7813,22 +7813,35 @@
glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, mTextures[0],
0);
- glBindTexture(GL_TEXTURE_2D, mTextures[0]);
- glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA16_EXT, 1, 1, 0, GL_RGBA, GL_UNSIGNED_SHORT, nullptr);
+ for (int i = 0; i < 2; ++i)
+ {
+ bool isSubImage = i == 1;
+ SCOPED_TRACE("is subimage:" + std::to_string(isSubImage));
+ glBindTexture(GL_TEXTURE_2D, mTextures[0]);
+ glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA16_EXT, 1, 1, 0, GL_RGBA, GL_UNSIGNED_SHORT,
+ nullptr);
- glBindTexture(GL_TEXTURE_2D, mTextures[1]);
- glTexImage2D(GL_TEXTURE_2D, 0, internalformat, 1, 1, 0, format, type, imageData);
+ glBindTexture(GL_TEXTURE_2D, mTextures[1]);
+ if (isSubImage)
+ {
+ glTexImage2D(GL_TEXTURE_2D, 0, internalformat, 1, 1, 0, format, type, nullptr);
+ glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, 1, 1, format, type, imageData);
+ }
+ else
+ {
+ glTexImage2D(GL_TEXTURE_2D, 0, internalformat, 1, 1, 0, format, type, imageData);
+ }
+ EXPECT_GL_NO_ERROR();
- EXPECT_GL_NO_ERROR();
+ drawQuad(mProgram, "position", 0.5f);
- drawQuad(mProgram, "position", 0.5f);
+ GLubyte expectedValue =
+ (type == GL_SHORT) ? 0xFF : static_cast<GLubyte>(pixelValue >> 8);
- GLubyte expectedValue = (type == GL_SHORT) ? 0xFF : static_cast<GLubyte>(pixelValue >> 8);
-
- EXPECT_PIXEL_COLOR_EQ(0, 0,
- SliceFormatColor(format, GLColor(expectedValue, expectedValue,
- expectedValue, expectedValue)));
-
+ EXPECT_PIXEL_COLOR_EQ(0, 0,
+ SliceFormatColor(format, GLColor(expectedValue, expectedValue,
+ expectedValue, expectedValue)));
+ }
glBindFramebuffer(GL_FRAMEBUFFER, 0);
ASSERT_GL_NO_ERROR();
