Attaching an image to many color attachment points is unsupported

gpu/command_buffer/service/framebuffer_manager.cc

Index: gpu/command_buffer/service/framebuffer_manager.cc
diff --git a/gpu/command_buffer/service/framebuffer_manager.cc b/gpu/command_buffer/service/framebuffer_manager.cc
index cec5b104b7ab678b71a164d4da999293e99df8a8..1117ffb1d1364bbaa97ae89414226646f2402698 100644
--- a/gpu/command_buffer/service/framebuffer_manager.cc
+++ b/gpu/command_buffer/service/framebuffer_manager.cc
@@ -89,6 +89,15 @@ class RenderbufferAttachment
     return renderbuffer_.get() == renderbuffer;
   }
 
+  bool IsSameAttachment(const Attachment* attachment) const override {
+    if (attachment->IsRenderbufferAttachment()) {
+      const RenderbufferAttachment* other =
+          reinterpret_cast<const RenderbufferAttachment*>(attachment);
+      return IsRenderbuffer(other->renderbuffer());
+    }
+    return false;
+  }
+
   bool Is3D() const override { return false; }
 
   bool CanRenderTo(const FeatureInfo*) const override { return true; }
@@ -220,6 +229,19 @@ class TextureAttachment
     return texture == texture_ref_.get();
   }
 
+  bool IsSameAttachment(const Attachment* attachment) const override {
+    if (attachment->IsTextureAttachment()) {
+      const TextureAttachment* other =
+          reinterpret_cast<const TextureAttachment*>(attachment);
+      return IsTexture(other->texture()) &&
+          samples_ == other->samples() &&

[Zhenyao Mo, 2016/07/19 20:39:15]

samples should not be part of this. No matter what samples we use, it is still
the same image.

[qiankun, 2016/07/20 08:53:33]

Done.

+          layer_ == other->layer() &&
+          target_ == other->target() &&

[Zhenyao Mo, 2016/07/19 20:39:15]

This makes me think, can you add some test cases in the conformance test to make
sure if we attach different faces of the same cubemap level, or if we attach
different layer of the same 3D image level, they are allowed?

[qiankun, 2016/07/20 08:53:33]

I will add test for this.

+          level_ == other->level();
+    }
+    return false;
+  }
+
   bool IsRenderbuffer(Renderbuffer* /* renderbuffer */) const override {
     return false;
   }
@@ -423,6 +445,24 @@ bool Framebuffer::HasUnclearedColorAttachments() const {
   return false;
 }
 
+bool Framebuffer::HasDuplicateColorAttachments() const {
+  for (AttachmentMap::const_iterator it = attachments_.begin();
+       it != attachments_.end(); ++it) {
+    if (it->first >= GL_COLOR_ATTACHMENT0 &&
+        it->first < GL_COLOR_ATTACHMENT0 + manager_->max_draw_buffers_) {

[Zhenyao Mo, 2016/07/19 20:39:15]

max_color_attachments_

[qiankun, 2016/07/20 08:53:33]

That's fine.
I saw in WebGL 2.0 spec max_color_attachments_ must be equal to
max_draw_buffers. 
The usage of these two variables are mixed. See other places in this file.

+      const Attachment* attachment = it->second.get();
+      for (GLenum i = it->first + 1;
+           i < GL_COLOR_ATTACHMENT0 + manager_->max_draw_buffers_; i++) {

[Zhenyao Mo, 2016/07/19 20:39:15]

max_color_attachments_

+        const Attachment* other = GetAttachment(i);
+        if (other && attachment->IsSameAttachment(other)) {
+          return true;
+        }
+      }
+    }
+  }
+  return false;
+}
+
 bool Framebuffer::HasUnclearedIntRenderbufferAttachments() const {
   for (AttachmentMap::const_iterator it = attachments_.begin();
        it != attachments_.end(); ++it) {