Validate blitFramebuffer for overflows on all platforms

We can validate to avoid triggering driver issues even if the native GLES spec is not telling us to. This will fix WebGL in Chromium when it's being run with the --use-cmd-decoder=validating --use-angle=gl config. BUG=chromium:830046 TEST=WebGL conformance tests Change-Id: I2d61182cb6cbe46b52e1d9b7ed6b4035defee082 Reviewed-on: https://chromium-review.googlesource.com/1033743 Commit-Queue: Olli Etuaho <oetuaho@nvidia.com> Reviewed-by: Corentin Wallez <cwallez@chromium.org>

Validate blitFramebuffer for overflows on all platforms
9aef81c7 · Olli Etuaho · Commit Bot · 47155b17 · 9aef81c7 · 9aef81c7
Commit 9aef81c7 authored Apr 30, 2018 by Olli Etuaho Committed by Commit Bot May 02, 2018
Showing with 61 additions and 61 deletions

validationES.cpp src/libANGLE/validationES.cpp +6 -7

BlitFramebufferANGLETest.cpp src/tests/gl_tests/BlitFramebufferANGLETest.cpp +55 -0

WebGLCompatibilityTest.cpp src/tests/gl_tests/WebGLCompatibilityTest.cpp +0 -54

No files found.
--- a/src/libANGLE/validationES.cpp
+++ b/src/libANGLE/validationES.cpp
@@ -1294,14 +1294,13 @@ bool ValidateBlitFramebufferParameters(Context *context,
        return false;
    }

-    if (context->getExtensions().webglCompatibility)
+    // This validation is specified in the WebGL 2.0 spec and not in the GLES 3.0.5 spec, but we
+    // always run it in order to avoid triggering driver bugs.
+    if (DifferenceCanOverflow(srcX0, srcX1) || DifferenceCanOverflow(srcY0, srcY1) ||
+        DifferenceCanOverflow(dstX0, dstX1) || DifferenceCanOverflow(dstY0, dstY1))
    {
-        if (DifferenceCanOverflow(srcX0, srcX1) || DifferenceCanOverflow(srcY0, srcY1) ||
-            DifferenceCanOverflow(dstX0, dstX1) || DifferenceCanOverflow(dstY0, dstY1))
-        {
-            ANGLE_VALIDATION_ERR(context, InvalidValue(), BlitDimensionsOutOfRange);
-            return false;
-        }
+        ANGLE_VALIDATION_ERR(context, InvalidValue(), BlitDimensionsOutOfRange);
+        return false;
    }

    bool sameBounds = srcX0 == dstX0 && srcY0 == dstY0 && srcX1 == dstX1 && srcY1 == dstY1;

--- a/src/tests/gl_tests/BlitFramebufferANGLETest.cpp
+++ b/src/tests/gl_tests/BlitFramebufferANGLETest.cpp
@@ -1317,6 +1317,61 @@ TEST_P(BlitFramebufferTest, BlitSRGBToRGBOversizedSourceArea)
    EXPECT_PIXEL_COLOR_EQ(kWidth / 4, 3 * kHeight / 4, GLColor::black);
 }

+// Test blitFramebuffer size overflow checks. WebGL 2.0 spec section 5.41. We do validation for
+// overflows also in non-WebGL mode to avoid triggering driver bugs.
+TEST_P(BlitFramebufferTest, BlitFramebufferSizeOverflow)
+{
+    GLTexture textures[2];
+    glBindTexture(GL_TEXTURE_2D, textures[0]);
+    glTexStorage2D(GL_TEXTURE_2D, 3, GL_RGBA8, 4, 4);
+    glBindTexture(GL_TEXTURE_2D, textures[1]);
+    glTexStorage2D(GL_TEXTURE_2D, 3, GL_RGBA8, 4, 4);
+
+    GLFramebuffer framebuffers[2];
+    glBindFramebuffer(GL_READ_FRAMEBUFFER, framebuffers[0]);
+    glBindFramebuffer(GL_DRAW_FRAMEBUFFER, framebuffers[1]);
+
+    ASSERT_GL_NO_ERROR();
+
+    glFramebufferTexture2D(GL_READ_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, textures[0],
+                           0);
+    glFramebufferTexture2D(GL_DRAW_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, textures[1],
+                           0);
+    ASSERT_GL_NO_ERROR();
+
+    // srcX
+    glBlitFramebuffer(-1, 0, std::numeric_limits<GLint>::max(), 4, 0, 0, 4, 4, GL_COLOR_BUFFER_BIT,
+                      GL_NEAREST);
+    EXPECT_GL_ERROR(GL_INVALID_VALUE);
+    glBlitFramebuffer(std::numeric_limits<GLint>::max(), 0, -1, 4, 0, 0, 4, 4, GL_COLOR_BUFFER_BIT,
+                      GL_NEAREST);
+    EXPECT_GL_ERROR(GL_INVALID_VALUE);
+
+    // srcY
+    glBlitFramebuffer(0, -1, 4, std::numeric_limits<GLint>::max(), 0, 0, 4, 4, GL_COLOR_BUFFER_BIT,
+                      GL_NEAREST);
+    EXPECT_GL_ERROR(GL_INVALID_VALUE);
+    glBlitFramebuffer(0, std::numeric_limits<GLint>::max(), 4, -1, 0, 0, 4, 4, GL_COLOR_BUFFER_BIT,
+                      GL_NEAREST);
+    EXPECT_GL_ERROR(GL_INVALID_VALUE);
+
+    // dstX
+    glBlitFramebuffer(0, 0, 4, 4, -1, 0, std::numeric_limits<GLint>::max(), 4, GL_COLOR_BUFFER_BIT,
+                      GL_NEAREST);
+    EXPECT_GL_ERROR(GL_INVALID_VALUE);
+    glBlitFramebuffer(0, 0, 4, 4, std::numeric_limits<GLint>::max(), 0, -1, 4, GL_COLOR_BUFFER_BIT,
+                      GL_NEAREST);
+    EXPECT_GL_ERROR(GL_INVALID_VALUE);
+
+    // dstY
+    glBlitFramebuffer(0, 0, 4, 4, 0, -1, 4, std::numeric_limits<GLint>::max(), GL_COLOR_BUFFER_BIT,
+                      GL_NEAREST);
+    EXPECT_GL_ERROR(GL_INVALID_VALUE);
+    glBlitFramebuffer(0, 0, 4, 4, 0, std::numeric_limits<GLint>::max(), 4, -1, GL_COLOR_BUFFER_BIT,
+                      GL_NEAREST);
+    EXPECT_GL_ERROR(GL_INVALID_VALUE);
+}
+
 // Use this to select which configurations (e.g. which renderer, which GLES major version) these tests should be run against.
 ANGLE_INSTANTIATE_TEST(BlitFramebufferANGLETest,
                       ES2_D3D9(),

--- a/src/tests/gl_tests/WebGLCompatibilityTest.cpp
+++ b/src/tests/gl_tests/WebGLCompatibilityTest.cpp
@@ -4152,60 +4152,6 @@ TEST_P(WebGL2CompatibilityTest, TransformFeedbackCheckNullDeref)
    EXPECT_GL_ERROR(GL_INVALID_OPERATION);
 }

-// Test blitFramebuffer size overflow checks. WebGL 2.0 spec section 5.41.
-TEST_P(WebGL2CompatibilityTest, BlitFramebufferSizeOverflow)
-{
-    GLTexture textures[2];
-    glBindTexture(GL_TEXTURE_2D, textures[0]);
-    glTexStorage2D(GL_TEXTURE_2D, 3, GL_RGBA8, 4, 4);
-    glBindTexture(GL_TEXTURE_2D, textures[1]);
-    glTexStorage2D(GL_TEXTURE_2D, 3, GL_RGBA8, 4, 4);
-
-    GLFramebuffer framebuffers[2];
-    glBindFramebuffer(GL_READ_FRAMEBUFFER, framebuffers[0]);
-    glBindFramebuffer(GL_DRAW_FRAMEBUFFER, framebuffers[1]);
-
-    ASSERT_GL_NO_ERROR();
-
-    glFramebufferTexture2D(GL_READ_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, textures[0],
-                           0);
-    glFramebufferTexture2D(GL_DRAW_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, textures[1],
-                           0);
-    ASSERT_GL_NO_ERROR();
-
-    // srcX
-    glBlitFramebuffer(-1, 0, std::numeric_limits<GLint>::max(), 4, 0, 0, 4, 4, GL_COLOR_BUFFER_BIT,
-                      GL_NEAREST);
-    EXPECT_GL_ERROR(GL_INVALID_VALUE);
-    glBlitFramebuffer(std::numeric_limits<GLint>::max(), 0, -1, 4, 0, 0, 4, 4, GL_COLOR_BUFFER_BIT,
-                      GL_NEAREST);
-    EXPECT_GL_ERROR(GL_INVALID_VALUE);
-
-    // srcY
-    glBlitFramebuffer(0, -1, 4, std::numeric_limits<GLint>::max(), 0, 0, 4, 4, GL_COLOR_BUFFER_BIT,
-                      GL_NEAREST);
-    EXPECT_GL_ERROR(GL_INVALID_VALUE);
-    glBlitFramebuffer(0, std::numeric_limits<GLint>::max(), 4, -1, 0, 0, 4, 4, GL_COLOR_BUFFER_BIT,
-                      GL_NEAREST);
-    EXPECT_GL_ERROR(GL_INVALID_VALUE);
-
-    // dstX
-    glBlitFramebuffer(0, 0, 4, 4, -1, 0, std::numeric_limits<GLint>::max(), 4, GL_COLOR_BUFFER_BIT,
-                      GL_NEAREST);
-    EXPECT_GL_ERROR(GL_INVALID_VALUE);
-    glBlitFramebuffer(0, 0, 4, 4, std::numeric_limits<GLint>::max(), 0, -1, 4, GL_COLOR_BUFFER_BIT,
-                      GL_NEAREST);
-    EXPECT_GL_ERROR(GL_INVALID_VALUE);
-
-    // dstY
-    glBlitFramebuffer(0, 0, 4, 4, 0, -1, 4, std::numeric_limits<GLint>::max(), GL_COLOR_BUFFER_BIT,
-                      GL_NEAREST);
-    EXPECT_GL_ERROR(GL_INVALID_VALUE);
-    glBlitFramebuffer(0, 0, 4, 4, 0, std::numeric_limits<GLint>::max(), 4, -1, GL_COLOR_BUFFER_BIT,
-                      GL_NEAREST);
-    EXPECT_GL_ERROR(GL_INVALID_VALUE);
-}
-
 // Use this to select which configurations (e.g. which renderer, which GLES major version) these
 // tests should be run against.
 ANGLE_INSTANTIATE_TEST(WebGLCompatibilityTest,