Set attachment for bound framebuffer in FramebufferTextureLayer

third_party/WebKit/Source/modules/webgl/WebGLFramebuffer.cpp

Index: third_party/WebKit/Source/modules/webgl/WebGLFramebuffer.cpp
diff --git a/third_party/WebKit/Source/modules/webgl/WebGLFramebuffer.cpp b/third_party/WebKit/Source/modules/webgl/WebGLFramebuffer.cpp
index 0bc636e62e9c9dc296f709fb1be1657b783ff60c..65e85127bf306175a4b8664ad966d61f8f40a36e 100644
--- a/third_party/WebKit/Source/modules/webgl/WebGLFramebuffer.cpp
+++ b/third_party/WebKit/Source/modules/webgl/WebGLFramebuffer.cpp
@@ -146,12 +146,12 @@ GLenum WebGLRenderbufferAttachment::type() const
 
 class WebGLTextureAttachment final : public WebGLFramebuffer::WebGLAttachment {
 public:
-    static WebGLFramebuffer::WebGLAttachment* create(WebGLTexture*, GLenum target, GLint level);
+    static WebGLFramebuffer::WebGLAttachment* create(WebGLTexture*, GLenum target, GLint level, GLint layer);
 
     DECLARE_VIRTUAL_TRACE();
 
 private:
-    WebGLTextureAttachment(WebGLTexture*, GLenum target, GLint level);
+    WebGLTextureAttachment(WebGLTexture*, GLenum target, GLint level, GLint layer);
     WebGLTextureAttachment() { }
 
     GLsizei width() const override;
@@ -168,11 +168,12 @@ private:
     Member<WebGLTexture> m_texture;
     GLenum m_target;
     GLint m_level;
+    GLint m_layer;
 };
 
-WebGLFramebuffer::WebGLAttachment* WebGLTextureAttachment::create(WebGLTexture* texture, GLenum target, GLint level)
+WebGLFramebuffer::WebGLAttachment* WebGLTextureAttachment::create(WebGLTexture* texture, GLenum target, GLint level, GLint layer)
 {
-    return new WebGLTextureAttachment(texture, target, level);
+    return new WebGLTextureAttachment(texture, target, level, layer);
 }
 
 DEFINE_TRACE(WebGLTextureAttachment)
@@ -181,10 +182,11 @@ DEFINE_TRACE(WebGLTextureAttachment)
     WebGLFramebuffer::WebGLAttachment::trace(visitor);
 }
 
-WebGLTextureAttachment::WebGLTextureAttachment(WebGLTexture* texture, GLenum target, GLint level)
+WebGLTextureAttachment::WebGLTextureAttachment(WebGLTexture* texture, GLenum target, GLint level, GLint layer)
     : m_texture(texture)
     , m_target(target)
     , m_level(level)
+    , m_layer(layer)
 {
 }
 
@@ -226,16 +228,24 @@ void WebGLTextureAttachment::onDetached(WebGraphicsContext3D* context)
 void WebGLTextureAttachment::attach(WebGraphicsContext3D* context, GLenum target, GLenum attachment)
 {
     Platform3DObject object = objectOrZero(m_texture.get());
-    context->framebufferTexture2D(target, attachment, m_target, object, m_level);
+    if (target == GL_TEXTURE_3D || target == GL_TEXTURE_2D_ARRAY) {
+        context->framebufferTextureLayer(target, attachment, object, m_level, m_layer);

[Ken Russell (switch to Gerrit), 2015/10/27 16:47:59]

The logic here has become really confusing. I am highly dubious about the
correctness of the two overloads of "removeAttachmentFromBoundFramebuffer". It
looks to me like during calls like framebufferRenderbuffer and
framebufferTexture2D that duplicate work, at least, is being done -- two calls
to WebGraphicsContext3D::framebufferRenderbuffer, one from
WebGLRenderingContextBase::framebufferRenderbuffer and one from
WebGLRenderbufferAttachment::attach. zmo@, do you remember the motivation for
adding this code?

+    } else {
+        context->framebufferTexture2D(target, attachment, m_target, object, m_level);
+    }
 }
 
 void WebGLTextureAttachment::unattach(WebGraphicsContext3D* context, GLenum target, GLenum attachment)
 {
-    if (attachment == GL_DEPTH_STENCIL_ATTACHMENT) {
-        context->framebufferTexture2D(target, GL_DEPTH_ATTACHMENT, m_target, 0, m_level);
-        context->framebufferTexture2D(target, GL_STENCIL_ATTACHMENT, m_target, 0, m_level);
+    if (target == GL_TEXTURE_3D || target == GL_TEXTURE_2D_ARRAY) {
+        context->framebufferTextureLayer(target, attachment, 0, m_level, m_layer);

[Zhenyao Mo, 2015/10/27 21:21:31]

Here I think you need to handle the same DEPTH_STENCIL_ATTACHMENT situation.

[qiankun, 2015/10/27 23:25:27]

DEPTH_STENCIL_ATTACHMENT is supported in ES 3. Do we still need to handle it
with GL_DEPTH_ATTACHMENT and the GL_STENCIL_ATTACHMENT attachment? I intend to
submit a update   to fix this in another CL.

[Zhenyao Mo, 2015/10/27 23:40:28]

OK, then could you please add a comment that DEPTH_STENCIL_ATTACHMENT is valid
in ES3?

[qiankun, 2015/10/27 23:55:59]

Done.

     } else {
-        context->framebufferTexture2D(target, attachment, m_target, 0, m_level);
+        if (attachment == GL_DEPTH_STENCIL_ATTACHMENT) {
+            context->framebufferTexture2D(target, GL_DEPTH_ATTACHMENT, m_target, 0, m_level);
+            context->framebufferTexture2D(target, GL_STENCIL_ATTACHMENT, m_target, 0, m_level);
+        } else {
+            context->framebufferTexture2D(target, attachment, m_target, 0, m_level);
+        }
     }
 }
 
@@ -350,14 +360,14 @@ WebGLFramebuffer::~WebGLFramebuffer()
     detachAndDeleteObject();
 }
 
-void WebGLFramebuffer::setAttachmentForBoundFramebuffer(GLenum target, GLenum attachment, GLenum texTarget, WebGLTexture* texture, GLint level)
+void WebGLFramebuffer::setAttachmentForBoundFramebuffer(GLenum target, GLenum attachment, GLenum texTarget, WebGLTexture* texture, GLint level, GLint layer)
 {
     ASSERT(isBound(target));
     removeAttachmentFromBoundFramebuffer(target, attachment);
     if (!m_object)
         return;
     if (texture && texture->object()) {
-        m_attachments.add(attachment, WebGLTextureAttachment::create(texture, texTarget, level));
+        m_attachments.add(attachment, WebGLTextureAttachment::create(texture, texTarget, level, 0));
         drawBuffersIfNecessary(false);
         texture->onAttached();
     }