Fix how we calculate potential overflows with PBOs

src/OpenGL/common/Image.cpp

@@ -436,7 +436,7 @@ namespace gl
 
 	// Returns the size, in bytes, of a single client-side pixel.
     // OpenGL ES 3.0.5 table 3.2.
-	static int ComputePixelSize(GLenum format, GLenum type)
+	GLsizei ComputePixelSize(GLenum format, GLenum type)
 	{
 		switch(format)
 		{

src/OpenGL/common/Image.hpp

@@ -58,6 +58,7 @@ bool IsUnsizedInternalFormat(GLint internalformat);
 GLenum GetBaseInternalFormat(GLint internalformat);
 GLsizei ComputePitch(GLsizei width, GLenum format, GLenum type, GLint alignment);
 GLsizei ComputeCompressedSize(GLsizei width, GLsizei height, GLenum format);
+GLsizei ComputePixelSize(GLenum format, GLenum type);
 size_t ComputePackingOffset(GLenum format, GLenum type, GLsizei width, GLsizei height, const PixelStorageModes &storageModes);
 
 }

src/OpenGL/libGLESv2/Context.cpp

@@ -1567,13 +1567,31 @@ Buffer *Context::getGenericUniformBuffer() const
 	return mState.genericUniformBuffer;
 }
 
+// The "required buffer size" is the number of bytes from the start of the
+// buffer to the last byte referenced within the buffer. If the caller of this
+// function has to worry about offsets within the buffer, it only needs to add
+// that byte offset to this function's return value to get its required buffer
+// size.
 size_t Context::getRequiredBufferSize(GLsizei width, GLsizei height, GLsizei depth, GLenum format, GLenum type) const
 {
-	GLsizei inputWidth = (mState.unpackParameters.rowLength == 0) ? width : mState.unpackParameters.rowLength;
-	GLsizei inputPitch = gl::ComputePitch(inputWidth, format, type, mState.unpackParameters.alignment);
-	GLsizei inputHeight = (mState.unpackParameters.imageHeight == 0) ? height : mState.unpackParameters.imageHeight;
+	// 0-dimensional images have no bytes in them.
+	if (width == 0 || height == 0 || depth == 0)
+	{
+		return 0;
+	}
+
+	GLint pixelsPerRow = (mState.unpackParameters.rowLength) > 0 ? mState.unpackParameters.rowLength : width;
+	GLint rowsPerImage = (mState.unpackParameters.imageHeight) > 0 ? mState.unpackParameters.imageHeight : height;
+
+	GLint bytesPerPixel = gl::ComputePixelSize(format, type);
+	GLint bytesPerRow = gl::ComputePitch(pixelsPerRow, format, type, mState.unpackParameters.alignment);
+	GLint bytesPerImage = rowsPerImage * bytesPerRow;
 
-	return static_cast<size_t>(inputPitch) * inputHeight * depth;
+	// Depth and height are subtracted by 1, while width is not, because we're not
+	// reading the full last row or image, but we are reading the full last pixel.
+	return (mState.unpackParameters.skipImages + (depth - 1))  * bytesPerImage
+		 + (mState.unpackParameters.skipRows   + (height - 1)) * bytesPerRow
+		 + (mState.unpackParameters.skipPixels + (width))      * bytesPerPixel;
 }
 
 GLenum Context::getPixels(const GLvoid **pixels, GLenum type, size_t imageSize) const

tests/GLESUnitTests/unittests.cpp

@@ -952,6 +952,84 @@ TEST_F(SwiftShaderTest, CopyTexImage)
 	Uninitialize();
 }
 
+// Tests copying to a texture from a pixel buffer object
+TEST_F(SwiftShaderTest, CopyTexImageFromPixelBuffer)
+{
+	Initialize(3, false);
+	const GLuint red   = 0xff0000ff;
+	const GLuint green = 0x00ff00ff;
+	const GLuint blue  = 0x0000ffff;
+	// Set up texture
+	GLuint texture = 0;
+	glGenTextures(1, &texture);
+	EXPECT_NO_GL_ERROR();
+	GLuint tex_data[4][4] = {
+		{red, red, red, red},
+		{red, red, red, red},
+		{red, red, red, red},
+		{red, red, red, red}
+	};
+	glBindTexture(GL_TEXTURE_2D, texture);
+	EXPECT_NO_GL_ERROR();
+	glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 4, 4, 0, GL_RGBA, GL_UNSIGNED_BYTE, (void *) tex_data[0]);
+	EXPECT_NO_GL_ERROR();
+	// Set up Pixel Buffer Object
+	GLuint pixelBuffer = 0;
+	glGenBuffers(1, &pixelBuffer);
+	glBindBuffer(GL_PIXEL_UNPACK_BUFFER, pixelBuffer);
+	EXPECT_NO_GL_ERROR();
+	glPixelStorei(GL_UNPACK_ROW_LENGTH, 4);
+	glPixelStorei(GL_UNPACK_ALIGNMENT, 1);
+	EXPECT_NO_GL_ERROR();
+	GLuint pixel_data[4][4] = {
+		{blue, blue, green, green},
+		{blue, blue, green, green},
+		{blue, blue, green, green},
+		{blue, blue, green, green},
+	};
+	glBufferData(GL_PIXEL_UNPACK_BUFFER, sizeof(pixel_data), (void *) pixel_data, GL_STREAM_DRAW);
+	// Should set the 2-rightmost columns of the currently bound texture to the
+	// 2-rightmost columns of the PBO;
+	GLintptr offset = 2 * sizeof(GLuint);
+	glTexSubImage2D(GL_TEXTURE_2D, 0, 2, 0, 2, 4, GL_RGBA, GL_UNSIGNED_BYTE, reinterpret_cast<void *>(offset));
+	EXPECT_NO_GL_ERROR();
+	// Create an off-screen framebuffer to render the texture data to.
+	GLuint fbo = 0;
+	glGenFramebuffers(1, &fbo);
+	glBindFramebuffer(GL_FRAMEBUFFER, fbo);
+	glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, texture, 0);
+	EXPECT_NO_GL_ERROR();
+	unsigned int color[4][4] = {
+		{0, 0, 0, 0},
+		{0, 0, 0, 0},
+		{0, 0, 0, 0},
+		{0, 0, 0, 0}
+	};
+	glReadPixels(0, 0, 4, 4, GL_RGBA, GL_UNSIGNED_BYTE, &color);
+	EXPECT_NO_GL_ERROR();
+	bool allEqual = true;
+	for (int i = 0; i < 4; i++)
+	{
+		for (int j = 0; j < 2; j++)
+		{
+			allEqual = allEqual && (color[i][j] == tex_data[i][j]);
+			allEqual = allEqual && (color[i][j+2] == pixel_data[i][j+2]);
+			if (!allEqual)
+				break;
+		}
+		if (!allEqual)
+			break;
+	}
+	EXPECT_EQ(allEqual, true);
+	// We can't use an offset of 3 GLuints or more, because the PBO is not large
+	// enough to satisfy such a request with the current GL_UNPACK_ROW_LENGTH.
+	offset = 3 * sizeof(GLuint);
+	glTexSubImage2D(GL_TEXTURE_2D, 0, 2, 0, 2, 4, GL_RGBA, GL_UNSIGNED_BYTE, reinterpret_cast<void *>(offset));
+	GLenum error = glGetError();
+	EXPECT_GLENUM_EQ(GL_INVALID_OPERATION, error);
+	Uninitialize();
+}
+
 // Tests reading of half-float textures.
 TEST_F(SwiftShaderTest, ReadHalfFloat)
 {