Fix crash when viewport Y position is negative
More specifically, crash occurs when viewport.Y is negative, and
scissoring isn't explicitly set by user. Problem was a combination
of computing invalid scissor bounds, and after clamping to these
bounds, proceeding to index buffers when yMin >= yMax. Fixed
both computation of scissor bounds, and making sure to early out
when yMin >= yMax to avoid OOB buffer indexing.
Bug: chromium:904276
Change-Id: I14580d7c4f0d9888c19e037b47113624a247ede1
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/29971
Tested-by: Antonio Maiorano <amaiorano@google.com>
Presubmit-Ready: Antonio Maiorano <amaiorano@google.com>
Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>
Reviewed-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Alexis Hétu <sugoi@google.com>
diff --git a/src/OpenGL/libGLESv2/Device.cpp b/src/OpenGL/libGLESv2/Device.cpp
index a98131e..c94165b 100644
--- a/src/OpenGL/libGLESv2/Device.cpp
+++ b/src/OpenGL/libGLESv2/Device.cpp
@@ -817,6 +817,12 @@
scissor.y1 = min(scissor.y1, stencilBuffer->getHeight());
}
+ // Ensure scissor range is positive
+ scissor.x0 = max(scissor.x0, 0);
+ scissor.x1 = max(scissor.x1, 0);
+ scissor.y0 = max(scissor.y0, 0);
+ scissor.y1 = max(scissor.y1, 0);
+
setScissor(scissor);
}
diff --git a/src/Pipeline/SetupRoutine.cpp b/src/Pipeline/SetupRoutine.cpp
index 5e9b714..692cfc45 100644
--- a/src/Pipeline/SetupRoutine.cpp
+++ b/src/Pipeline/SetupRoutine.cpp
@@ -175,14 +175,17 @@
yMax = (yMax + 0x0F) >> 4;
}
- If(yMin == yMax)
+ yMin = Max(yMin, *Pointer<Int>(data + OFFSET(DrawData,scissorY0)));
+ yMax = Min(yMax, *Pointer<Int>(data + OFFSET(DrawData,scissorY1)));
+
+ // If yMin and yMax are initially negative, the scissor clamping above will typically result
+ // in yMin == 0 and yMax unchanged. We bail as we don't need to rasterize this primitive, and
+ // code below assumes yMin < yMax.
+ If(yMin >= yMax)
{
Return(false);
}
- yMin = Max(yMin, *Pointer<Int>(data + OFFSET(DrawData,scissorY0)));
- yMax = Min(yMax, *Pointer<Int>(data + OFFSET(DrawData,scissorY1)));
-
For(Int q = 0, q < state.multiSample, q++)
{
Array<Int> Xq(16);
diff --git a/src/Shader/SetupRoutine.cpp b/src/Shader/SetupRoutine.cpp
index e5bd7fd..7ecfa3d 100644
--- a/src/Shader/SetupRoutine.cpp
+++ b/src/Shader/SetupRoutine.cpp
@@ -189,14 +189,17 @@
yMax = (yMax + 0x0F) >> 4;
}
- If(yMin == yMax)
+ yMin = Max(yMin, *Pointer<Int>(data + OFFSET(DrawData,scissorY0)));
+ yMax = Min(yMax, *Pointer<Int>(data + OFFSET(DrawData,scissorY1)));
+
+ // If yMin and yMax are initially negative, the scissor clamping above will typically result
+ // in yMin == 0 and yMax unchanged. We bail as we don't need to rasterize this primitive, and
+ // code below assumes yMin < yMax.
+ If(yMin >= yMax)
{
Return(false);
}
- yMin = Max(yMin, *Pointer<Int>(data + OFFSET(DrawData,scissorY0)));
- yMax = Min(yMax, *Pointer<Int>(data + OFFSET(DrawData,scissorY1)));
-
For(Int q = 0, q < state.multiSample, q++)
{
Array<Int> Xq(16);
diff --git a/tests/GLESUnitTests/unittests.cpp b/tests/GLESUnitTests/unittests.cpp
index a9c4f31..0f6298b 100644
--- a/tests/GLESUnitTests/unittests.cpp
+++ b/tests/GLESUnitTests/unittests.cpp
@@ -1174,6 +1174,58 @@
}
}
+TEST_F(SwiftShaderTest, ViewportBounds)
+{
+ auto doRenderWithViewportSettings = [&](GLint x, GLint y, GLsizei w, GLsizei h)
+ {
+ Initialize(3, false);
+
+ std::string vs =
+ "#version 300 es\n"
+ "in vec4 position;\n"
+ "out float unfoldable;\n"
+ "void main()\n"
+ "{\n"
+ " unfoldable = position.x;\n"
+ " gl_Position = vec4(position.xy, 0.0, 1.0);\n"
+ "}\n";
+
+ std::string fs =
+ "#version 300 es\n"
+ "precision mediump float;\n"
+ "in float unfoldable;\n"
+ "out vec4 fragColor;\n"
+ "void main()\n"
+ "{\n"
+ " fragColor = vec4(1.0, 1.0, 1.0, 1.0);\n"
+ "}\n";
+
+ const ProgramHandles ph = createProgram(vs, fs);
+
+ glUseProgram(ph.program);
+
+ glViewport(x, y, w, h);
+
+ drawQuad(ph.program);
+ EXPECT_GLENUM_EQ(GL_NONE, glGetError());
+
+ deleteProgram(ph);
+ Uninitialize();
+ };
+
+ GLsizei w = 100;
+ GLsizei h = 100;
+ GLint minPos = -2000;
+
+ doRenderWithViewportSettings(0, 0, 0, 0);
+ doRenderWithViewportSettings(0, 0, w, h);
+
+ // Negative positions
+ doRenderWithViewportSettings(minPos, 0, w, h);
+ doRenderWithViewportSettings(0, minPos, w, h);
+ doRenderWithViewportSettings(minPos, minPos, w, h);
+}
+
// Test using TexImage2D to define a rectangle texture
TEST_F(SwiftShaderTest, TextureRectangle_TexImage2D)
