2011-01-06 Zhenyao Mo <zmo@google.com>
authorzmo@google.com <zmo@google.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 7 Jan 2011 18:19:54 +0000 (18:19 +0000)
committerzmo@google.com <zmo@google.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 7 Jan 2011 18:19:54 +0000 (18:19 +0000)
        Reviewed by Kenneth Russell.

        copyTexSubImage2D shouldn't have undefined pixels
        https://bugs.webkit.org/show_bug.cgi?id=51559

        * html/canvas/WebGLRenderingContext.cpp:
        (WebCore::WebGLRenderingContext::copyTexImage2D): Refactor to share some code with copyTexSubImage2D through helper function clip2D.
        (WebCore::WebGLRenderingContext::copyTexSubImage2D): Initialize undefined pixels to 0.
        (WebCore::WebGLRenderingContext::validateTexFuncLevel): Seperate the validation of level from validateTexFuncParameters.
        (WebCore::WebGLRenderingContext::validateTexFuncParameters): Ditto.
        * html/canvas/WebGLRenderingContext.h:
        * html/canvas/WebGLTexture.cpp:
        (WebCore::WebGLTexture::getType): Expose the type of a texture.
        * html/canvas/WebGLTexture.h:
2011-01-06  Zhenyao Mo  <zmo@google.com>

        Reviewed by Kenneth Russell.

        copyTexSubImage2D shouldn't have undefined pixels
        https://bugs.webkit.org/show_bug.cgi?id=51559

        * fast/canvas/webgl/uninitialized-test-expected.txt:
        * fast/canvas/webgl/uninitialized-test.html: Add test cases for copyTexSubImage2D.

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

LayoutTests/ChangeLog
LayoutTests/fast/canvas/webgl/uninitialized-test-expected.txt
LayoutTests/fast/canvas/webgl/uninitialized-test.html
WebCore/ChangeLog
WebCore/html/canvas/WebGLRenderingContext.cpp
WebCore/html/canvas/WebGLRenderingContext.h
WebCore/html/canvas/WebGLTexture.cpp
WebCore/html/canvas/WebGLTexture.h

index 6446845..9c6dba3 100644 (file)
@@ -1,3 +1,13 @@
+2011-01-06  Zhenyao Mo  <zmo@google.com>
+
+        Reviewed by Kenneth Russell.
+
+        copyTexSubImage2D shouldn't have undefined pixels
+        https://bugs.webkit.org/show_bug.cgi?id=51559
+
+        * fast/canvas/webgl/uninitialized-test-expected.txt:
+        * fast/canvas/webgl/uninitialized-test.html: Add test cases for copyTexSubImage2D.
+
 2011-01-07  Stephen White  <senorblanco@chromium.org>
 
         Unreviewed; new test results and expectations updates.
index 89d88a5..e0cd9b8 100644 (file)
@@ -29,6 +29,29 @@ PASS gl.checkFramebufferStatus(gl.FRAMEBUFFER) is gl.FRAMEBUFFER_COMPLETE
 PASS All data initialized
 PASS getError was expected value: NO_ERROR : 
 
+Reading an uninitialized portion of a texture (copyTexSubImage2D) should succeed with all bytes set to 0.
+PASS getError was expected value: NO_ERROR : 
+PASS gl.checkFramebufferStatus(gl.FRAMEBUFFER) is gl.FRAMEBUFFER_COMPLETE
+PASS getError was expected value: NO_ERROR : 
+PASS gl.checkFramebufferStatus(gl.FRAMEBUFFER) is gl.FRAMEBUFFER_COMPLETE
+PASS All data initialized
+PASS getError was expected value: NO_ERROR : 
+
+Reading an uninitialized portion of a texture (copyTexSubImage2D with negative x and y) should succeed with all bytes set to 0.
+PASS getError was expected value: NO_ERROR : 
+PASS gl.checkFramebufferStatus(gl.FRAMEBUFFER) is gl.FRAMEBUFFER_COMPLETE
+PASS getError was expected value: NO_ERROR : 
+PASS gl.checkFramebufferStatus(gl.FRAMEBUFFER) is gl.FRAMEBUFFER_COMPLETE
+PASS All data initialized
+PASS getError was expected value: NO_ERROR : 
+
+Reading an uninitialized portion of a texture (copyTexSubImage2D from WebGL internal fbo) should succeed with all bytes set to 0.
+PASS getError was expected value: NO_ERROR : 
+PASS getError was expected value: NO_ERROR : 
+PASS gl.checkFramebufferStatus(gl.FRAMEBUFFER) is gl.FRAMEBUFFER_COMPLETE
+PASS All data initialized
+PASS getError was expected value: NO_ERROR : 
+
 PASS successfullyParsed is true
 
 TEST COMPLETE
index 0975006..18eb165 100644 (file)
@@ -152,6 +152,84 @@ gl.deleteTexture(tex);
 gl.finish();
 glErrorShouldBe(gl, gl.NO_ERROR);
 
+debug("");
+debug("Reading an uninitialized portion of a texture (copyTexSubImage2D) should succeed with all bytes set to 0.");
+
+var tex = gl.createTexture();
+gl.bindTexture(gl.TEXTURE_2D, tex);
+var data = new Uint8Array(width * height * 4);
+for (var i = 0; i < width * height * 4; ++i)
+    data[i] = 255;
+gl.texImage2D(gl.TEXTURE_2D, 0, gl.RGBA, width, height, 0, gl.RGBA, gl.UNSIGNED_BYTE, data);
+glErrorShouldBe(gl, gl.NO_ERROR);
+var fbo = gl.createFramebuffer();
+gl.bindFramebuffer(gl.FRAMEBUFFER, fbo);
+var rbo = gl.createRenderbuffer();
+gl.bindRenderbuffer(gl.RENDERBUFFER, rbo);
+var fboWidth = 16;
+var fboHeight = 16;
+gl.renderbufferStorage(gl.RENDERBUFFER, gl.RGBA4, fboWidth, fboHeight);
+gl.framebufferRenderbuffer(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.RENDERBUFFER, rbo);
+shouldBe("gl.checkFramebufferStatus(gl.FRAMEBUFFER)", "gl.FRAMEBUFFER_COMPLETE");
+gl.clearColor(1.0, 0.0, 0.0, 1.0);
+gl.clear(gl.COLOR_BUFFER_BIT);
+glErrorShouldBe(gl, gl.NO_ERROR);
+gl.copyTexSubImage2D(gl.TEXTURE_2D, 0, 0, 0, 0, 0, width, height);
+checkNonZeroPixels(tex, width, height, 0, 0, fboWidth, fboHeight, 255, 0, 0, 255);
+gl.deleteTexture(tex);
+gl.finish();
+glErrorShouldBe(gl, gl.NO_ERROR);
+
+debug("");
+debug("Reading an uninitialized portion of a texture (copyTexSubImage2D with negative x and y) should succeed with all bytes set to 0.");
+
+var tex = gl.createTexture();
+gl.bindTexture(gl.TEXTURE_2D, tex);
+var data = new Uint8Array(width * height * 4);
+for (var i = 0; i < width * height * 4; ++i)
+    data[i] = 255;
+gl.texImage2D(gl.TEXTURE_2D, 0, gl.RGBA, width, height, 0, gl.RGBA, gl.UNSIGNED_BYTE, data);
+glErrorShouldBe(gl, gl.NO_ERROR);
+var fbo = gl.createFramebuffer();
+gl.bindFramebuffer(gl.FRAMEBUFFER, fbo);
+var rbo = gl.createRenderbuffer();
+gl.bindRenderbuffer(gl.RENDERBUFFER, rbo);
+var fboWidth = 16;
+var fboHeight = 16;
+gl.renderbufferStorage(gl.RENDERBUFFER, gl.RGBA4, fboWidth, fboHeight);
+gl.framebufferRenderbuffer(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.RENDERBUFFER, rbo);
+shouldBe("gl.checkFramebufferStatus(gl.FRAMEBUFFER)", "gl.FRAMEBUFFER_COMPLETE");
+gl.clearColor(1.0, 0.0, 0.0, 1.0);
+gl.clear(gl.COLOR_BUFFER_BIT);
+glErrorShouldBe(gl, gl.NO_ERROR);
+var x = -8;
+var y = -8;
+gl.copyTexSubImage2D(gl.TEXTURE_2D, 0, 0, 0, x, y, width, height);
+checkNonZeroPixels(tex, width, height, -x, -y, fboWidth, fboHeight, 255, 0, 0, 255);
+gl.deleteTexture(tex);
+gl.finish();
+glErrorShouldBe(gl, gl.NO_ERROR);
+
+debug("");
+debug("Reading an uninitialized portion of a texture (copyTexSubImage2D from WebGL internal fbo) should succeed with all bytes set to 0.");
+
+var tex = gl.createTexture();
+gl.bindTexture(gl.TEXTURE_2D, tex);
+var data = new Uint8Array(width * height * 4);
+for (var i = 0; i < width * height * 4; ++i)
+    data[i] = 255;
+gl.texImage2D(gl.TEXTURE_2D, 0, gl.RGBA, width, height, 0, gl.RGBA, gl.UNSIGNED_BYTE, data);
+glErrorShouldBe(gl, gl.NO_ERROR);
+gl.bindFramebuffer(gl.FRAMEBUFFER, null);
+gl.clearColor(0.0, 1.0, 0.0, 0.0);
+gl.clear(gl.COLOR_BUFFER_BIT);
+glErrorShouldBe(gl, gl.NO_ERROR);
+gl.copyTexSubImage2D(gl.TEXTURE_2D, 0, 0, 0, 0, 0, width, height);
+checkNonZeroPixels(tex, width, height, 0, 0, canvas.width, canvas.height, 0, 255, 0, 0);
+gl.deleteTexture(tex);
+gl.finish();
+glErrorShouldBe(gl, gl.NO_ERROR);
+
 //TODO: uninitialized vertex array buffer
 //TODO: uninitialized vertex elements buffer
 //TODO: uninitialized framebuffer? (implementations would need to do a GL clear at first binding?)
index bc2e248..3e3dc32 100644 (file)
@@ -1,3 +1,20 @@
+2011-01-06  Zhenyao Mo  <zmo@google.com>
+
+        Reviewed by Kenneth Russell.
+
+        copyTexSubImage2D shouldn't have undefined pixels
+        https://bugs.webkit.org/show_bug.cgi?id=51559
+
+        * html/canvas/WebGLRenderingContext.cpp:
+        (WebCore::WebGLRenderingContext::copyTexImage2D): Refactor to share some code with copyTexSubImage2D through helper function clip2D.
+        (WebCore::WebGLRenderingContext::copyTexSubImage2D): Initialize undefined pixels to 0.
+        (WebCore::WebGLRenderingContext::validateTexFuncLevel): Seperate the validation of level from validateTexFuncParameters.
+        (WebCore::WebGLRenderingContext::validateTexFuncParameters): Ditto.
+        * html/canvas/WebGLRenderingContext.h:
+        * html/canvas/WebGLTexture.cpp:
+        (WebCore::WebGLTexture::getType): Expose the type of a texture.
+        * html/canvas/WebGLTexture.h:
+
 2011-01-07  Takashi Toyoshima  <toyoshim@google.com>
 
         Reviewed by Kenneth Russell.
index cc92fd0..206b1b2 100644 (file)
@@ -60,6 +60,7 @@
 
 #include <wtf/ByteArray.h>
 #include <wtf/OwnArrayPtr.h>
+#include <wtf/PassOwnArrayPtr.h>
 
 namespace WebCore {
 
@@ -72,7 +73,7 @@ namespace {
         return object ? object->object() : 0;
     }
 
-    void clip(long start, long range, long sourceRange, long* clippedStart, long* clippedRange)
+    void clip1D(long start, long range, long sourceRange, long* clippedStart, long* clippedRange)
     {
         ASSERT(clippedStart && clippedRange);
         if (start < 0) {
@@ -86,6 +87,17 @@ namespace {
         *clippedRange = range;
     }
 
+    // Returns false if no clipping is necessary, i.e., x, y, width, height stay the same.
+    bool clip2D(long x, long y, long width, long height,
+                long sourceWidth, long sourceHeight,
+                long* clippedX, long* clippedY, long* clippedWidth, long*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);
+    }
+
     // Return true if a character belongs to the ASCII subset as defined in
     // GLSL ES 1.0 spec section 3.1.
     bool validateCharacter(unsigned char c)
@@ -693,9 +705,7 @@ void WebGLRenderingContext::copyTexImage2D(unsigned long target, long level, uns
         m_context->copyTexImage2D(target, level, internalformat, x, y, width, height, border);
     else {
         long clippedX, clippedY, clippedWidth, clippedHeight;
-        clip(x, width, getBoundFramebufferWidth(), &clippedX, &clippedWidth);
-        clip(y, height, getBoundFramebufferHeight(), &clippedY, &clippedHeight);
-        if (clippedX != x || clippedY != y || clippedWidth != static_cast<long>(width) || clippedHeight != static_cast<long>(height)) {
+        if (clip2D(x, y, width, height, getBoundFramebufferWidth(), getBoundFramebufferHeight(), &clippedX, &clippedY, &clippedWidth, &clippedHeight)) {
             m_context->texImage2DResourceSafe(target, level, internalformat, width, height, border,
                                               internalformat, GraphicsContext3D::UNSIGNED_BYTE);
             if (clippedWidth > 0 && clippedHeight > 0) {
@@ -714,11 +724,17 @@ void WebGLRenderingContext::copyTexSubImage2D(unsigned long target, long level,
 {
     if (isContextLost())
         return;
+    if (!validateTexFuncLevel(target, level))
+        return;
     WebGLTexture* tex = validateTextureBinding(target, true);
     if (!tex)
         return;
     if (!validateSize(xoffset, yoffset) || !validateSize(width, height))
         return;
+    if (xoffset + width > tex->getWidth(target, level) || yoffset + height > tex->getHeight(target, level)) {
+        m_context->synthesizeGLError(GraphicsContext3D::INVALID_VALUE);
+        return;
+    }
     if (!isTexInternalFormatColorBufferCombinationValid(tex->getInternalFormat(target, level), getBoundFramebufferColorFormat())) {
         m_context->synthesizeGLError(GraphicsContext3D::INVALID_OPERATION);
         return;
@@ -727,7 +743,42 @@ void WebGLRenderingContext::copyTexSubImage2D(unsigned long target, long level,
         m_context->synthesizeGLError(GraphicsContext3D::INVALID_FRAMEBUFFER_OPERATION);
         return;
     }
-    m_context->copyTexSubImage2D(target, level, xoffset, yoffset, x, y, width, height);
+    if (isResourceSafe())
+        m_context->copyTexSubImage2D(target, level, xoffset, yoffset, x, y, width, height);
+    else {
+        long clippedX, clippedY, clippedWidth, clippedHeight;
+        if (clip2D(x, y, width, height, getBoundFramebufferWidth(), getBoundFramebufferHeight(), &clippedX, &clippedY, &clippedWidth, &clippedHeight)) {
+            unsigned long format = tex->getInternalFormat(target, level);
+            unsigned long type = tex->getType(target, level);
+            unsigned long componentsPerPixel = 0;
+            unsigned long bytesPerComponent = 0;
+            bool valid = m_context->computeFormatAndTypeParameters(format, type, &componentsPerPixel, &bytesPerComponent);
+            if (!valid) {
+                m_context->synthesizeGLError(GraphicsContext3D::INVALID_OPERATION);
+                return;
+            }
+            OwnArrayPtr<unsigned char> zero;
+            if (width && height) {
+                unsigned long size = componentsPerPixel * bytesPerComponent * width * height;
+                zero = adoptArrayPtr(new unsigned char[size]);
+                if (!zero) {
+                    m_context->synthesizeGLError(GraphicsContext3D::INVALID_VALUE);
+                    return;
+                }
+                memset(zero.get(), 0, size);
+            }
+            if (zero)
+                m_context->pixelStorei(GraphicsContext3D::UNPACK_ALIGNMENT, 1);
+            m_context->texSubImage2D(target, level, xoffset, yoffset, width, height, format, type, zero.get());
+            if (zero)
+                m_context->pixelStorei(GraphicsContext3D::UNPACK_ALIGNMENT, m_unpackAlignment);
+            if (clippedWidth > 0 && clippedHeight > 0) {
+                m_context->copyTexSubImage2D(target, level, xoffset + clippedX - x, yoffset + clippedY - y,
+                                             clippedX, clippedY, clippedWidth, clippedHeight);
+            }
+        } else
+            m_context->copyTexSubImage2D(target, level, xoffset, yoffset, x, y, width, height);
+    }
     cleanupAfterGraphicsCall(false);
 }
 
@@ -3814,6 +3865,36 @@ bool WebGLRenderingContext::validateTexFuncFormatAndType(unsigned long format, u
     return true;
 }
 
+bool WebGLRenderingContext::validateTexFuncLevel(unsigned long target, long level)
+{
+    if (level < 0) {
+        m_context->synthesizeGLError(GraphicsContext3D::INVALID_VALUE);
+        return false;
+    }
+    switch (target) {
+    case GraphicsContext3D::TEXTURE_2D:
+        if (level > m_maxTextureLevel) {
+            m_context->synthesizeGLError(GraphicsContext3D::INVALID_VALUE);
+            return false;
+        }
+        break;
+    case GraphicsContext3D::TEXTURE_CUBE_MAP_POSITIVE_X:
+    case GraphicsContext3D::TEXTURE_CUBE_MAP_NEGATIVE_X:
+    case GraphicsContext3D::TEXTURE_CUBE_MAP_POSITIVE_Y:
+    case GraphicsContext3D::TEXTURE_CUBE_MAP_NEGATIVE_Y:
+    case GraphicsContext3D::TEXTURE_CUBE_MAP_POSITIVE_Z:
+    case GraphicsContext3D::TEXTURE_CUBE_MAP_NEGATIVE_Z:
+        if (level > m_maxCubeMapTextureLevel) {
+            m_context->synthesizeGLError(GraphicsContext3D::INVALID_VALUE);
+            return false;
+        }
+        break;
+    }
+    // This function only checks if level is legal, so we return true and don't
+    // generate INVALID_ENUM if target is illegal.
+    return true;
+}
+
 bool WebGLRenderingContext::validateTexFuncParameters(unsigned long target, long level,
                                                       unsigned long internalformat,
                                                       long width, long height, long border,
@@ -3822,17 +3903,17 @@ bool WebGLRenderingContext::validateTexFuncParameters(unsigned long target, long
     // We absolutely have to validate the format and type combination.
     // The texImage2D entry points taking HTMLImage, etc. will produce
     // temporary data based on this combination, so it must be legal.
-    if (!validateTexFuncFormatAndType(format, type))
+    if (!validateTexFuncFormatAndType(format, type) || !validateTexFuncLevel(target, level))
         return false;
 
-    if (width < 0 || height < 0 || level < 0) {
+    if (width < 0 || height < 0) {
         m_context->synthesizeGLError(GraphicsContext3D::INVALID_VALUE);
         return false;
     }
 
     switch (target) {
     case GraphicsContext3D::TEXTURE_2D:
-        if (width > m_maxTextureSize || height > m_maxTextureSize || level > m_maxTextureLevel) {
+        if (width > m_maxTextureSize || height > m_maxTextureSize) {
             m_context->synthesizeGLError(GraphicsContext3D::INVALID_VALUE);
             return false;
         }
@@ -3843,7 +3924,7 @@ bool WebGLRenderingContext::validateTexFuncParameters(unsigned long target, long
     case GraphicsContext3D::TEXTURE_CUBE_MAP_NEGATIVE_Y:
     case GraphicsContext3D::TEXTURE_CUBE_MAP_POSITIVE_Z:
     case GraphicsContext3D::TEXTURE_CUBE_MAP_NEGATIVE_Z:
-        if (width != height || width > m_maxCubeMapTextureSize || level > m_maxCubeMapTextureLevel) {
+        if (width != height || width > m_maxCubeMapTextureSize) {
             m_context->synthesizeGLError(GraphicsContext3D::INVALID_VALUE);
             return false;
         }
index d0c55e7..1414c7c 100644 (file)
@@ -528,6 +528,10 @@ public:
     // Generates GL error and returns false if parameters are invalid.
     bool validateTexFuncFormatAndType(unsigned long format, unsigned long type);
 
+    // Helper function to check input level for functions {copy}Tex{Sub}Image.
+    // Generates GL error and returns false if level is invalid.
+    bool validateTexFuncLevel(unsigned long target, long level);
+
     // Helper function to check input parameters for functions {copy}Tex{Sub}Image.
     // Generates GL error and returns false if parameters are invalid.
     bool validateTexFuncParameters(unsigned long target, long level,
index 5d06c3f..2982e51 100644 (file)
@@ -177,6 +177,14 @@ unsigned long WebGLTexture::getInternalFormat(unsigned long target, int level) c
     return info->internalFormat;
 }
 
+unsigned long WebGLTexture::getType(unsigned long target, int level) const
+{
+    const LevelInfo* info = getLevelInfo(target, level);
+    if (!info)
+        return 0;
+    return info->type;
+}
+
 int WebGLTexture::getWidth(unsigned long target, int level) const
 {
     const LevelInfo* info = getLevelInfo(target, level);
index 12b89aa..419c94d 100644 (file)
@@ -53,6 +53,7 @@ public:
     void generateMipmapLevelInfo();
 
     unsigned long getInternalFormat(unsigned long target, int level) const;
+    unsigned long getType(unsigned long target, int level) const;
     int getWidth(unsigned long target, int level) const;
     int getHeight(unsigned long target, int level) const;