Accurately clip copyTexImage2D and copyTexSubImage2D
authordino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 7 Jan 2018 05:18:47 +0000 (05:18 +0000)
committerdino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 7 Jan 2018 05:18:47 +0000 (05:18 +0000)
https://bugs.webkit.org/show_bug.cgi?id=181356
<rdar://problem/35083877>

Reviewed by Eric Carlson.

Source/WebCore:

The code to make sure copyTexSubImage2D and copyTexImage2D will not try to read
out of bounds had a bad bug introduced here:
https://bugs.webkit.org/show_bug.cgi?id=51421

With appropriate parameters, it would produce a rectangle with
negative dimensions. Most GL drivers just ignored this, but some
are not happy.

Test: fast/canvas/webgl/copy-tex-image-and-sub-image-2d-bad-input.html

* html/canvas/WebGLRenderingContextBase.cpp:
(WebCore::clip2D): Reimplement this in a more sane manner, and use
checked arithmetic while here.
* html/canvas/WebGLRenderingContextBase.h:
(WebCore::clip1D): Deleted.
(WebCore::clip2D): Deleted.

LayoutTests:

* fast/canvas/webgl/copy-tex-image-and-sub-image-2d-bad-input-expected.txt: Added.
* fast/canvas/webgl/copy-tex-image-and-sub-image-2d-bad-input.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@226490 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/canvas/webgl/copy-tex-image-and-sub-image-2d-bad-input-expected.txt [new file with mode: 0644]
LayoutTests/fast/canvas/webgl/copy-tex-image-and-sub-image-2d-bad-input.html [new file with mode: 0644]
LayoutTests/platform/ios-simulator/fast/canvas/webgl/copy-tex-image-and-sub-image-2d-bad-input-expected.txt [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp
Source/WebCore/html/canvas/WebGLRenderingContextBase.h

index b1ce252..1e6f6ce 100644 (file)
@@ -1,3 +1,14 @@
+2018-01-05  Dean Jackson  <dino@apple.com>
+
+        Accurately clip copyTexImage2D and copyTexSubImage2D
+        https://bugs.webkit.org/show_bug.cgi?id=181356
+        <rdar://problem/35083877>
+
+        Reviewed by Eric Carlson.
+
+        * fast/canvas/webgl/copy-tex-image-and-sub-image-2d-bad-input-expected.txt: Added.
+        * fast/canvas/webgl/copy-tex-image-and-sub-image-2d-bad-input.html: Added.
+
 2018-01-06  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         Object.getOwnPropertyNames includes "arguments" and "caller" for bound functions
diff --git a/LayoutTests/fast/canvas/webgl/copy-tex-image-and-sub-image-2d-bad-input-expected.txt b/LayoutTests/fast/canvas/webgl/copy-tex-image-and-sub-image-2d-bad-input-expected.txt
new file mode 100644 (file)
index 0000000..22a8599
--- /dev/null
@@ -0,0 +1,43 @@
+CONSOLE MESSAGE: line 113: WebGL: drawArrays: texture bound to texture unit 0 is not renderable. It maybe non-power-of-2 and have incompatible texture filtering or is not 'texture complete', or it is a float/half-float type with linear filtering and without the relevant float/half-float linear extension enabled.
+CONSOLE MESSAGE: line 145: WebGL: drawArrays: texture bound to texture unit 0 is not renderable. It maybe non-power-of-2 and have incompatible texture filtering or is not 'texture complete', or it is a float/half-float type with linear filtering and without the relevant float/half-float linear extension enabled.
+CONSOLE MESSAGE: line 113: WebGL: drawArrays: texture bound to texture unit 0 is not renderable. It maybe non-power-of-2 and have incompatible texture filtering or is not 'texture complete', or it is a float/half-float type with linear filtering and without the relevant float/half-float linear extension enabled.
+CONSOLE MESSAGE: line 145: WebGL: drawArrays: texture bound to texture unit 0 is not renderable. It maybe non-power-of-2 and have incompatible texture filtering or is not 'texture complete', or it is a float/half-float type with linear filtering and without the relevant float/half-float linear extension enabled.
+Verify copyTexImage2D and copyTexSubImage2D can handle bad input
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+Testing with antialias on
+Testing copyTexImage2D
+PASS getError was expected value: NO_ERROR : 
+PASS getError was expected value: NO_ERROR : 
+PASS pixel is correctColor
+PASS pixel is correctColor
+PASS pixel is correctColor
+PASS pixel is correctColor
+Testing copyTexSubImage2D
+PASS getError was expected value: NO_ERROR : 
+PASS getError was expected value: NO_ERROR : 
+PASS pixel is correctColor
+PASS pixel is correctColor
+PASS pixel is correctColor
+PASS pixel is correctColor
+Testing with antialias off
+Testing copyTexImage2D
+PASS getError was expected value: NO_ERROR : 
+PASS getError was expected value: NO_ERROR : 
+PASS pixel is correctColor
+PASS pixel is correctColor
+PASS pixel is correctColor
+PASS pixel is correctColor
+Testing copyTexSubImage2D
+PASS getError was expected value: NO_ERROR : 
+PASS getError was expected value: NO_ERROR : 
+PASS pixel is correctColor
+PASS pixel is correctColor
+PASS pixel is correctColor
+PASS pixel is correctColor
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/canvas/webgl/copy-tex-image-and-sub-image-2d-bad-input.html b/LayoutTests/fast/canvas/webgl/copy-tex-image-and-sub-image-2d-bad-input.html
new file mode 100644 (file)
index 0000000..a1348ae
--- /dev/null
@@ -0,0 +1,186 @@
+<html>
+<head>
+<script src="../../../resources/js-test.js"></script>
+<script src="resources/webgl-test.js"></script>
+<script id="vshader" type="x-shader/x-vertex">
+attribute vec3 g_Position;
+attribute vec2 g_TexCoord0;
+
+varying vec2 texCoord;
+
+void main()
+{
+    gl_Position = vec4(g_Position.x, g_Position.y, g_Position.z, 1.0);
+    texCoord = g_TexCoord0;
+}
+</script>
+
+<script id="fshader" type="x-shader/x-fragment">
+#ifdef GL_ES
+precision mediump float;
+#endif
+uniform sampler2D tex;
+varying vec2 texCoord;
+void main()
+{
+    gl_FragColor = texture2D(tex, texCoord);
+}
+</script>
+
+<script>
+
+function init()
+{
+    if (window.initNonKhronosFramework) {
+        window.initNonKhronosFramework(true);
+    }
+
+    description('Verify copyTexImage2D and copyTexSubImage2D can handle bad input');
+
+    runTest();
+}
+
+// These declarations need to be global for "shouldBe" to see them
+var pixel = [0, 0, 0];
+var correctColor = null;
+var gl = null;
+
+function runTestIteration(antialias)
+{
+    if (antialias)
+        gl = initWebGL("antialiasOn", "vshader", "fshader", [ "g_Position", "g_TexCoord0" ], [ 0, 0, 0, 1 ], 1);
+    else
+        gl = initWebGL("antialiasOff", "vshader", "fshader", [ "g_Position", "g_TexCoord0" ], [ 0, 0, 0, 1 ], 1, { antialias: false });
+
+    var textureLoc = gl.getUniformLocation(gl.program, "tex");
+
+    var vertices = new Float32Array([
+         1.0,  1.0, 0.0,
+        -1.0,  1.0, 0.0,
+        -1.0, -1.0, 0.0,
+         1.0,  1.0, 0.0,
+        -1.0, -1.0, 0.0,
+         1.0, -1.0, 0.0]);
+    var texCoords = new Float32Array([
+        1.0, 1.0,
+        0.0, 1.0,
+        0.0, 0.0,
+        1.0, 1.0,
+        0.0, 0.0,
+        1.0, 0.0]);
+    var texCoordOffset = vertices.byteLength;
+
+    var vbo = gl.createBuffer();
+    gl.bindBuffer(gl.ARRAY_BUFFER, vbo);
+    gl.bufferData(gl.ARRAY_BUFFER,
+                  texCoordOffset + texCoords.byteLength,
+                  gl.STATIC_DRAW);
+    gl.bufferSubData(gl.ARRAY_BUFFER, 0, vertices);
+    gl.bufferSubData(gl.ARRAY_BUFFER, texCoordOffset, texCoords);
+
+    gl.enableVertexAttribArray(0);
+    gl.vertexAttribPointer(0, 3, gl.FLOAT, false, 0, 0);
+    gl.enableVertexAttribArray(1);
+    gl.vertexAttribPointer(1, 2, gl.FLOAT, false, 0, texCoordOffset);
+
+    gl.colorMask(1, 1, 1, 0);
+    gl.disable(gl.BLEND);
+    debug('Testing copyTexImage2D');
+
+    // Red canvas
+    gl.clearColor(1, 0, 0, 1);
+    gl.clear(gl.COLOR_BUFFER_BIT | gl.DEPTH_BUFFER_BIT);
+
+    var texture = gl.createTexture();
+    // Bind the texture to texture unit 0
+    gl.bindTexture(gl.TEXTURE_2D, texture);
+    // Set up texture
+    gl.texImage2D(gl.TEXTURE_2D, 0, gl.RGBA, 2, 2, 0, gl.RGBA, gl.UNSIGNED_BYTE, null);
+    gl.texParameteri(gl.TEXTURE_2D, gl.TEXTURE_MIN_FILTER, gl.NEAREST);
+    gl.texParameteri(gl.TEXTURE_2D, gl.TEXTURE_MAG_FILTER, gl.NEAREST);
+
+    glErrorShouldBe(gl, gl.NO_ERROR);
+    gl.copyTexImage2D(gl.TEXTURE_2D, 0, gl.RGBA, 1000, 1000, 20, 20, 0);
+    glErrorShouldBe(gl, gl.NO_ERROR);
+
+    // Green canvas
+    gl.clearColor(0, 1, 0, 1);
+    gl.clear(gl.COLOR_BUFFER_BIT | gl.DEPTH_BUFFER_BIT);
+
+    // Point the uniform sampler to texture unit 0
+    gl.uniform1i(textureLoc, 0);
+    // Draw the triangles
+    gl.drawArrays(gl.TRIANGLES, 0, 6);
+
+    // Read back the rendering results, should be black (blending is off)
+    var buf = new Uint8Array(2 * 2 * 4);
+    gl.readPixels(0, 0, 2, 2, gl.RGBA, gl.UNSIGNED_BYTE, buf);
+    var idx = 0;
+    correctColor = [0, 0, 0];
+    for (var y = 0; y < 2; y++) {
+        for (var x = 0; x < 2; x++) {
+            idx = (y * 2 + x) * 4;
+            pixel[0] = buf[idx];
+            pixel[1] = buf[idx + 1];
+            pixel[2] = buf[idx + 2];
+            shouldBe("pixel", "correctColor");
+        }
+    }
+
+    debug('Testing copyTexSubImage2D');
+
+    // Green canvas
+    gl.clearColor(0, 1, 0, 1);
+    gl.clear(gl.COLOR_BUFFER_BIT | gl.DEPTH_BUFFER_BIT);
+
+    glErrorShouldBe(gl, gl.NO_ERROR);
+    gl.copyTexSubImage2D(gl.TEXTURE_2D, 0, 0, 0, 1000, 1000, 20, 20);
+    glErrorShouldBe(gl, gl.NO_ERROR);
+
+    // Blue canvas
+    gl.clearColor(0, 0, 1, 1);
+    gl.clear(gl.COLOR_BUFFER_BIT | gl.DEPTH_BUFFER_BIT);
+
+    // Draw the triangles
+    gl.drawArrays(gl.TRIANGLES, 0, 6);
+
+    // Read back the rendering results, should be black (blending is off)
+    gl.readPixels(0, 0, 2, 2, gl.RGBA, gl.UNSIGNED_BYTE, buf);
+    correctColor = [0, 0, 0];
+    for (var y = 0; y < 2; y++) {
+        for (var x = 0; x < 2; x++) {
+            idx = (y * 2 + x) * 4;
+            pixel[0] = buf[idx];
+            pixel[1] = buf[idx + 1];
+            pixel[2] = buf[idx + 2];
+            shouldBe("pixel", "correctColor");
+        }
+    }
+}
+
+function runTest(antialias)
+{
+    debug("Testing with antialias on");
+    runTestIteration(true);
+    debug("Testing with antialias off");
+    runTestIteration(false);
+    var epilogue = document.createElement("script");
+    epilogue.onload = finish;
+    epilogue.src = "../../../resources/js-test-post.js";
+    document.body.appendChild(epilogue);
+}
+
+function finish() {
+    if (window.nonKhronosFrameworkNotifyDone) {
+        window.nonKhronosFrameworkNotifyDone();
+    }
+}
+</script>
+</head>
+<body onload="init()">
+<canvas id="antialiasOn" width="2px" height="2px"></canvas>
+<canvas id="antialiasOff" width="2px" height="2px"></canvas>
+<div id="description"></div>
+<div id="console"></div>
+</body>
+</html>
diff --git a/LayoutTests/platform/ios-simulator/fast/canvas/webgl/copy-tex-image-and-sub-image-2d-bad-input-expected.txt b/LayoutTests/platform/ios-simulator/fast/canvas/webgl/copy-tex-image-and-sub-image-2d-bad-input-expected.txt
new file mode 100644 (file)
index 0000000..cc71412
--- /dev/null
@@ -0,0 +1,39 @@
+Verify copyTexImage2D and copyTexSubImage2D can handle bad input
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+Testing with antialias on
+Testing copyTexImage2D
+PASS getError was expected value: NO_ERROR : 
+PASS getError was expected value: NO_ERROR : 
+PASS pixel is correctColor
+PASS pixel is correctColor
+PASS pixel is correctColor
+PASS pixel is correctColor
+Testing copyTexSubImage2D
+PASS getError was expected value: NO_ERROR : 
+PASS getError was expected value: NO_ERROR : 
+PASS pixel is correctColor
+PASS pixel is correctColor
+PASS pixel is correctColor
+PASS pixel is correctColor
+Testing with antialias off
+Testing copyTexImage2D
+PASS getError was expected value: NO_ERROR : 
+PASS getError was expected value: NO_ERROR : 
+PASS pixel is correctColor
+PASS pixel is correctColor
+PASS pixel is correctColor
+PASS pixel is correctColor
+Testing copyTexSubImage2D
+PASS getError was expected value: NO_ERROR : 
+PASS getError was expected value: NO_ERROR : 
+PASS pixel is correctColor
+PASS pixel is correctColor
+PASS pixel is correctColor
+PASS pixel is correctColor
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
index 27971a2..2d01d99 100644 (file)
@@ -1,3 +1,28 @@
+2018-01-05  Dean Jackson  <dino@apple.com>
+
+        Accurately clip copyTexImage2D and copyTexSubImage2D
+        https://bugs.webkit.org/show_bug.cgi?id=181356
+        <rdar://problem/35083877>
+
+        Reviewed by Eric Carlson.
+
+        The code to make sure copyTexSubImage2D and copyTexImage2D will not try to read
+        out of bounds had a bad bug introduced here:
+        https://bugs.webkit.org/show_bug.cgi?id=51421
+
+        With appropriate parameters, it would produce a rectangle with
+        negative dimensions. Most GL drivers just ignored this, but some
+        are not happy.
+
+        Test: fast/canvas/webgl/copy-tex-image-and-sub-image-2d-bad-input.html
+
+        * html/canvas/WebGLRenderingContextBase.cpp:
+        (WebCore::clip2D): Reimplement this in a more sane manner, and use
+        checked arithmetic while here.
+        * html/canvas/WebGLRenderingContextBase.h:
+        (WebCore::clip1D): Deleted.
+        (WebCore::clip2D): Deleted.
+
 2018-01-06  Antti Koivisto  <antti@apple.com>
 
         Use WeakPtr for RenderTreePosition::m_nextSibling
index 70e2b3e..e1a23e4 100644 (file)
@@ -331,6 +331,41 @@ namespace {
     }
 } // namespace anonymous
 
+// Returns false if no clipping is necessary, i.e., x, y, width, height stay the same.
+static bool clip2D(GC3Dint x, GC3Dint y, GC3Dsizei width, GC3Dsizei height,
+    GC3Dsizei sourceWidth, GC3Dsizei sourceHeight,
+    GC3Dint* clippedX, GC3Dint* clippedY, GC3Dsizei* clippedWidth, GC3Dsizei*clippedHeight)
+{
+    ASSERT(clippedX && clippedY && clippedWidth && clippedHeight);
+
+    GC3Dint left = std::max(x, 0);
+    GC3Dint top = std::max(y, 0);
+    GC3Dint right = 0;
+    GC3Dint bottom = 0;
+
+    Checked<GC3Dint, RecordOverflow> checkedInputRight = Checked<GC3Dint>(x) + Checked<GC3Dsizei>(width);
+    Checked<GC3Dint, RecordOverflow> checkedInputBottom = Checked<GC3Dint>(y) + Checked<GC3Dsizei>(height);
+    if (!checkedInputRight.hasOverflowed() && !checkedInputBottom.hasOverflowed()) {
+        right = std::min(checkedInputRight.unsafeGet(), sourceWidth);
+        bottom = std::min(checkedInputBottom.unsafeGet(), sourceHeight);
+    }
+
+    if (left >= right || top >= bottom) {
+        *clippedX = 0;
+        *clippedY = 0;
+        *clippedWidth = 0;
+        *clippedHeight = 0;
+        return true;
+    }
+
+    *clippedX = left;
+    *clippedY = top;
+    *clippedWidth = right - left;
+    *clippedHeight = bottom - top;
+
+    return (*clippedX != x || *clippedY != y || *clippedWidth != width || *clippedHeight != height);
+}
+
 class WebGLRenderingContextLostCallback : public GraphicsContext3D::ContextLostCallback {
     WTF_MAKE_FAST_ALLOCATED;
 public:
index 2f798a3..b7bb76c 100644 (file)
@@ -90,31 +90,6 @@ class WebGLUniformLocation;
 class HTMLVideoElement;
 #endif
 
-inline void clip1D(GC3Dint start, GC3Dsizei range, GC3Dsizei sourceRange, GC3Dint* clippedStart, GC3Dsizei* clippedRange)
-{
-    ASSERT(clippedStart && clippedRange);
-    if (start < 0) {
-        range += start;
-        start = 0;
-    }
-    GC3Dint end = start + range;
-    if (end > sourceRange)
-        range -= end - sourceRange;
-    *clippedStart = start;
-    *clippedRange = range;
-}
-
-// Returns false if no clipping is necessary, i.e., x, y, width, height stay the same.
-inline bool clip2D(GC3Dint x, GC3Dint y, GC3Dsizei width, GC3Dsizei height,
-    GC3Dsizei sourceWidth, GC3Dsizei sourceHeight,
-    GC3Dint* clippedX, GC3Dint* clippedY, GC3Dsizei* clippedWidth, GC3Dsizei*clippedHeight)
-{
-    ASSERT(clippedX && clippedY && clippedWidth && clippedHeight);
-    clip1D(x, width, sourceWidth, clippedX, clippedWidth);
-    clip1D(y, height, sourceHeight, clippedY, clippedHeight);
-    return (*clippedX != x || *clippedY != y || *clippedWidth != width || *clippedHeight != height);
-}
-
 using WebGLCanvas = WTF::Variant<RefPtr<HTMLCanvasElement>, RefPtr<OffscreenCanvas>>;
 
 class WebGLRenderingContextBase : public GPUBasedCanvasRenderingContext, private ActivityStateChangeObserver {