WebGL 2: add read/write framebuffer binding points to related APIs

Source/core/html/canvas/WebGLFramebuffer.h

Index: Source/core/html/canvas/WebGLFramebuffer.h
diff --git a/Source/core/html/canvas/WebGLFramebuffer.h b/Source/core/html/canvas/WebGLFramebuffer.h
index f15ae11db68379276210d8d5091a95b5b065b57c..de819cda782ef57f054a1d86f6bd441e319fcd94 100644
--- a/Source/core/html/canvas/WebGLFramebuffer.h
+++ b/Source/core/html/canvas/WebGLFramebuffer.h
@@ -55,8 +55,8 @@ public:
         virtual bool isSharedObject(WebGLSharedObject*) const = 0;
         virtual bool valid() const = 0;
         virtual void onDetached(WebGraphicsContext3D*) = 0;
-        virtual void attach(WebGraphicsContext3D*, GLenum attachment) = 0;
-        virtual void unattach(WebGraphicsContext3D*, GLenum attachment) = 0;
+        virtual void attach(WebGraphicsContext3D*, GLenum target, GLenum attachment) = 0;
+        virtual void unattach(WebGraphicsContext3D*, GLenum target, GLenum attachment) = 0;
 
         DEFINE_INLINE_VIRTUAL_TRACE() { }
 
@@ -70,12 +70,12 @@ public:
 
     Platform3DObject object() const { return m_object; }
 
-    void setAttachmentForBoundFramebuffer(GLenum attachment, GLenum texTarget, WebGLTexture*, GLint level);
-    void setAttachmentForBoundFramebuffer(GLenum attachment, WebGLRenderbuffer*);
+    void setAttachmentForBoundFramebuffer(GLenum target, GLenum attachment, GLenum texTarget, WebGLTexture*, GLint level);
+    void setAttachmentForBoundFramebuffer(GLenum target, GLenum attachment, WebGLRenderbuffer*);
     // If an object is attached to the currently bound framebuffer, remove it.
-    void removeAttachmentFromBoundFramebuffer(WebGLSharedObject*);
+    void removeAttachmentFromBoundFramebuffer(WebGLSharedObject*, GLenum target = GL_FRAMEBUFFER);

[bajones, 2015/05/06 17:47:53]

I realize why you did it this way, because it cuts down on changes elsewhere,
but the fact that target is frequently the first argument but sometimes the last
is confusing and not very GL-like. I'd prefer to see target consistently be the
first argument for this function and others in this patch. That will prevent use
of default arguments, which makes the patch a bit more noisy, but I think the
uniformity will be worth it long term.

[yunchao, 2015/05/22 09:54:35]

Acknowledged.

     // If a given attachment point for the currently bound framebuffer is not null, remove the attached object.
-    void removeAttachmentFromBoundFramebuffer(GLenum);
+    void removeAttachmentFromBoundFramebuffer(GLenum, GLenum);
     WebGLSharedObject* getAttachmentObject(GLenum) const;
 
     GLenum colorBufferFormat() const;
@@ -84,13 +84,13 @@ public:
     // readPixels, copyTexImage2D, copyTexSubImage2D if this framebuffer is
     // currently bound.
     // Return false if the framebuffer is incomplete.
-    bool onAccess(WebGraphicsContext3D*, const char** reason);
+    bool onAccess(WebGraphicsContext3D*, const char** reason, GLenum target = GL_FRAMEBUFFER);
 
     // Software version of glCheckFramebufferStatus(), except that when
     // FRAMEBUFFER_COMPLETE is returned, it is still possible for
     // glCheckFramebufferStatus() to return FRAMEBUFFER_UNSUPPORTED,
     // depending on hardware implementation.
-    GLenum checkStatus(const char** reason) const;
+    GLenum checkStatus(const char** reason, GLenum target) const;
 
     bool hasEverBeenBound() const { return object() && m_hasEverBeenBound; }
 
@@ -112,14 +112,14 @@ protected:
     void deleteObjectImpl(WebGraphicsContext3D*) override;
 
 private:
-    WebGLAttachment* getAttachment(GLenum) const;
+    WebGLAttachment* getAttachment(GLenum attachment, GLenum target = GL_FRAMEBUFFER) const;
     bool isAttachmentComplete(WebGLAttachment* attachedObject, GLenum attachment, const char** reason) const;
 
     // Check if the framebuffer is currently bound.
     bool isBound() const;
 
     // attach 'attachment' at 'attachmentPoint'.
-    void attach(GLenum attachment, GLenum attachmentPoint);
+    void attach(GLenum target, GLenum attachment, GLenum attachmentPoint);
 
     // Check if a new drawBuffers call should be issued. This is called when we add or remove an attachment.
     void drawBuffersIfNecessary(bool force);
@@ -129,6 +129,7 @@ private:
     typedef WillBeHeapHashMap<GLenum, RefPtrWillBeMember<WebGLAttachment>> AttachmentMap;
 
     AttachmentMap m_attachments;
+    AttachmentMap m_readAttachments;

[Ken Russell (switch to Gerrit), 2015/05/06 21:17:33]

This is not how framebuffer attachments work. An individual framebuffer object
does not have separate "draw" and "read" attachments; there are separate binding
points for draw and read framebuffers in the GL state. Please study the OpenGL
ES 3.0 spec in more depth.

[yunchao, 2015/05/22 09:54:35]

Yes. I know that FBO doesn't have "read" and "draw" attachments. The problem is
that I give a bad name to this parameter. I have renamed it to
m_attachmentsForReadFramebuffer.

 
     bool m_hasEverBeenBound;