Clamp ReadPixels calls to unsigned limits in D3D11.
GL allows reads beyond the defined pixel rectangle, although their
resultant values are undefined. D3D11 gives an error when reading
out-of-bounds, hence clamping the area satisfies both APIs.
This fixes a Renderer crash in our ReadPixels out of bounds test,
and also fixes the test to test the correct area of pixels.
BUG=angle:590
Change-Id: I12439c22d53ec6a2d69e1b8cf80f53e9c18e11f7
Reviewed-on: https://chromium-review.googlesource.com/191080
Reviewed-by: Shannon Woods <shannonwoods@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Tested-by: Jamie Madill <jmadill@chromium.org>
diff --git a/src/libGLESv2/renderer/d3d11/Renderer11.cpp b/src/libGLESv2/renderer/d3d11/Renderer11.cpp
index 3ba36d0..ca97c04 100644
--- a/src/libGLESv2/renderer/d3d11/Renderer11.cpp
+++ b/src/libGLESv2/renderer/d3d11/Renderer11.cpp
@@ -3319,12 +3319,35 @@
GLenum format, GLenum type, GLsizei outputPitch, bool packReverseRowOrder,
GLint packAlignment, void *pixels)
{
+ ASSERT(area.width >= 0);
+ ASSERT(area.height >= 0);
+
D3D11_TEXTURE2D_DESC textureDesc;
texture->GetDesc(&textureDesc);
+ // Clamp read region to the defined texture boundaries, preventing out of bounds reads
+ // and reads of uninitialized data.
+ gl::Rectangle safeArea;
+ safeArea.x = gl::clamp(area.x, 0, static_cast<int>(textureDesc.Width));
+ safeArea.y = gl::clamp(area.y, 0, static_cast<int>(textureDesc.Height));
+ safeArea.width = gl::clamp(area.width + std::min(area.x, 0), 0,
+ static_cast<int>(textureDesc.Width) - safeArea.x);
+ safeArea.height = gl::clamp(area.height + std::min(area.y, 0), 0,
+ static_cast<int>(textureDesc.Height) - safeArea.y);
+
+ ASSERT(safeArea.x >= 0 && safeArea.y >= 0);
+ ASSERT(safeArea.x + safeArea.width <= static_cast<int>(textureDesc.Width));
+ ASSERT(safeArea.y + safeArea.height <= static_cast<int>(textureDesc.Height));
+
+ if (safeArea.width == 0 || safeArea.height == 0)
+ {
+ // no work to do
+ return;
+ }
+
D3D11_TEXTURE2D_DESC stagingDesc;
- stagingDesc.Width = area.width;
- stagingDesc.Height = area.height;
+ stagingDesc.Width = safeArea.width;
+ stagingDesc.Height = safeArea.height;
stagingDesc.MipLevels = 1;
stagingDesc.ArraySize = 1;
stagingDesc.Format = textureDesc.Format;
@@ -3377,12 +3400,12 @@
}
D3D11_BOX srcBox;
- srcBox.left = area.x;
- srcBox.right = area.x + area.width;
- srcBox.top = area.y;
- srcBox.bottom = area.y + area.height;
- srcBox.front = 0;
- srcBox.back = 1;
+ srcBox.left = static_cast<UINT>(safeArea.x);
+ srcBox.right = static_cast<UINT>(safeArea.x + safeArea.width);
+ srcBox.top = static_cast<UINT>(safeArea.y);
+ srcBox.bottom = static_cast<UINT>(safeArea.y + safeArea.height);
+ srcBox.front = 0;
+ srcBox.back = 1;
mDeviceContext->CopySubresourceRegion(stagingTex, 0, 0, 0, 0, srcTex, subResource, &srcBox);
@@ -3395,7 +3418,7 @@
int inputPitch;
if (packReverseRowOrder)
{
- source = static_cast<unsigned char*>(mapping.pData) + mapping.RowPitch * (area.height - 1);
+ source = static_cast<unsigned char*>(mapping.pData) + mapping.RowPitch * (safeArea.height - 1);
inputPitch = -static_cast<int>(mapping.RowPitch);
}
else
@@ -3416,9 +3439,9 @@
{
// Direct copy possible
unsigned char *dest = static_cast<unsigned char*>(pixels);
- for (int y = 0; y < area.height; y++)
+ for (int y = 0; y < safeArea.height; y++)
{
- memcpy(dest + y * outputPitch, source + y * inputPitch, area.width * sourcePixelSize);
+ memcpy(dest + y * outputPitch, source + y * inputPitch, safeArea.width * sourcePixelSize);
}
}
else
@@ -3430,9 +3453,9 @@
if (fastCopyFunc)
{
// Fast copy is possible through some special function
- for (int y = 0; y < area.height; y++)
+ for (int y = 0; y < safeArea.height; y++)
{
- for (int x = 0; x < area.width; x++)
+ for (int x = 0; x < safeArea.width; x++)
{
void *dest = static_cast<unsigned char*>(pixels) + y * outputPitch + x * destPixelSize;
void *src = static_cast<unsigned char*>(source) + y * inputPitch + x * sourcePixelSize;
@@ -3451,9 +3474,9 @@
sizeof(temp) >= sizeof(gl::ColorUI) &&
sizeof(temp) >= sizeof(gl::ColorI));
- for (int y = 0; y < area.height; y++)
+ for (int y = 0; y < safeArea.height; y++)
{
- for (int x = 0; x < area.width; x++)
+ for (int x = 0; x < safeArea.width; x++)
{
void *dest = static_cast<unsigned char*>(pixels) + y * outputPitch + x * destPixelSize;
void *src = static_cast<unsigned char*>(source) + y * inputPitch + x * sourcePixelSize;
diff --git a/tests/angle_tests/ReadPixelsTest.cpp b/tests/angle_tests/ReadPixelsTest.cpp
index 93af5ee..d90122f 100644
--- a/tests/angle_tests/ReadPixelsTest.cpp
+++ b/tests/angle_tests/ReadPixelsTest.cpp
@@ -22,22 +22,27 @@
GLsizei pixelsWidth = 32;
GLsizei pixelsHeight = 32;
- std::vector<GLubyte> pixels(pixelsWidth * pixelsHeight * 4);
+ GLint offset = 16;
+ std::vector<GLubyte> pixels((pixelsWidth + offset) * (pixelsHeight + offset) * 4);
- glReadPixels(-pixelsWidth / 2, -pixelsHeight / 2, pixelsWidth, pixelsHeight, GL_RGBA, GL_UNSIGNED_BYTE, pixels.data());
+ glReadPixels(-offset, -offset, pixelsWidth + offset, pixelsHeight + offset, GL_RGBA, GL_UNSIGNED_BYTE, pixels.data());
EXPECT_GL_NO_ERROR();
for (int y = pixelsHeight / 2; y < pixelsHeight; y++)
{
for (int x = pixelsWidth / 2; x < pixelsWidth; x++)
{
- const GLubyte* pixel = pixels.data() + ((y * pixelsWidth + x) * 4);
+ const GLubyte* pixel = pixels.data() + ((y * (pixelsWidth + offset) + x) * 4);
+ unsigned int r = static_cast<unsigned int>(pixel[0]);
+ unsigned int g = static_cast<unsigned int>(pixel[1]);
+ unsigned int b = static_cast<unsigned int>(pixel[2]);
+ unsigned int a = static_cast<unsigned int>(pixel[3]);
// Expect that all pixels which fell within the framebuffer are red
- EXPECT_EQ(pixel[0], 255);
- EXPECT_EQ(pixel[1], 0);
- EXPECT_EQ(pixel[2], 0);
- EXPECT_EQ(pixel[3], 255);
+ EXPECT_EQ(255, r);
+ EXPECT_EQ(0, g);
+ EXPECT_EQ(0, b);
+ EXPECT_EQ(255, a);
}
}
}