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 (file)
--- a/src/OpenGL/libGLESv2/Context.cpp
+++ b/src/OpenGL/libGLESv2/Context.cpp
@@ -1585,11 +1585,10 @@ GLsizei Context::getRequiredBufferSize(GLsizei width, GLsizei height, GLsizei de
        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 @@ GLenum Context::getPixels(const GLvoid **data, GLsizei imageSize) const
                {
                        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 (file)
--- a/src/OpenGL/libGLESv2/Context.h
+++ b/src/OpenGL/libGLESv2/Context.h
@@ -647,7 +647,7 @@ public:
        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 (file)
--- a/src/OpenGL/libGLESv2/libGLESv2.cpp
+++ b/src/OpenGL/libGLESv2/libGLESv2.cpp
@@ -876,7 +876,7 @@ void CompressedTexImage2D(GLenum target, GLint level, GLenum internalformat, GLs
                                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 @@ void CompressedTexImage2D(GLenum target, GLint level, GLenum internalformat, GLs
                        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 @@ void CompressedTexSubImage2D(GLenum target, GLint level, GLint xoffset, GLint yo
 
                        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 @@ void CompressedTexSubImage2D(GLenum target, GLint level, GLint xoffset, GLint yo
 
                        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 @@ void TexImage2D(GLenum target, GLint level, GLint internalformat, GLsizei width,
 
                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 @@ void TexSubImage2D(GLenum target, GLint level, GLint xoffset, GLint yoffset, GLs
        {
                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 @@ void TexImage3DOES(GLenum target, GLint level, GLenum internalformat, GLsizei wi
                }
 
                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 @@ void TexSubImage3DOES(GLenum target, GLint level, GLint xoffset, GLint yoffset,
 
                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 @@ void CompressedTexImage3DOES(GLenum target, GLint level, GLenum internalformat,
                        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 @@ void CompressedTexSubImage3DOES(GLenum target, GLint level, GLint xoffset, GLint
                        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 (file)
--- a/src/OpenGL/libGLESv2/libGLESv3.cpp
+++ b/src/OpenGL/libGLESv2/libGLESv3.cpp
@@ -680,7 +680,7 @@ GL_APICALL void GL_APIENTRY glTexImage3D(GLenum target, GLint level, GLint inter
 
                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 @@ GL_APICALL void GL_APIENTRY glTexSubImage3D(GLenum target, GLint level, GLint xo
                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 @@ GL_APICALL void GL_APIENTRY glCompressedTexImage3D(GLenum target, GLint level, G
                        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 @@ GL_APICALL void GL_APIENTRY glCompressedTexSubImage3D(GLenum target, GLint level
                        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 (file)
--- a/src/OpenGL/libGLESv2/utilities.cpp
+++ b/src/OpenGL/libGLESv2/utilities.cpp
@@ -1045,6 +1045,38 @@ namespace es2
                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 (file)
--- a/src/OpenGL/libGLESv2/utilities.h
+++ b/src/OpenGL/libGLESv2/utilities.h
@@ -54,6 +54,7 @@ namespace es2
        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);