Command buffer: feedback loop detection for CopyTexSubImage3D

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 1335 matching lines @@
   void RestoreClearState();
 
   // Remembers the state of some capabilities.
   // Returns: true if glEnable/glDisable should actually be called.
   bool SetCapabilityState(GLenum cap, bool enabled);
 
   // Infer color encoding from internalformat
   static GLint GetColorEncodingFromInternalFormat(GLenum internalformat);
 
   // Check that the currently bound read framebuffer's color image
-  // isn't the target texture of the glCopyTex{Sub}Image2D.
-  bool FormsTextureCopyingFeedbackLoop(TextureRef* texture, GLint level);
+  // isn't the target texture of the glCopyTex{Sub}Image{2D|3D}.
+  bool FormsTextureCopyingFeedbackLoop(
+      TextureRef* texture,
+      GLint level,
+      GLint layer);
 
   // Check if a framebuffer meets our requirements.
   // Generates |gl_error| if the framebuffer is incomplete.
   bool CheckFramebufferValid(
       Framebuffer* framebuffer,
       GLenum target,
       GLenum gl_error,
       const char* func_name);
 
   bool CheckBoundDrawFramebufferValid(const char* func_name);
@@ skipping 2867 matching lines @@
     // behave correctly.
     bool enable_framebuffer_srgb =
         framebuffer && framebuffer->HasSRGBAttachments();
     state_.EnableDisableFramebufferSRGB(enable_framebuffer_srgb);
   }
   return valid;
 }
 
 bool GLES2DecoderImpl::CheckBoundReadFramebufferValid(
     const char* func_name, GLenum gl_error) {
-  GLenum target = features().chromium_framebuffer_multisample ?
-      GL_READ_FRAMEBUFFER : GL_FRAMEBUFFER;
+  GLenum target = GL_FRAMEBUFFER;
+  if (feature_info_->IsWebGL2OrES3Context() ||

[qiankun, 2016/07/28 13:59:31]

Can you explain why webgl2 or es3 are different here? 

[Zhenyao Mo, 2016/07/28 14:51:37]

I don't think this is necessary.  chromium_framebuffer_multisample has to be
true if it's WebGL2 or ES3.

[yunchao, 2016/07/28 15:53:05]

Got it.

[yunchao, 2016/07/28 15:53:06]

For read buffer, we should get info from GL_READ_FRAMEBUFFER for webgl 2 or es3,
instead of GL_FRAMEBUFFER(Which is identical to GL_DRAW_FRAMEBUFFER). 
However, as Zhenyao pointed out, the chromium_framebuffer_multisample should be
true for webgl2 or es3 context. 

+      features().chromium_framebuffer_multisample) {
+    target = GL_READ_FRAMEBUFFER;
+  }
   Framebuffer* framebuffer = GetFramebufferInfoForTarget(target);
   bool valid = CheckFramebufferValid(
       framebuffer, target, gl_error, func_name);
   return valid;
 }
 
 bool GLES2DecoderImpl::CheckBoundFramebufferValid(const char* func_name) {
   GLenum target = features().chromium_framebuffer_multisample ?
       GL_DRAW_FRAMEBUFFER : GL_FRAMEBUFFER;
   Framebuffer* draw_framebuffer = GetFramebufferInfoForTarget(target);
@@ skipping 23 matching lines @@
     case GL_SRGB_ALPHA_EXT:
     case GL_SRGB8:
     case GL_SRGB8_ALPHA8:
       return GL_SRGB;
     default:
       return GL_LINEAR;
   }
 }
 
 bool GLES2DecoderImpl::FormsTextureCopyingFeedbackLoop(
-    TextureRef* texture, GLint level) {
-  Framebuffer* framebuffer = features().chromium_framebuffer_multisample ?
-      framebuffer_state_.bound_read_framebuffer.get() :
-      framebuffer_state_.bound_draw_framebuffer.get();
+    TextureRef* texture, GLint level, GLint layer) {
+  bool is_webgl2_or_es3 = feature_info_->IsWebGL2OrES3Context();
+  Framebuffer* framebuffer = NULL;

[qiankun, 2016/07/28 13:59:31]

Use nullptr.

+  if (is_webgl2_or_es3) {
+    framebuffer = GetFramebufferInfoForTarget(GL_READ_FRAMEBUFFER_EXT);

[Zhenyao Mo, 2016/07/28 14:51:37]

I don't think it's necessary.  is_webgl2_or_es3 is always true if
features().chromium_framebuffer_multisample is true. The original code is
enough.

[yunchao, 2016/07/28 15:53:06]

Done.

+  } else {
+    framebuffer = features().chromium_framebuffer_multisample ?
+        framebuffer_state_.bound_read_framebuffer.get() :
+        framebuffer_state_.bound_draw_framebuffer.get();
+  }
   if (!framebuffer)
     return false;
-  const Framebuffer::Attachment* attachment = framebuffer->GetAttachment(
-      GL_COLOR_ATTACHMENT0);
+  const Framebuffer::Attachment* attachment = is_webgl2_or_es3 ?

[Zhenyao Mo, 2016/07/28 14:51:37]

This is unnecessary.  In ES2 READ_BUFFER is just COLOR_ATTACHMENT0.

[yunchao, 2016/07/28 15:04:04]

Why here is not necessary, Zhenyao? I don't understand. What if we set read
buffer by readBuffer() to other attachment instead of COLOR_ATTACHMENT0? 
You can also see the code GLES2DecoderImpl::DoReadBuffer() ->
FramebufferManager::set_read_buffer(). And
Framebuffer::GetReadBufferAttachment() will get the correct read attachment when
binding to a fbo.

Well, I know that in ES2 or WebGL 1, the read buffer is definitely
COLOR_ATTACHMENT0. But you know, for WebGL 2 or ES3, we have readBuffer() API to
change the read buffer attachment. 

[yunchao, 2016/07/28 15:06:05]

Sorry, I got what you mean. We can only use
framebuffer->GetReadBufferAttachment() to cover all situations. 

[Zhenyao Mo, 2016/07/28 15:06:38]

What I mean is you can always call framebuffer->GetReadBufferAttachment().

[yunchao, 2016/07/28 15:53:06]

Yeah.

[yunchao, 2016/07/28 15:53:06]

Yeah.

+      framebuffer->GetReadBufferAttachment() :
+      framebuffer->GetAttachment(GL_COLOR_ATTACHMENT0);
   if (!attachment)
     return false;
-  return attachment->FormsFeedbackLoop(texture, level);
+  return attachment->FormsFeedbackLoop(texture, level, layer);
 }
 
 gfx::Size GLES2DecoderImpl::GetBoundReadFrameBufferSize() {
   Framebuffer* framebuffer =
       GetFramebufferInfoForTarget(GL_READ_FRAMEBUFFER_EXT);
   if (framebuffer != NULL) {
     const Framebuffer::Attachment* attachment =
         framebuffer->GetReadBufferAttachment();
     if (attachment) {
       return gfx::Size(attachment->width(), attachment->height());
@@ skipping 8694 matching lines @@
       state_.unpack_alignment, &pixels_size, NULL, NULL)) {
     LOCAL_SET_GL_ERROR(GL_OUT_OF_MEMORY, func_name, "dimensions too large");
     return;
   }
 
   if (!EnsureGPUMemoryAvailable(pixels_size)) {
     LOCAL_SET_GL_ERROR(GL_OUT_OF_MEMORY, func_name, "out of memory");
     return;
   }
 
-  if (FormsTextureCopyingFeedbackLoop(texture_ref, level)) {
+  if (FormsTextureCopyingFeedbackLoop(texture_ref, level, 0)) {
     LOCAL_SET_GL_ERROR(
         GL_INVALID_OPERATION,
         func_name, "source and destination textures are the same");
     return;
   }
 
   LOCAL_COPY_REAL_GL_ERRORS_TO_WRAPPER(func_name);
   ScopedResolvedFrameBufferBinder binder(this, false, true);
   gfx::Size size = GetBoundReadFrameBufferSize();
 
@@ skipping 153 matching lines @@
     return;
   }
 
   GLenum read_format = GetBoundReadFrameBufferInternalFormat();
   GLenum read_type = GetBoundReadFrameBufferTextureType();
   if (!ValidateCopyTexFormat(func_name, internal_format,
                              read_format, read_type)) {
     return;
   }
 
-  if (FormsTextureCopyingFeedbackLoop(texture_ref, level)) {
+  if (FormsTextureCopyingFeedbackLoop(texture_ref, level, 0)) {
     LOCAL_SET_GL_ERROR(
         GL_INVALID_OPERATION,
         func_name, "source and destination textures are the same");
     return;
   }
 
   ScopedResolvedFrameBufferBinder binder(this, false, true);
   gfx::Size size = GetBoundReadFrameBufferSize();
   GLint copyX = 0;
   GLint copyY = 0;
@@ skipping 86 matching lines @@
     return;
   }
 
   GLenum read_format = GetBoundReadFrameBufferInternalFormat();
   GLenum read_type = GetBoundReadFrameBufferTextureType();
   if (!ValidateCopyTexFormat(func_name, internal_format,
                              read_format, read_type)) {
     return;
   }
 
-  // TODO(yunchao): Follow-up CLs are necessary. For instance, feedback loop
-  // detection, emulation of unsized formats in core profile, clear the 3d
-  // textures if it is uncleared, out-of-bounds reading, etc.
+  if (FormsTextureCopyingFeedbackLoop(texture_ref, level, zoffset)) {
+    LOCAL_SET_GL_ERROR(
+        GL_INVALID_OPERATION,
+        func_name, "source and destination textures are the same");
+    return;
+  }
+
+  // TODO(yunchao): Follow-up CLs are necessary. For instance:
+  // 1. emulation of unsized formats in core profile
+  // 2. clear the 3d textures if it is uncleared.
+  // 3. out-of-bounds reading, etc.
 
   glCopyTexSubImage3D(target, level, xoffset, yoffset, zoffset, x, y, width,
                       height);
 
   // This may be a slow command.  Exit command processing to allow for
   // context preemption and GPU watchdog checks.
   ExitCommandProcessingEarly();
 }
 
 error::Error GLES2DecoderImpl::HandleTexSubImage2D(uint32_t immediate_data_size,
@@ skipping 4444 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