Improve WebGL object lifetime management in WebGLRenderingContext

2011-06-30  Zhenyao Mo  <zmo@google.com>

        Reviewed by Kenneth Russell.

        Improve WebGL object lifetime management in WebGLRenderingContext
        https://bugs.webkit.org/show_bug.cgi?id=63635

        * fast/canvas/webgl/gl-object-get-calls-expected.txt:
        * fast/canvas/webgl/gl-object-get-calls.html: Fix a bug so getFramebufferAtatchmentParameter generates an error if nothing is attached and something other than TYPE is queried.
        * fast/canvas/webgl/object-deletion-behaviour-expected.txt:
        * fast/canvas/webgl/object-deletion-behaviour.html: Ditto.
        * fast/canvas/webgl/program-test.html: Fix the test so the order of shaders returned by getAttachedShaders doesn't matter.
2011-06-30  Zhenyao Mo  <zmo@google.com>

        Reviewed by Kenneth Russell.

        Improve WebGL object lifetime management in WebGLRenderingContext
        https://bugs.webkit.org/show_bug.cgi?id=63635

        * html/canvas/WebGLRenderingContext.cpp:
        (WebCore::WebGLRenderingContext::getAttachedShaders): Use cached objects instead of querying the underlying GL.
        (WebCore::WebGLRenderingContext::getFramebufferAttachmentParameter): Ditto.
        (WebCore::WebGLRenderingContext::detachAndRemoveAllObjects): Multiple loop because objects might be removed from the table within an iteration.
        * html/canvas/WebGLRenderingContext.h:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@90180 268f45cc-cd09-0410-ab3c-d52691b4dbfc
CRs-fixed: 407009

(cherry-picked from commit 2dbba5d5facb2c0c57a37eb0cf3958bae0f697fa)

Change-Id: I2cb88cc5790452c18f6e58420cc7b063c9ffce26

5 files changed, 24 insertions, 90 deletions

diff --git a/Source/WebCore/bindings/v8/custom/V8WebGLRenderingContextCustom.cpp b/Source/WebCore/bindings/v8/custom/V8WebGLRenderingContextCustom.cpp
index a23b707..2441dfb 100644
--- a/Source/WebCore/bindings/v8/custom/V8WebGLRenderingContextCustom.cpp
+++ b/Source/WebCore/bindings/v8/custom/V8WebGLRenderingContextCustom.cpp
@@ -164,7 +164,7 @@ static v8::Handle<v8::Value> toV8Object(WebGLExtension* extension, v8::Handle<v8
     if (!extension)
         return v8::Null();
     v8::Handle<v8::Value> extensionObject;
-    const char* referenceName;
+    const char* referenceName = 0;
     switch (extension->getName()) {
     case WebGLExtension::WebKitLoseContextName:
         extensionObject = toV8(static_cast<WebKitLoseContext*>(extension));
diff --git a/Source/WebCore/html/canvas/WebGLObject.cpp b/Source/WebCore/html/canvas/WebGLObject.cpp
index d47013e..7b629a6 100644
--- a/Source/WebCore/html/canvas/WebGLObject.cpp
+++ b/Source/WebCore/html/canvas/WebGLObject.cpp
@@ -66,6 +66,16 @@ void WebGLObject::deleteObject()
     }
 }
 
+void WebGLObject::detachContext()
+{
+    m_attachmentCount = 0; // Make sure OpenGL resource is deleted.
+    if (m_context) {
+        deleteObject();
+        m_context->removeObject(this);
+        m_context = 0;
+    }
+}
+
 }
 
 #endif // ENABLE(WEBGL)
diff --git a/Source/WebCore/html/canvas/WebGLObject.h b/Source/WebCore/html/canvas/WebGLObject.h
index 3bb9f29..44cc36f 100644
--- a/Source/WebCore/html/canvas/WebGLObject.h
+++ b/Source/WebCore/html/canvas/WebGLObject.h
@@ -46,12 +46,7 @@ public:
     // FIXME: revisit this when resource sharing between contexts are implemented.
     void deleteObject();
 
-    void detachContext()
-    {
-        m_attachmentCount = 0; // Make sure OpenGL resource is deleted.
-        deleteObject();
-        m_context = 0;
-    }
+    void detachContext();
 
     WebGLRenderingContext* context() const { return m_context; }
 
diff --git a/Source/WebCore/html/canvas/WebGLRenderingContext.cpp b/Source/WebCore/html/canvas/WebGLRenderingContext.cpp
index 30a88fa..7e7bfe3 100644
--- a/Source/WebCore/html/canvas/WebGLRenderingContext.cpp
+++ b/Source/WebCore/html/canvas/WebGLRenderingContext.cpp
@@ -1929,23 +1929,15 @@ bool WebGLRenderingContext::getAttachedShaders(WebGLProgram* program, Vector<Web
     shaderObjects.clear();
     if (isContextLost() || !validateWebGLObject(program))
         return false;
-    GC3Dint numShaders = 0;
-    m_context->getProgramiv(objectOrZero(program), GraphicsContext3D::ATTACHED_SHADERS, &numShaders);
-    if (numShaders) {
-        OwnArrayPtr<Platform3DObject> shaders = adoptArrayPtr(new Platform3DObject[numShaders]);
-        GC3Dsizei count = 0;
-        m_context->getAttachedShaders(objectOrZero(program), numShaders, &count, shaders.get());
-        if (count != numShaders)
-            return false;
-        shaderObjects.resize(numShaders);
-        for (GC3Dint ii = 0; ii < numShaders; ++ii) {
-            WebGLShader* shader = findShader(shaders[ii]);
-            if (!shader) {
-                shaderObjects.clear();
-                return false;
-            }
-            shaderObjects[ii] = shader;
-        }
+
+    const GC3Denum shaderType[] = {
+        GraphicsContext3D::VERTEX_SHADER,
+        GraphicsContext3D::FRAGMENT_SHADER
+    };
+    for (unsigned i = 0; i < sizeof(shaderType) / sizeof(GC3Denum); ++i) {
+        WebGLShader* shader = program->getAttachedShader(shaderType[i]);
+        if (shader)
+            shaderObjects.append(shader);
     }
     return true;
 }
@@ -2047,16 +2039,6 @@ WebGLGetInfo WebGLRenderingContext::getFramebufferAttachmentParameter(GC3Denum t
     UNUSED_PARAM(ec);
     if (isContextLost() || !validateFramebufferFuncParameters(target, attachment))
         return WebGLGetInfo();
-    switch (pname) {
-    case GraphicsContext3D::FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE:
-    case GraphicsContext3D::FRAMEBUFFER_ATTACHMENT_OBJECT_NAME:
-    case GraphicsContext3D::FRAMEBUFFER_ATTACHMENT_TEXTURE_LEVEL:
-    case GraphicsContext3D::FRAMEBUFFER_ATTACHMENT_TEXTURE_CUBE_MAP_FACE:
-        break;
-    default:
-        m_context->synthesizeGLError(GraphicsContext3D::INVALID_ENUM);
-        return WebGLGetInfo();
-    }
 
     if (!m_framebufferBinding || !m_framebufferBinding->object()) {
         m_context->synthesizeGLError(GraphicsContext3D::INVALID_OPERATION);
@@ -4123,59 +4105,10 @@ void WebGLRenderingContext::addObject(WebGLObject* object)
 
 void WebGLRenderingContext::detachAndRemoveAllObjects()
 {
-    HashSet<RefPtr<WebGLObject> >::iterator pend = m_canvasObjects.end();
-    for (HashSet<RefPtr<WebGLObject> >::iterator it = m_canvasObjects.begin(); it != pend; ++it)
+    while (m_canvasObjects.size() > 0) {
+        HashSet<WebGLObject*>::iterator it = m_canvasObjects.begin();
         (*it)->detachContext();
-
-    m_canvasObjects.clear();
-}
-
-WebGLTexture* WebGLRenderingContext::findTexture(Platform3DObject obj)
-{
-    if (!obj)
-        return 0;
-    HashSet<RefPtr<WebGLObject> >::iterator pend = m_canvasObjects.end();
-    for (HashSet<RefPtr<WebGLObject> >::iterator it = m_canvasObjects.begin(); it != pend; ++it) {
-        if ((*it)->isTexture() && (*it)->object() == obj)
-            return reinterpret_cast<WebGLTexture*>((*it).get());
-    }
-    return 0;
-}
-
-WebGLRenderbuffer* WebGLRenderingContext::findRenderbuffer(Platform3DObject obj)
-{
-    if (!obj)
-        return 0;
-    HashSet<RefPtr<WebGLObject> >::iterator pend = m_canvasObjects.end();
-    for (HashSet<RefPtr<WebGLObject> >::iterator it = m_canvasObjects.begin(); it != pend; ++it) {
-        if ((*it)->isRenderbuffer() && (*it)->object() == obj)
-            return reinterpret_cast<WebGLRenderbuffer*>((*it).get());
-    }
-    return 0;
-}
-
-WebGLBuffer* WebGLRenderingContext::findBuffer(Platform3DObject obj)
-{
-    if (!obj)
-        return 0;
-    HashSet<RefPtr<WebGLObject> >::iterator pend = m_canvasObjects.end();
-    for (HashSet<RefPtr<WebGLObject> >::iterator it = m_canvasObjects.begin(); it != pend; ++it) {
-        if ((*it)->isBuffer() && (*it)->object() == obj)
-            return reinterpret_cast<WebGLBuffer*>((*it).get());
     }
-    return 0;
-}
-
-WebGLShader* WebGLRenderingContext::findShader(Platform3DObject obj)
-{
-    if (!obj)
-        return 0;
-    HashSet<RefPtr<WebGLObject> >::iterator pend = m_canvasObjects.end();
-    for (HashSet<RefPtr<WebGLObject> >::iterator it = m_canvasObjects.begin(); it != pend; ++it) {
-        if ((*it)->isShader() && (*it)->object() == obj)
-            return reinterpret_cast<WebGLShader*>((*it).get());
-    }
-    return 0;
 }
 
 WebGLGetInfo WebGLRenderingContext::getBooleanParameter(GC3Denum pname)
diff --git a/Source/WebCore/html/canvas/WebGLRenderingContext.h b/Source/WebCore/html/canvas/WebGLRenderingContext.h
index c4a167f..2a0a90d 100644
--- a/Source/WebCore/html/canvas/WebGLRenderingContext.h
+++ b/Source/WebCore/html/canvas/WebGLRenderingContext.h
@@ -329,10 +329,6 @@ public:
 
     void addObject(WebGLObject*);
     void detachAndRemoveAllObjects();
-    WebGLTexture* findTexture(Platform3DObject);
-    WebGLRenderbuffer* findRenderbuffer(Platform3DObject);
-    WebGLBuffer* findBuffer(Platform3DObject);
-    WebGLShader* findShader(Platform3DObject);
 
     void markContextChanged();
     void cleanupAfterGraphicsCall(bool changed)
@@ -388,7 +384,7 @@ public:
 
     bool m_needsUpdate;
     bool m_markedCanvasDirty;
-    HashSet<RefPtr<WebGLObject> > m_canvasObjects;
+    HashSet<WebGLObject*> m_canvasObjects;
 
     // List of bound VBO's. Used to maintain info about sizes for ARRAY_BUFFER and stored values for ELEMENT_ARRAY_BUFFER
     RefPtr<WebGLBuffer> m_boundArrayBuffer;