Skip to content

Commit

Permalink
Update TexImage2D Parameter Checking
Browse files Browse the repository at this point in the history 
Update the parameter checking performed within ValidateTexImage2D() to
pass the following tests:

dEQP-GLES2.functional.negative_api.texture.teximage2d_invalid_format
dEQP-GLES2.functional.negative_api.texture.teximage2d_invalid_internalformat
dEQP-GLES2.functional.negative_api.texture.texsubimage2d_invalid_type

Bug: angleproject:3250
Bug: angleproject:3251
Test: angle_deqp_gles2_tests --use-angle=vulkan
Change-Id: I4d9be4fe0a9b377e61e3132db262750e6285464b
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/1534519
Commit-Queue: Tim Van Patten <timvp@google.com>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
  • Loading branch information
@timvpGoogle
timvpGoogle authored and Commit Bot committed Mar 27, 2019
1 parent 74ba10f commit 208af3e
Show file tree
Hide file tree
Showing 4 changed files with 110 additions and 71 deletions.
3 changes: 1 addition & 2 deletions src/libANGLE/Display.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,7 @@ void Display::setAttributes(rx::DisplayImpl *impl, const AttributeMap &attribMap

Error Display::initialize()
{
ASSERT(mImplementation != nullptr);
mImplementation->setBlobCache(&mBlobCache);

// TODO(jmadill): Store Platform in Display and init here.
Expand All @@ -516,8 +517,6 @@ Error Display::initialize()
SCOPED_ANGLE_HISTOGRAM_TIMER("GPU.ANGLE.DisplayInitializeMS");
TRACE_EVENT0("gpu.angle", "egl::Display::initialize");

ASSERT(mImplementation != nullptr);

if (isInitialized())
{
return NoError();
Expand Down
167 changes: 105 additions & 62 deletions src/libANGLE/validationES2.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1155,21 +1155,6 @@ bool ValidateES2TexImageParameters(Context *context,
return false;
}

// From GL_CHROMIUM_color_buffer_float_rgb[a]:
// GL_RGB[A] / GL_RGB[A]32F becomes an allowable format / internalformat parameter pair for
// TexImage2D. The restriction in section 3.7.1 of the OpenGL ES 2.0 spec that the
// internalformat parameter and format parameter of TexImage2D must match is lifted for this
// case.
bool nonEqualFormatsAllowed =
(internalformat == GL_RGB32F && context->getExtensions().colorBufferFloatRGB) ||
(internalformat == GL_RGBA32F && context->getExtensions().colorBufferFloatRGBA);

if (!isSubImage && !isCompressed && internalformat != format && !nonEqualFormatsAllowed)
{
context->validationError(GL_INVALID_OPERATION, kInvalidFormatCombination);
return false;
}

const gl::Caps &caps = context->getCaps();

switch (texType)
Expand Down Expand Up @@ -1225,55 +1210,15 @@ bool ValidateES2TexImageParameters(Context *context,
return false;
}

if (isSubImage)
{
const InternalFormat &textureInternalFormat = *texture->getFormat(target, level).info;
if (textureInternalFormat.internalFormat == GL_NONE)
{
context->validationError(GL_INVALID_OPERATION, kInvalidTextureLevel);
return false;
}

if (format != GL_NONE)
{
if (GetInternalFormatInfo(format, type).sizedInternalFormat !=
textureInternalFormat.sizedInternalFormat)
{
context->validationError(GL_INVALID_OPERATION, kTypeMismatch);
return false;
}
}

if (static_cast<size_t>(xoffset + width) > texture->getWidth(target, level) ||
static_cast<size_t>(yoffset + height) > texture->getHeight(target, level))
{
context->validationError(GL_INVALID_VALUE, kOffsetOverflow);
return false;
}

if (width > 0 && height > 0 && pixels == nullptr &&
context->getState().getTargetBuffer(BufferBinding::PixelUnpack) == nullptr)
{
context->validationError(GL_INVALID_VALUE, kPixelDataNull);
return false;
}
}
else
{
if (texture->getImmutableFormat())
{
context->validationError(GL_INVALID_OPERATION, kTextureIsImmutable);
return false;
}
}

// Verify zero border
if (border != 0)
{
context->validationError(GL_INVALID_VALUE, kInvalidBorder);
return false;
}

bool nonEqualFormatsAllowed = false;

if (isCompressed)
{
GLenum actualInternalFormat =
Expand Down Expand Up @@ -1583,12 +1528,23 @@ bool ValidateES2TexImageParameters(Context *context,
{
switch (internalformat)
{
// Core ES 2.0 formats
case GL_ALPHA:
case GL_LUMINANCE:
case GL_LUMINANCE_ALPHA:
case GL_RGB:
case GL_RGBA:
break;

case GL_RGBA32F:
if (!context->getExtensions().colorBufferFloatRGBA)
{
context->validationError(GL_INVALID_ENUM, kInvalidFormat);
return false;
}

nonEqualFormatsAllowed = true;

if (type != GL_FLOAT)
{
context->validationError(GL_INVALID_OPERATION, kMismatchedTypeAndFormat);
Expand All @@ -1607,6 +1563,9 @@ bool ValidateES2TexImageParameters(Context *context,
context->validationError(GL_INVALID_ENUM, kInvalidFormat);
return false;
}

nonEqualFormatsAllowed = true;

if (type != GL_FLOAT)
{
context->validationError(GL_INVALID_OPERATION, kMismatchedTypeAndFormat);
Expand All @@ -1619,8 +1578,44 @@ bool ValidateES2TexImageParameters(Context *context,
}
break;

default:
case GL_BGRA_EXT:
if (!context->getExtensions().textureFormatBGRA8888)
{
context->validationError(GL_INVALID_ENUM, kInvalidFormat);
return false;
}
break;

case GL_DEPTH_COMPONENT:
case GL_DEPTH_STENCIL:
if (!context->getExtensions().depthTextures)
{
context->validationError(GL_INVALID_ENUM, kInvalidFormat);
return false;
}
break;

case GL_RED:
case GL_RG:
if (!context->getExtensions().textureRG)
{
context->validationError(GL_INVALID_ENUM, kInvalidFormat);
return false;
}
break;

case GL_SRGB_EXT:
case GL_SRGB_ALPHA_EXT:
if (!context->getExtensions().sRGB)
{
context->validationError(GL_INVALID_ENUM, kEnumNotSupported);
return false;
}
break;

default:
context->validationError(GL_INVALID_VALUE, kInvalidInternalFormat);
return false;
}
}

Expand All @@ -1642,14 +1637,62 @@ bool ValidateES2TexImageParameters(Context *context,
}
}

GLenum sizeCheckFormat = isSubImage ? format : internalformat;
if (!ValidImageDataSize(context, texType, width, height, 1, sizeCheckFormat, type, pixels,
imageSize))
if (isSubImage)
{
const InternalFormat &textureInternalFormat = *texture->getFormat(target, level).info;
if (textureInternalFormat.internalFormat == GL_NONE)
{
context->validationError(GL_INVALID_OPERATION, kInvalidTextureLevel);
return false;
}

if (format != GL_NONE)
{
if (GetInternalFormatInfo(format, type).sizedInternalFormat !=
textureInternalFormat.sizedInternalFormat)
{
context->validationError(GL_INVALID_OPERATION, kTypeMismatch);
return false;
}
}

if (static_cast<size_t>(xoffset + width) > texture->getWidth(target, level) ||
static_cast<size_t>(yoffset + height) > texture->getHeight(target, level))
{
context->validationError(GL_INVALID_VALUE, kOffsetOverflow);
return false;
}

if (width > 0 && height > 0 && pixels == nullptr &&
context->getState().getTargetBuffer(BufferBinding::PixelUnpack) == nullptr)
{
context->validationError(GL_INVALID_VALUE, kPixelDataNull);
return false;
}
}
else
{
if (texture->getImmutableFormat())
{
context->validationError(GL_INVALID_OPERATION, kTextureIsImmutable);
return false;
}
}

// From GL_CHROMIUM_color_buffer_float_rgb[a]:
// GL_RGB[A] / GL_RGB[A]32F becomes an allowable format / internalformat parameter pair for
// TexImage2D. The restriction in section 3.7.1 of the OpenGL ES 2.0 spec that the
// internalformat parameter and format parameter of TexImage2D must match is lifted for this
// case.
if (!isSubImage && !isCompressed && internalformat != format && !nonEqualFormatsAllowed)
{
context->validationError(GL_INVALID_OPERATION, kInvalidFormatCombination);
return false;
}

return true;
GLenum sizeCheckFormat = isSubImage ? format : internalformat;
return ValidImageDataSize(context, texType, width, height, 1, sizeCheckFormat, type, pixels,
imageSize);
}

bool ValidateES2TexStorageParameters(Context *context,
Expand Down
5 changes: 0 additions & 5 deletions src/tests/deqp_support/deqp_gles2_test_expectations.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,6 @@
// Half float OES either has an implementation bug or a dEQP bug.
3283 DEBUG RELEASE : dEQP-GLES2.functional.fbo.completeness.renderable.texture.color0.rgba_half_float_oes = FAIL

// Negative API failures.
3250 DEBUG RELEASE : dEQP-GLES2.functional.negative_api.texture.teximage2d_invalid_format = FAIL
3250 DEBUG RELEASE : dEQP-GLES2.functional.negative_api.texture.teximage2d_invalid_internalformat = FAIL
3251 DEBUG RELEASE : dEQP-GLES2.functional.negative_api.texture.texsubimage2d_invalid_type = FAIL

// Shader failures.
3285 DEBUG RELEASE : dEQP-GLES2.functional.shaders.preprocessor.pragmas.pragma_fragment = FAIL
3285 DEBUG RELEASE : dEQP-GLES2.functional.shaders.preprocessor.extensions.after_non_preprocessing_tokens_vertex = FAIL
Expand Down
6 changes: 4 additions & 2 deletions src/tests/gl_tests/WebGLCompatibilityTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3382,10 +3382,12 @@ TEST_P(WebGLCompatibilityTest, SizedRGBA32FFormats)
glBindTexture(GL_TEXTURE_2D, texture);

glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA32F, 1, 1, 0, GL_RGBA, GL_FLOAT, nullptr);
EXPECT_GL_ERROR(GL_INVALID_OPERATION);
// dEQP implicitly defines error code ordering
EXPECT_GL_ERROR(GL_INVALID_ENUM);

glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB32F, 1, 1, 0, GL_RGB, GL_FLOAT, nullptr);
EXPECT_GL_ERROR(GL_INVALID_OPERATION);
// dEQP implicitly defines error code ordering
EXPECT_GL_ERROR(GL_INVALID_ENUM);

if (extensionRequestable("GL_CHROMIUM_color_buffer_float_rgba"))
{
Expand Down

0 comments on commit 208af3e

Please sign in to comment.