2010-05-04 Zhenyao Mo <zmo@google.com>
authoreric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 5 May 2010 00:30:34 +0000 (00:30 +0000)
committereric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 5 May 2010 00:30:34 +0000 (00:30 +0000)
        Reviewed by Dimitri Glazkov.

        getFramebufferAttachmentParameter should return the original WebGLTexture/WebGLRenderbuffer instead of creating new ones sharing names.
        https://bugs.webkit.org/show_bug.cgi?id=38236

        * fast/canvas/webgl/gl-object-get-calls-expected.txt: Check if getFramebufferAttachmentParameter return a texture/renderbuffer that matches the original one.
        * fast/canvas/webgl/script-tests/gl-object-get-calls.js: Ditto.
2010-05-04  Zhenyao Mo  <zmo@google.com>

        Reviewed by Dimitri Glazkov.

        getFramebufferAttachmentParameter should return the original WebGLTexture/WebGLRenderbuffer instead of creating new ones sharing names.
        https://bugs.webkit.org/show_bug.cgi?id=38236

        * html/canvas/CanvasObject.h: Add type check functions.
        (WebCore::CanvasObject::isBuffer):
        (WebCore::CanvasObject::isFramebuffer):
        (WebCore::CanvasObject::isProgram):
        (WebCore::CanvasObject::isRenderbuffer):
        (WebCore::CanvasObject::isShader):
        (WebCore::CanvasObject::isTexture):
        * html/canvas/WebGLBuffer.h: Add type check functions.
        (WebCore::WebGLBuffer::isBuffer):
        * html/canvas/WebGLFramebuffer.h: Add type check functions.
        (WebCore::WebGLFramebuffer::isFramebuffer):
        * html/canvas/WebGLProgram.h: Add type check functions.
        (WebCore::WebGLProgram::isProgram):
        * html/canvas/WebGLRenderbuffer.cpp: remove constructor using existing name.
        * html/canvas/WebGLRenderbuffer.h: Add type check functions; remove constructor using existing name.
        (WebCore::WebGLRenderbuffer::isRenderbuffer):
        * html/canvas/WebGLRenderingContext.cpp:
        (WebCore::WebGLRenderingContext::getFramebufferAttachmentParameter): Return original Texture/Renderbuffer instead of creating new ones.
        (WebCore::WebGLRenderingContext::findTexture): Find a WebGLTexture using a name.
        (WebCore::WebGLRenderingContext::findRenderbuffer): Find a WebGLRenderbuffer using a name.
        * html/canvas/WebGLRenderingContext.h: Add find* functions.
        * html/canvas/WebGLShader.h: Add type check functions.
        (WebCore::WebGLShader::isShader):
        * html/canvas/WebGLTexture.cpp: remove constructor using existing name.
        * html/canvas/WebGLTexture.h: Add type check functions; remove constructor using existing name.
        (WebCore::WebGLTexture::isTexture):

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

15 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/canvas/webgl/gl-object-get-calls-expected.txt
LayoutTests/fast/canvas/webgl/script-tests/gl-object-get-calls.js
WebCore/ChangeLog
WebCore/html/canvas/CanvasObject.h
WebCore/html/canvas/WebGLBuffer.h
WebCore/html/canvas/WebGLFramebuffer.h
WebCore/html/canvas/WebGLProgram.h
WebCore/html/canvas/WebGLRenderbuffer.cpp
WebCore/html/canvas/WebGLRenderbuffer.h
WebCore/html/canvas/WebGLRenderingContext.cpp
WebCore/html/canvas/WebGLRenderingContext.h
WebCore/html/canvas/WebGLShader.h
WebCore/html/canvas/WebGLTexture.cpp
WebCore/html/canvas/WebGLTexture.h

index 056a8c5..055296d 100644 (file)
@@ -1,3 +1,13 @@
+2010-05-04  Zhenyao Mo  <zmo@google.com>
+
+        Reviewed by Dimitri Glazkov.
+
+        getFramebufferAttachmentParameter should return the original WebGLTexture/WebGLRenderbuffer instead of creating new ones sharing names.
+        https://bugs.webkit.org/show_bug.cgi?id=38236
+
+        * fast/canvas/webgl/gl-object-get-calls-expected.txt: Check if getFramebufferAttachmentParameter return a texture/renderbuffer that matches the original one.
+        * fast/canvas/webgl/script-tests/gl-object-get-calls.js: Ditto.
+
 2010-05-04  David Levin  <levin@chromium.org>
 
         Unreviewed tests skips for qt/gtk.
index 58b8702..2254d73 100644 (file)
@@ -13,11 +13,11 @@ PASS gl.getError() is 0
 PASS gl.getError() is 0
 PASS gl.getError() is 0
 PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE) is gl.TEXTURE
-PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_NAME) is non-null.
+PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_NAME) is texture
 PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_TEXTURE_LEVEL) is 0
 PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_TEXTURE_CUBE_MAP_FACE) is 0
 PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.DEPTH_ATTACHMENT, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE) is gl.RENDERBUFFER
-PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.DEPTH_ATTACHMENT, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_NAME) is non-null.
+PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.DEPTH_ATTACHMENT, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_NAME) is renderbuffer
 PASS gl.getProgramParameter(standardProgram, gl.DELETE_STATUS) is false
 PASS gl.getProgramParameter(standardProgram, gl.LINK_STATUS) is true
 PASS typeof gl.getProgramParameter(standardProgram, gl.VALIDATE_STATUS) is "boolean"
index 0b1e6e3..6a3ee3c 100644 (file)
@@ -62,12 +62,12 @@ gl.framebufferRenderbuffer(gl.FRAMEBUFFER, gl.DEPTH_ATTACHMENT, gl.RENDERBUFFER,
 // parameters to this one. See https://bugs.webkit.org/show_bug.cgi?id=31843.
 //shouldBe('gl.checkFramebufferStatus(gl.FRAMEBUFFER)', 'gl.FRAMEBUFFER_COMPLETE');
 shouldBe('gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE)', 'gl.TEXTURE');
-shouldBeNonNull('gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_NAME)');
+shouldBe('gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_NAME)', 'texture');
 shouldBe('gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_TEXTURE_LEVEL)', '0');
 shouldBe('gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_TEXTURE_CUBE_MAP_FACE)', '0');
 
 shouldBe('gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.DEPTH_ATTACHMENT, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE)', 'gl.RENDERBUFFER');
-shouldBeNonNull('gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.DEPTH_ATTACHMENT, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_NAME)');
+shouldBe('gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.DEPTH_ATTACHMENT, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_NAME)', 'renderbuffer');
 
 // Test getProgramParameter
 shouldBe('gl.getProgramParameter(standardProgram, gl.DELETE_STATUS)', 'false');
index c075ca4..c172f85 100644 (file)
@@ -1,3 +1,37 @@
+2010-05-04  Zhenyao Mo  <zmo@google.com>
+
+        Reviewed by Dimitri Glazkov.
+
+        getFramebufferAttachmentParameter should return the original WebGLTexture/WebGLRenderbuffer instead of creating new ones sharing names.
+        https://bugs.webkit.org/show_bug.cgi?id=38236
+
+        * html/canvas/CanvasObject.h: Add type check functions.
+        (WebCore::CanvasObject::isBuffer):
+        (WebCore::CanvasObject::isFramebuffer):
+        (WebCore::CanvasObject::isProgram):
+        (WebCore::CanvasObject::isRenderbuffer):
+        (WebCore::CanvasObject::isShader):
+        (WebCore::CanvasObject::isTexture):
+        * html/canvas/WebGLBuffer.h: Add type check functions.
+        (WebCore::WebGLBuffer::isBuffer):
+        * html/canvas/WebGLFramebuffer.h: Add type check functions.
+        (WebCore::WebGLFramebuffer::isFramebuffer):
+        * html/canvas/WebGLProgram.h: Add type check functions.
+        (WebCore::WebGLProgram::isProgram):
+        * html/canvas/WebGLRenderbuffer.cpp: remove constructor using existing name.
+        * html/canvas/WebGLRenderbuffer.h: Add type check functions; remove constructor using existing name.
+        (WebCore::WebGLRenderbuffer::isRenderbuffer):
+        * html/canvas/WebGLRenderingContext.cpp:
+        (WebCore::WebGLRenderingContext::getFramebufferAttachmentParameter): Return original Texture/Renderbuffer instead of creating new ones.
+        (WebCore::WebGLRenderingContext::findTexture): Find a WebGLTexture using a name.
+        (WebCore::WebGLRenderingContext::findRenderbuffer): Find a WebGLRenderbuffer using a name.
+        * html/canvas/WebGLRenderingContext.h: Add find* functions.
+        * html/canvas/WebGLShader.h: Add type check functions.
+        (WebCore::WebGLShader::isShader):
+        * html/canvas/WebGLTexture.cpp: remove constructor using existing name.
+        * html/canvas/WebGLTexture.h: Add type check functions; remove constructor using existing name.
+        (WebCore::WebGLTexture::isTexture):
+
 2010-05-04  Luiz Agostini  <luiz.agostini@openbossa.org>
 
         Reviewed by Simon Hausmann.
index b7b016a..6f89f12 100644 (file)
@@ -51,6 +51,13 @@ namespace WebCore {
 
         WebGLRenderingContext* context() const { return m_context; }
 
+        virtual bool isBuffer() const { return false; }
+        virtual bool isFramebuffer() const { return false; }
+        virtual bool isProgram() const { return false; }
+        virtual bool isRenderbuffer() const { return false; }
+        virtual bool isShader() const { return false; }
+        virtual bool isTexture() const { return false; }
+
     protected:
         CanvasObject(WebGLRenderingContext*);
         virtual void _deleteObject(Platform3DObject) = 0;
index bdb7052..f56d374 100644 (file)
@@ -64,6 +64,8 @@ namespace WebCore {
         virtual void _deleteObject(Platform3DObject o);
     
     private:
+        virtual bool isBuffer() const { return true; }
+
         RefPtr<WebGLArrayBuffer> m_elementArrayBuffer;
         unsigned m_elementArrayBufferByteLength;
         unsigned m_arrayBufferByteLength;
index b9402a0..71e5a27 100644 (file)
@@ -50,7 +50,9 @@ namespace WebCore {
         
         virtual void _deleteObject(Platform3DObject);
 
-      private:
+    private:
+        virtual bool isFramebuffer() const { return true; }
+
         bool m_isDepthAttached;
         bool m_isStencilAttached;
         bool m_isDepthStencilAttached;
index 56bce15..b13fc22 100644 (file)
@@ -45,13 +45,15 @@ namespace WebCore {
         bool cacheActiveAttribLocations();
         int numActiveAttribLocations();
         int getActiveAttribLocation(int index);
-        
+
     protected:
         WebGLProgram(WebGLRenderingContext*);
         
         virtual void _deleteObject(Platform3DObject);
 
     private:
+        virtual bool isProgram() const { return true; }
+
         Vector<int> m_activeAttribLocations;
     };
     
index cad4298..0ac9c7e 100644 (file)
@@ -37,11 +37,6 @@ PassRefPtr<WebGLRenderbuffer> WebGLRenderbuffer::create(WebGLRenderingContext* c
     return adoptRef(new WebGLRenderbuffer(ctx));
 }
 
-PassRefPtr<WebGLRenderbuffer> WebGLRenderbuffer::create(WebGLRenderingContext* ctx, Platform3DObject obj)
-{
-    return adoptRef(new WebGLRenderbuffer(ctx, obj));
-}
-
 WebGLRenderbuffer::WebGLRenderbuffer(WebGLRenderingContext* ctx)
     : CanvasObject(ctx)
     , m_internalformat(GraphicsContext3D::RGBA4)
@@ -49,13 +44,6 @@ WebGLRenderbuffer::WebGLRenderbuffer(WebGLRenderingContext* ctx)
     setObject(context()->graphicsContext3D()->createRenderbuffer());
 }
 
-WebGLRenderbuffer::WebGLRenderbuffer(WebGLRenderingContext* ctx, Platform3DObject obj)
-    : CanvasObject(ctx)
-    , m_internalformat(GraphicsContext3D::RGBA4)
-{
-    setObject(obj, false);
-}
-
 void WebGLRenderbuffer::_deleteObject(Platform3DObject object)
 {
     context()->graphicsContext3D()->deleteRenderbuffer(object);
index 1bdf1b7..e399012 100644 (file)
@@ -38,21 +38,18 @@ namespace WebCore {
         virtual ~WebGLRenderbuffer() { deleteObject(); }
         
         static PassRefPtr<WebGLRenderbuffer> create(WebGLRenderingContext*);
-        
-        // For querying previously created objects via e.g. getFramebufferAttachmentParameter
-        // FIXME: should consider canonicalizing these objects
-        static PassRefPtr<WebGLRenderbuffer> create(WebGLRenderingContext*, Platform3DObject renderbuffer);
 
         void setInternalformat(unsigned long internalformat) { m_internalformat = internalformat; }
         unsigned long getInternalformat() const { return m_internalformat; }
 
     protected:
         WebGLRenderbuffer(WebGLRenderingContext*);
-        WebGLRenderbuffer(WebGLRenderingContext*, Platform3DObject);
         
         virtual void _deleteObject(Platform3DObject);
 
-      private:
+    private:
+        virtual bool isRenderbuffer() const { return true; }
+
         unsigned long m_internalformat;
     };
     
index 5d0e73b..05af759 100644 (file)
@@ -979,18 +979,11 @@ WebGLGetInfo WebGLRenderingContext::getFramebufferAttachmentParameter(unsigned l
         m_context->getFramebufferAttachmentParameteriv(target, attachment, GraphicsContext3D::FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE, &type);
         int value = 0;
         m_context->getFramebufferAttachmentParameteriv(target, attachment, GraphicsContext3D::FRAMEBUFFER_ATTACHMENT_OBJECT_NAME, &value);
-        // FIXME: should consider canonicalizing these objects
         switch (type) {
-        case GraphicsContext3D::RENDERBUFFER: {
-            RefPtr<WebGLRenderbuffer> tmp = WebGLRenderbuffer::create(this, value);
-            addObject(tmp.get());
-            return WebGLGetInfo(PassRefPtr<WebGLRenderbuffer>(tmp));
-        }
-        case GraphicsContext3D::TEXTURE: {
-            RefPtr<WebGLTexture> tmp = WebGLTexture::create(this, value);
-            addObject(tmp.get());
-            return WebGLGetInfo(PassRefPtr<WebGLTexture>(tmp));
-        }
+        case GraphicsContext3D::RENDERBUFFER:
+            return WebGLGetInfo(findRenderbuffer(static_cast<Platform3DObject>(value)));
+        case GraphicsContext3D::TEXTURE:
+            return WebGLGetInfo(findTexture(static_cast<Platform3DObject>(value)));
         default:
             // FIXME: raise exception?
             return WebGLGetInfo();
@@ -2843,6 +2836,30 @@ void WebGLRenderingContext::detachAndRemoveAllObjects()
     m_canvasObjects.clear();
 }
 
+PassRefPtr<WebGLTexture> WebGLRenderingContext::findTexture(Platform3DObject obj)
+{
+    HashSet<RefPtr<CanvasObject> >::iterator pend = m_canvasObjects.end();
+    for (HashSet<RefPtr<CanvasObject> >::iterator it = m_canvasObjects.begin(); it != pend; ++it) {
+        if ((*it)->isTexture() && (*it)->object() == obj) {
+            RefPtr<WebGLTexture> tex = reinterpret_cast<WebGLTexture*>((*it).get());
+            return tex.release();
+        }
+    }
+    return 0;
+}
+
+PassRefPtr<WebGLRenderbuffer> WebGLRenderingContext::findRenderbuffer(Platform3DObject obj)
+{
+    HashSet<RefPtr<CanvasObject> >::iterator pend = m_canvasObjects.end();
+    for (HashSet<RefPtr<CanvasObject> >::iterator it = m_canvasObjects.begin(); it != pend; ++it) {
+        if ((*it)->isRenderbuffer() && (*it)->object() == obj) {
+            RefPtr<WebGLRenderbuffer> buffer = reinterpret_cast<WebGLRenderbuffer*>((*it).get());
+            return buffer.release();
+        }
+    }
+    return 0;
+}
+
 WebGLGetInfo WebGLRenderingContext::getBooleanParameter(unsigned long pname)
 {
     unsigned char value;
index 17dfe8c..aefa6cc 100644 (file)
@@ -299,6 +299,8 @@ class WebKitCSSMatrix;
 
         void addObject(CanvasObject*);
         void detachAndRemoveAllObjects();
+        PassRefPtr<WebGLTexture> findTexture(Platform3DObject);
+        PassRefPtr<WebGLRenderbuffer> findRenderbuffer(Platform3DObject);
 
         void markContextChanged();
         void cleanupAfterGraphicsCall(bool changed)
index 1ef912c..d4006aa 100644 (file)
@@ -38,11 +38,13 @@ namespace WebCore {
         virtual ~WebGLShader() { deleteObject(); }
         
         static PassRefPtr<WebGLShader> create(WebGLRenderingContext*, GraphicsContext3D::WebGLEnumType);
-        
+
     private:
         WebGLShader(WebGLRenderingContext*, GraphicsContext3D::WebGLEnumType);
 
         virtual void _deleteObject(Platform3DObject);
+
+        virtual bool isShader() const { return true; }
     };
     
 } // namespace WebCore
index ae09b48..0c58edb 100644 (file)
@@ -37,11 +37,6 @@ PassRefPtr<WebGLTexture> WebGLTexture::create(WebGLRenderingContext* ctx)
     return adoptRef(new WebGLTexture(ctx));
 }
 
-PassRefPtr<WebGLTexture> WebGLTexture::create(WebGLRenderingContext* ctx, Platform3DObject obj)
-{
-    return adoptRef(new WebGLTexture(ctx, obj));
-}
-
 WebGLTexture::WebGLTexture(WebGLRenderingContext* ctx)
     : CanvasObject(ctx)
     , cubeMapRWrapModeInitialized(false)
@@ -49,13 +44,6 @@ WebGLTexture::WebGLTexture(WebGLRenderingContext* ctx)
     setObject(context()->graphicsContext3D()->createTexture());
 }
 
-WebGLTexture::WebGLTexture(WebGLRenderingContext* ctx, Platform3DObject obj)
-    : CanvasObject(ctx)
-    , cubeMapRWrapModeInitialized(false)
-{
-    setObject(obj, false);
-}
-
 void WebGLTexture::_deleteObject(Platform3DObject object)
 {
     context()->graphicsContext3D()->deleteTexture(object);
index c64dd41..b3c1f05 100644 (file)
@@ -38,10 +38,6 @@ namespace WebCore {
         virtual ~WebGLTexture() { deleteObject(); }
         
         static PassRefPtr<WebGLTexture> create(WebGLRenderingContext*);
-    
-        // For querying previously created objects via e.g. getFramebufferAttachmentParameter
-        // FIXME: should consider canonicalizing these objects
-        static PassRefPtr<WebGLTexture> create(WebGLRenderingContext*, Platform3DObject);
 
         bool isCubeMapRWrapModeInitialized() {
             return cubeMapRWrapModeInitialized;
@@ -53,11 +49,12 @@ namespace WebCore {
 
     protected:
         WebGLTexture(WebGLRenderingContext*);
-        WebGLTexture(WebGLRenderingContext*, Platform3DObject);
 
         virtual void _deleteObject(Platform3DObject);
 
     private:
+        virtual bool isTexture() const { return true; }
+
         bool cubeMapRWrapModeInitialized;
     };