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

gpu/command_buffer/service/gles2_cmd_decoder.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "gpu/command_buffer/service/gles2_cmd_decoder.h"
 
 #include <limits.h>
 #include <stddef.h>
 #include <stdint.h>
 #include <stdio.h>
@@ skipping 7892 matching lines @@
   }
 
   GLsizei read_buffer_samples = GetBoundFramebufferSamples(GL_READ_FRAMEBUFFER);
   if (read_buffer_samples > 0 &&
       (srcX0 != dstX0 || srcY0 != dstY0 || srcX1 != dstX1 || srcY1 != dstY1)) {
     LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, func_name,
         "src framebuffer is multisampled, but src/dst regions are different");
     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,
   // draw_framebuffer doesn't change, then use the cached status.
   enum FeedbackLoopState {
     FeedbackLoopTrue,
     FeedbackLoopFalse,
     FeedbackLoopUnknown
   };
 
   FeedbackLoopState is_feedback_loop = FeedbackLoopUnknown;
   Framebuffer* read_framebuffer =
       framebuffer_state_.bound_read_framebuffer.get();
   Framebuffer* draw_framebuffer =
       framebuffer_state_.bound_draw_framebuffer.get();
   // If both read framebuffer and draw framebuffer are default framebuffer,
   // They always have identical image. Otherwise, if one of read framebuffer
   // and draw framebuffe is default framebuffer, but the other is fbo, they
   // always have no identical image.
   if (!read_framebuffer && !draw_framebuffer) {
     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) {
       const Framebuffer::Attachment* depth_buffer_read =
           read_framebuffer->GetAttachment(GL_DEPTH_ATTACHMENT);
       const Framebuffer::Attachment* depth_buffer_draw =
           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;
       }
     }
     if ((mask & GL_STENCIL_BUFFER_BIT) != 0) {
       const Framebuffer::Attachment* stencil_buffer_read =
           read_framebuffer->GetAttachment(GL_STENCIL_ATTACHMENT);
       const Framebuffer::Attachment* stencil_buffer_draw =
           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;
   }
 
   GLenum src_internal_format = GetBoundReadFramebufferInternalFormat();
   GLenum src_type = GetBoundReadFramebufferTextureType();
 
   bool read_buffer_has_srgb =
       GetColorEncodingFromInternalFormat(src_internal_format) == GL_SRGB;
   bool draw_buffers_has_srgb = false;
   if ((mask & GL_COLOR_BUFFER_BIT) != 0) {
@@ skipping 14 matching lines @@
     DCHECK(read_framebuffer || (is_feedback_loop != FeedbackLoopUnknown));
     const Framebuffer::Attachment* read_buffer =
         is_feedback_loop == FeedbackLoopUnknown ?
         read_framebuffer->GetReadBufferAttachment() : nullptr;
     for (uint32_t ii = 0; ii < group_->max_draw_buffers(); ++ii) {
       GLenum dst_format = GetBoundColorDrawBufferInternalFormat(
           static_cast<GLint>(ii));
       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 &&
           (src_sized_format !=
            GLES2Util::ConvertToSizedFormat(dst_format, dst_type))) {
         LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, func_name,
                            "src and dst formats differ for color");
         return;
       }
       bool is_dst_signed_int = GLES2Util::IsSignedIntegerFormat(dst_format);
@@ skipping 19 matching lines @@
           break;
         }
       }
     }
   }
   if (is_feedback_loop == FeedbackLoopTrue) {
     LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, func_name,
         "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) {
       LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, func_name,
                          "invalid filter for depth/stencil");
       return;
     }
 
     if ((GetBoundFramebufferDepthFormat(GL_READ_FRAMEBUFFER) !=
          GetBoundFramebufferDepthFormat(GL_DRAW_FRAMEBUFFER)) ||
@@ skipping 10927 matching lines @@
 }
 
 // Include the auto-generated part of this file. We split this because it means
 // we can easily edit the non-auto generated parts right here in this file
 // instead of having to edit some template or the code generator.
 #include "base/macros.h"
 #include "gpu/command_buffer/service/gles2_cmd_decoder_autogen.h"
 
 }  // namespace gles2
 }  // namespace gpu