Reland [Command buffer]: Generate INVALID_OPERATION if designated attachments in read fb miss image

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 ba6b5efb598496ebad28ea7a4c14c37be6442a83..38f6b5f166661888395a01c26dcf1aa5f44f02e5 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -7910,6 +7910,10 @@ void GLES2DecoderImpl::DoBlitFramebufferCHROMIUM(
     return;
   }
 
+  // Detect that designated read/depth/stencil buffer in read framebuffer miss
+  // image, and the corresponding buffers in draw framebuffer have image.
+  bool read_framebuffer_miss_image = false;
+
   // Check whether read framebuffer and draw framebuffer have identical image
   // TODO(yunchao): consider doing something like CheckFramebufferStatus().
   // We cache the validation results, and if read_framebuffer doesn't change,
@@ -7933,6 +7937,18 @@ void GLES2DecoderImpl::DoBlitFramebufferCHROMIUM(
     is_feedback_loop = FeedbackLoopTrue;
   } else if (!read_framebuffer || !draw_framebuffer) {
     is_feedback_loop = FeedbackLoopFalse;
+    if (read_framebuffer) {
+      if (((mask & GL_COLOR_BUFFER_BIT) != 0 &&
+          !GetBoundReadFramebufferInternalFormat()) ||
+          ((mask & GL_DEPTH_BUFFER_BIT) != 0 &&
+          !read_framebuffer->GetAttachment(GL_DEPTH_ATTACHMENT) &&
+          BoundFramebufferHasDepthAttachment()) ||

[Zhenyao Mo, 2016/12/13 19:21:37]

This is unnecessary.  This is under !read_framebuffer || !draw_framebuffer, but
red_framebuffer, so draw_framebuffer has to be null.

[yunchao, 2016/12/14 02:04:44]

That's true. draw_framebuffer should be null here. However, this code snippet is
designed for blitting to back buffer (browser top-window's framebuffer),
GLDE2DecoderImpl::BoundFramebufferDepthAttachment() will access to
back_buffer_has_depth/back_buffer_has_stencil finally. For color buffer, I think
the back buffer always has a color buffer, so It is not necessary to detect it.

For WebGL use caces, you know, it will not execute this code snippet. Because
its default framebuffer is still an internal fbo. The code is here for back
buffer. I suppose the behavior of back buffer should be aligned with fbo during
blitting when read buffer missing image. If it is not necessary, the code
snippet from line 7940 to line 7951 should be removed directly. WDYT? Zhenyao?

+          ((mask & GL_STENCIL_BUFFER_BIT) != 0 &&
+          !read_framebuffer->GetAttachment(GL_STENCIL_ATTACHMENT) &&

[Zhenyao Mo, 2016/12/13 19:21:37]

Same here.

+          BoundFramebufferHasStencilAttachment())) {
+        read_framebuffer_miss_image = true;
+      }
+    }
   } else {
     DCHECK(read_framebuffer && draw_framebuffer);
     if ((mask & GL_DEPTH_BUFFER_BIT) != 0) {
@@ -7942,6 +7958,9 @@ void GLES2DecoderImpl::DoBlitFramebufferCHROMIUM(
           draw_framebuffer->GetAttachment(GL_DEPTH_ATTACHMENT);
       if (!depth_buffer_draw || !depth_buffer_read) {
         mask &= ~GL_DEPTH_BUFFER_BIT;
+        if (depth_buffer_draw) {
+          read_framebuffer_miss_image = true;
+        }
       } else if (depth_buffer_draw->IsSameAttachment(depth_buffer_read)) {
         is_feedback_loop = FeedbackLoopTrue;
       }
@@ -7953,11 +7972,14 @@ void GLES2DecoderImpl::DoBlitFramebufferCHROMIUM(
           draw_framebuffer->GetAttachment(GL_STENCIL_ATTACHMENT);
       if (!stencil_buffer_draw || !stencil_buffer_read) {
         mask &= ~GL_STENCIL_BUFFER_BIT;
+        if (stencil_buffer_draw) {
+          read_framebuffer_miss_image = true;
+        }
       } else if (stencil_buffer_draw->IsSameAttachment(stencil_buffer_read)) {
         is_feedback_loop = FeedbackLoopTrue;
       }
     }
-    if (!mask)
+    if (!mask && !read_framebuffer_miss_image)
       return;
   }
 
@@ -7992,6 +8014,9 @@ void GLES2DecoderImpl::DoBlitFramebufferCHROMIUM(
       GLenum dst_type = GetBoundColorDrawBufferType(static_cast<GLint>(ii));
       if (dst_format == 0)
         continue;
+      if (!src_internal_format) {
+        read_framebuffer_miss_image = true;
+      }
       if (GetColorEncodingFromInternalFormat(dst_format) == GL_SRGB)
         draw_buffers_has_srgb = true;
       if (read_buffer_samples > 0 &&
@@ -8031,6 +8056,11 @@ void GLES2DecoderImpl::DoBlitFramebufferCHROMIUM(
         "source buffer and destination buffers are identical");
     return;
   }
+  if (read_framebuffer_miss_image == true) {
+    LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, func_name,
+       "The designated attachment point(s) in read framebuffer miss image");
+    return;
+  }
 
   if ((mask & (GL_DEPTH_BUFFER_BIT | GL_STENCIL_BUFFER_BIT)) != 0) {
     if (filter != GL_NEAREST) {