Pixel unpack buffer validation follow up
2 fixes:
- The offset check was removed, as it only affects the pointer
and not the size
- The modulo check is on the type only and not the entire image size
Change-Id: I8c4b64e845b2fae61959d7c62d2c5dc222249c68
Reviewed-on: https://swiftshader-review.googlesource.com/14009
Reviewed-by: Alexis Hétu <sugoi@google.com>
Tested-by: Alexis Hétu <sugoi@google.com>
diff --git a/src/OpenGL/libGLESv2/Context.cpp b/src/OpenGL/libGLESv2/Context.cpp
index 8a1a4b7..49e6ba2 100644
--- a/src/OpenGL/libGLESv2/Context.cpp
+++ b/src/OpenGL/libGLESv2/Context.cpp
@@ -1585,11 +1585,10 @@
GLsizei inputWidth = (mState.unpackInfo.rowLength == 0) ? width : mState.unpackInfo.rowLength;
GLsizei inputPitch = egl::ComputePitch(inputWidth, format, type, mState.unpackInfo.alignment);
GLsizei inputHeight = (mState.unpackInfo.imageHeight == 0) ? height : mState.unpackInfo.imageHeight;
- size_t offset = egl::ComputePackingOffset(format, type, inputWidth, inputHeight, mState.unpackInfo.alignment, mState.unpackInfo.skipImages, mState.unpackInfo.skipRows, mState.unpackInfo.skipPixels);
- return inputPitch * inputHeight * depth + static_cast<GLsizei>(offset);
+ return inputPitch * inputHeight * depth;
}
-GLenum Context::getPixels(const GLvoid **data, GLsizei imageSize) const
+GLenum Context::getPixels(const GLvoid **data, GLenum type, GLsizei imageSize) const
{
if(mState.pixelUnpackBuffer)
{
@@ -1597,7 +1596,7 @@
{
if(mState.pixelUnpackBuffer->isMapped() ||
(mState.pixelUnpackBuffer->size() < static_cast<size_t>(imageSize)) ||
- ((*data) && (imageSize % static_cast<GLsizei>((ptrdiff_t)(*data)))))
+ (static_cast<GLsizei>((ptrdiff_t)(*data)) % GetTypeSize(type)))
{
return GL_INVALID_OPERATION;
}
diff --git a/src/OpenGL/libGLESv2/Context.h b/src/OpenGL/libGLESv2/Context.h
index 254ef1f..1b1f807 100644
--- a/src/OpenGL/libGLESv2/Context.h
+++ b/src/OpenGL/libGLESv2/Context.h
@@ -647,7 +647,7 @@
Buffer *getPixelUnpackBuffer() const;
Buffer *getGenericUniformBuffer() const;
GLsizei getRequiredBufferSize(GLsizei width, GLsizei height, GLsizei depth, GLint internalformat, GLenum type) const;
- GLenum getPixels(const GLvoid **data, GLsizei imageSize) const;
+ GLenum getPixels(const GLvoid **data, GLenum type, GLsizei imageSize) const;
bool getBuffer(GLenum target, es2::Buffer **buffer) const;
Program *getCurrentProgram() const;
Texture2D *getTexture2D() const;
diff --git a/src/OpenGL/libGLESv2/libGLESv2.cpp b/src/OpenGL/libGLESv2/libGLESv2.cpp
index 9c44665..2ba9176 100644
--- a/src/OpenGL/libGLESv2/libGLESv2.cpp
+++ b/src/OpenGL/libGLESv2/libGLESv2.cpp
@@ -876,7 +876,7 @@
return error(GL_INVALID_OPERATION);
}
- GLenum validationError = context->getPixels(&data, imageSize);
+ GLenum validationError = context->getPixels(&data, texture->getType(target, level), imageSize);
if(validationError != GL_NONE)
{
return error(validationError);
@@ -903,7 +903,7 @@
case GL_TEXTURE_CUBE_MAP_NEGATIVE_Z:
{
- GLenum validationError = context->getPixels(&data, imageSize);
+ GLenum validationError = context->getPixels(&data, texture->getType(target, level), imageSize);
if(validationError != GL_NONE)
{
return error(validationError);
@@ -977,7 +977,7 @@
if(validationError == GL_NONE)
{
- validationError = context->getPixels(&data, imageSize);
+ validationError = context->getPixels(&data, texture->getType(target, level), imageSize);
}
if(validationError == GL_NONE)
@@ -997,7 +997,7 @@
if(validationError == GL_NONE)
{
- validationError = context->getPixels(&data, imageSize);
+ validationError = context->getPixels(&data, texture->getType(target, level), imageSize);
}
if(validationError == GL_NONE)
@@ -5129,7 +5129,7 @@
GLenum sizedInternalFormat = GetSizedInternalFormat(format, type);
- validationError = context->getPixels(&data, context->getRequiredBufferSize(width, height, 1, sizedInternalFormat, type));
+ validationError = context->getPixels(&data, type, context->getRequiredBufferSize(width, height, 1, sizedInternalFormat, type));
if(validationError != GL_NONE)
{
return error(validationError);
@@ -5502,7 +5502,7 @@
{
GLenum sizedInternalFormat = GetSizedInternalFormat(format, type);
- GLenum validationError = context->getPixels(&data, context->getRequiredBufferSize(width, height, 1, sizedInternalFormat, type));
+ GLenum validationError = context->getPixels(&data, type, context->getRequiredBufferSize(width, height, 1, sizedInternalFormat, type));
if(validationError != GL_NONE)
{
return error(validationError);
@@ -6361,7 +6361,7 @@
}
GLenum sizedInternalFormat = GetSizedInternalFormat(internalformat, type);
- GLenum validationError = context->getPixels(&data, context->getRequiredBufferSize(width, height, depth, sizedInternalFormat, type));
+ GLenum validationError = context->getPixels(&data, type, context->getRequiredBufferSize(width, height, depth, sizedInternalFormat, type));
if(validationError != GL_NONE)
{
return error(validationError);
@@ -6413,7 +6413,7 @@
if(validationError == GL_NONE)
{
- validationError = context->getPixels(&data, context->getRequiredBufferSize(width, height, depth, sizedInternalFormat, type));
+ validationError = context->getPixels(&data, type, context->getRequiredBufferSize(width, height, depth, sizedInternalFormat, type));
}
if(validationError == GL_NONE)
@@ -6536,7 +6536,7 @@
return error(GL_INVALID_OPERATION);
}
- GLenum validationError = context->getPixels(&data, imageSize);
+ GLenum validationError = context->getPixels(&data, texture->getType(target, level), imageSize);
if(validationError != GL_NONE)
{
@@ -6594,7 +6594,7 @@
return error(GL_INVALID_OPERATION);
}
- GLenum validationError = context->getPixels(&data, imageSize);
+ GLenum validationError = context->getPixels(&data, texture->getType(target, level), imageSize);
if(validationError != GL_NONE)
{
diff --git a/src/OpenGL/libGLESv2/libGLESv3.cpp b/src/OpenGL/libGLESv2/libGLESv3.cpp
index d44647d..ce5872a 100644
--- a/src/OpenGL/libGLESv2/libGLESv3.cpp
+++ b/src/OpenGL/libGLESv2/libGLESv3.cpp
@@ -680,7 +680,7 @@
GLenum sizedInternalFormat = GetSizedInternalFormat(internalformat, type);
- GLenum validationError = context->getPixels(&data, context->getRequiredBufferSize(width, height, depth, sizedInternalFormat, type));
+ GLenum validationError = context->getPixels(&data, type, context->getRequiredBufferSize(width, height, depth, sizedInternalFormat, type));
if(validationError != GL_NONE)
{
return error(validationError);
@@ -732,7 +732,7 @@
GLenum validationError = ValidateSubImageParams(false, width, height, depth, xoffset, yoffset, zoffset, target, level, sizedInternalFormat, texture);
if(validationError == GL_NONE)
{
- GLenum validationError = context->getPixels(&data, context->getRequiredBufferSize(width, height, depth, sizedInternalFormat, type));
+ GLenum validationError = context->getPixels(&data, type, context->getRequiredBufferSize(width, height, depth, sizedInternalFormat, type));
if(validationError != GL_NONE)
{
return error(validationError);
@@ -870,7 +870,7 @@
return error(GL_INVALID_OPERATION);
}
- GLenum validationError = context->getPixels(&data, imageSize);
+ GLenum validationError = context->getPixels(&data, texture->getType(target, level), imageSize);
if(validationError != GL_NONE)
{
return error(validationError);
@@ -928,7 +928,7 @@
return error(GL_INVALID_OPERATION);
}
- GLenum validationError = context->getPixels(&data, imageSize);
+ GLenum validationError = context->getPixels(&data, texture->getType(target, level), imageSize);
if(validationError != GL_NONE)
{
return error(validationError);
diff --git a/src/OpenGL/libGLESv2/utilities.cpp b/src/OpenGL/libGLESv2/utilities.cpp
index 654bbbe..863565f 100644
--- a/src/OpenGL/libGLESv2/utilities.cpp
+++ b/src/OpenGL/libGLESv2/utilities.cpp
@@ -1045,6 +1045,38 @@
return true;
}
+ GLsizei GetTypeSize(GLenum type)
+ {
+ switch(type)
+ {
+ case GL_BYTE:
+ case GL_UNSIGNED_BYTE:
+ return 1;
+ case GL_UNSIGNED_SHORT_4_4_4_4:
+ case GL_UNSIGNED_SHORT_5_5_5_1:
+ case GL_UNSIGNED_SHORT_5_6_5:
+ case GL_HALF_FLOAT_OES:
+ case GL_UNSIGNED_SHORT:
+ case GL_SHORT:
+ case GL_HALF_FLOAT:
+ return 2;
+ case GL_FLOAT:
+ case GL_UNSIGNED_INT_24_8:
+ case GL_UNSIGNED_INT:
+ case GL_INT:
+ case GL_UNSIGNED_INT_2_10_10_10_REV:
+ case GL_UNSIGNED_INT_10F_11F_11F_REV:
+ case GL_UNSIGNED_INT_5_9_9_9_REV:
+ case GL_FLOAT_32_UNSIGNED_INT_24_8_REV:
+ return 4;
+ default:
+ UNREACHABLE(type);
+ break;
+ }
+
+ return 1;
+ }
+
bool IsColorRenderable(GLenum internalformat, GLint clientVersion, bool isTexture)
{
switch(internalformat)
diff --git a/src/OpenGL/libGLESv2/utilities.h b/src/OpenGL/libGLESv2/utilities.h
index 93690c7..815a191 100644
--- a/src/OpenGL/libGLESv2/utilities.h
+++ b/src/OpenGL/libGLESv2/utilities.h
@@ -54,6 +54,7 @@
int CubeFaceIndex(GLenum cubeTarget);
bool IsTextureTarget(GLenum target);
bool ValidateTextureFormatType(GLenum format, GLenum type, GLint internalformat, GLint clientVersion);
+ GLsizei GetTypeSize(GLenum type);
bool IsColorRenderable(GLenum internalformat, GLint clientVersion, bool isTexture);
bool IsDepthRenderable(GLenum internalformat, GLint clientVersion);
