command_buffer: correctly use glGetFramebufferAttachmentParameter

gpu/command_buffer/service/gles2_cmd_decoder.cc

Index: gpu/command_buffer/service/gles2_cmd_decoder.cc
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index a2fa8a4ce0cc9c61b801b577cec5a758ad9a360d..8d8f1438729197c863b7d10e628636af53b9bcd8 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -6067,9 +6067,11 @@ bool GLES2DecoderImpl::GetHelper(
           if (framebuffer->HasAlphaMRT() &&
               framebuffer->HasSameInternalFormatsMRT()) {
             if (feature_info_->gl_version_info().is_desktop_core_profile) {
-              glGetFramebufferAttachmentParameterivEXT(
-                  GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
-                  GL_FRAMEBUFFER_ATTACHMENT_ALPHA_SIZE, &v);
+              if (framebuffer->HasColorAttachment(0)) {

[Zhenyao Mo, 2016/07/04 23:16:06]

Here we have a pre-existing issue: we should query any attachment since they all
have the same format.  Here we limit to COLOR_ATTACHMENT0, but we don't have to
have an image attached to COLOR_ATTACHMENT0.

+                glGetFramebufferAttachmentParameterivEXT(
+                    GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
+                    GL_FRAMEBUFFER_ATTACHMENT_ALPHA_SIZE, &v);
+              }
             } else {
               glGetIntegerv(GL_ALPHA_BITS, &v);
             }
@@ -6087,7 +6089,7 @@ bool GLES2DecoderImpl::GetHelper(
         if (feature_info_->gl_version_info().is_desktop_core_profile) {
           Framebuffer* framebuffer =
               GetFramebufferInfoForTarget(GL_DRAW_FRAMEBUFFER_EXT);
-          if (framebuffer) {
+          if (framebuffer && framebuffer->HasDepthAttachment()) {

[Zhenyao Mo, 2016/07/04 23:16:06]

This should be an inter if statement.  Because if framebuffer is not null, and
you don't have depth attachment, you should return 0 (already set by default)
rather than querying the back_buffer.

             glGetFramebufferAttachmentParameterivEXT(
                 GL_FRAMEBUFFER, GL_DEPTH_ATTACHMENT,
                 GL_FRAMEBUFFER_ATTACHMENT_DEPTH_SIZE, &v);
@@ -6109,7 +6111,7 @@ bool GLES2DecoderImpl::GetHelper(
         if (feature_info_->gl_version_info().is_desktop_core_profile) {
           Framebuffer* framebuffer =
               GetFramebufferInfoForTarget(GL_DRAW_FRAMEBUFFER_EXT);
-          if (framebuffer) {
+          if (framebuffer && framebuffer->HasColorAttachment(0)) {

[Zhenyao Mo, 2016/07/04 23:16:06]

Here we should do similar things as GL_ALPHA_BITS, checking color formats are
the same, checking at least one color attachment exists, and put this condition
as an inner loop.

             GLenum framebuffer_enum = 0;
             switch (pname) {
               case GL_RED_BITS:
@@ -6140,7 +6142,7 @@ bool GLES2DecoderImpl::GetHelper(
         if (feature_info_->gl_version_info().is_desktop_core_profile) {
           Framebuffer* framebuffer =
               GetFramebufferInfoForTarget(GL_DRAW_FRAMEBUFFER_EXT);
-          if (framebuffer) {
+          if (framebuffer && framebuffer->HasStencilAttachment()) {

[Zhenyao Mo, 2016/07/04 23:16:06]

Same here, should be an inner if statement.

             glGetFramebufferAttachmentParameterivEXT(
                 GL_FRAMEBUFFER, GL_STENCIL_ATTACHMENT,
                 GL_FRAMEBUFFER_ATTACHMENT_STENCIL_SIZE, &v);