[Command buffer] Feedback loop detection between texture and framebuffer attachments

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 1304 matching lines @@
   // -1. If the current program is not valid generates the appropriate GL
   // error. Returns true if the current program is in a usable state and
   // location is not -1.
   bool CheckCurrentProgramForUniform(GLint location, const char* function_name);
 
   // Checks if the current program samples a texture that is also the color
   // image of the current bound framebuffer, i.e., the source and destination
   // of the draw operation are the same.
   bool CheckDrawingFeedbackLoops();
 
+  // Helper for CheckDrawingFeedbackLoops. Returns true if the attachment is
+  // the same one where it samples from during drawing.
+  bool CheckDrawingFeedbackLoopsHelper(
+    const Framebuffer::Attachment* attachment);
+
   bool SupportsDrawBuffers() const;
 
   // Checks if a draw buffer's format and its corresponding fragment shader
   // output's type are compatible, i.e., a signed integer typed variable is
   // incompatible with a float or unsigned integer buffer.
   // If incompaticle, generates an INVALID_OPERATION to avoid undefined buffer
   // contents and return false.
   // Otherwise, filter out the draw buffers that are not written to but are not
   // NONE through DrawBuffers, to be on the safe side. Return true.
   bool ValidateAndAdjustDrawBuffers(const char* function_name);
@@ skipping 7304 matching lines @@
 
 bool GLES2DecoderImpl::CheckCurrentProgramForUniform(
     GLint location, const char* function_name) {
   if (!CheckCurrentProgram(function_name)) {
     return false;
   }
   return !state_.current_program->IsInactiveUniformLocationByFakeLocation(
       location);
 }
 
-bool GLES2DecoderImpl::CheckDrawingFeedbackLoops() {
-  Framebuffer* framebuffer = GetFramebufferInfoForTarget(GL_FRAMEBUFFER);
-  if (!framebuffer)
-    return false;
-  const Framebuffer::Attachment* attachment =
-      framebuffer->GetAttachment(GL_COLOR_ATTACHMENT0);
-  if (!attachment)
-    return false;
-
+bool GLES2DecoderImpl::CheckDrawingFeedbackLoopsHelper(
+    const Framebuffer::Attachment* attachment) {
   DCHECK(state_.current_program.get());
   const Program::SamplerIndices& sampler_indices =
       state_.current_program->sampler_indices();
   for (size_t ii = 0; ii < sampler_indices.size(); ++ii) {
     const Program::UniformInfo* uniform_info =
         state_.current_program->GetUniformInfo(sampler_indices[ii]);
     DCHECK(uniform_info);
     for (size_t jj = 0; jj < uniform_info->texture_units.size(); ++jj) {
       GLuint texture_unit_index = uniform_info->texture_units[jj];
       if (texture_unit_index >= state_.texture_units.size())
         continue;
       TextureUnit& texture_unit = state_.texture_units[texture_unit_index];
       TextureRef* texture_ref =
           texture_unit.GetInfoForSamplerType(uniform_info->type).get();
       if (attachment->IsTexture(texture_ref))
         return true;
     }
   }
   return false;
 }
 
+bool GLES2DecoderImpl::CheckDrawingFeedbackLoops() {
+  if (feature_info_->IsWebGL1OrES2Context()) {

[Zhenyao Mo, 2016/10/31 18:03:46]

We are able to expose MRT in WebGL1 through extension, so you will need to
consider that possibility.

[yunchao, 2016/11/04 15:30:01]

Done.

+    Framebuffer* framebuffer = GetFramebufferInfoForTarget(GL_FRAMEBUFFER);
+    if (!framebuffer)
+      return false;
+    const Framebuffer::Attachment* attachment =
+        framebuffer->GetAttachment(GL_COLOR_ATTACHMENT0);
+    if (!attachment)
+      return false;
+    return CheckDrawingFeedbackLoopsHelper(attachment);
+  } else {
+    Framebuffer* framebuffer = GetFramebufferInfoForTarget(GL_DRAW_FRAMEBUFFER);
+    if (!framebuffer)
+      return false;
+    for (uint32_t ii = 0; ii < group_->max_draw_buffers(); ++ii) {

[piman, 2016/11/02 02:33:57]

Do we need to handle depth attachment vs depth texture too? (and ditto stencil).

[yunchao, 2016/11/04 15:30:01]

I think so.

+      GLenum drawbuffer = static_cast<GLenum>(GL_DRAW_BUFFER0 + ii);
+      if (framebuffer->GetDrawBuffer(drawbuffer) == GL_NONE)
+        continue;
+      GLenum attachment = static_cast<GLenum>(GL_COLOR_ATTACHMENT0 + ii);
+      const Framebuffer::Attachment* draw_buffer =
+          framebuffer->GetAttachment(attachment);
+      if (!draw_buffer)
+        continue;
+      if (CheckDrawingFeedbackLoopsHelper(draw_buffer))

[Zhenyao Mo, 2016/10/31 18:03:46]

We will want to optimize on this.  This is not trivia and we should be able to
avoid doing it per draw call. i.e., we should be able to only do it when related
states changed, including drawBuffers, framebufferTex calls, etc.

[yunchao, 2016/11/01 15:25:22]

The cache info is not easy if we try to optimize on this, we need to cache lots
of infos. In addition, if we cache the info, we still need to either 1) iterate
and calculate the current drawbuffers mask (to compare), iterate the framebuffer
attachments masks, iterate the active texture/sampler binding points masks, or
2) we set dirty states for all these related call sites. The code will be much
more complicated to detect feedback loop. If anyone is dirty, we still need to
do the validation.

For the current code, I think it will early 'continue' for common cases. we
still only have COLOR_ATTACHMENT0 as draw buffer for the majority of draw calls.
Even for MRT, most of the for loop will early 'continue' too in
CheckDrawFeedbackLoopsHelper(). 

WDYT, Zhenyao?

If you still think the optimization can bring in a great improvement against
current implementation, I will have a try. 

[Zhenyao Mo, 2016/11/01 22:55:46]

I think we just need one state, indicating if a feedback loop exists between
drawing dst / sampling src.

Then we could do some rough dirty-bit updating:

1) At program link succeed time, gather which texture binding points are used by
this program, i.e., a set of (active_texture, sampler_type)
2) set dirty flag to the state if the following happens
* drawBuffers()
* framebufferTexture{2|3}D (if the binding point is in set from step 1)
* changing of the current program (useProgram that changed the program,
linkProgram of the currently used program)
* bindTexture (which changed the binding) if the binding point is in set from
step 1)

[yunchao, 2016/11/04 15:30:01]

That's a pretty good suggestion. I will update the code here and find out a good
solution after I go through all undefined behavior in ES3 spec.

+        return true;
+    }
+    return false;
+  }
+}
+
 bool GLES2DecoderImpl::SupportsDrawBuffers() const {
   return feature_info_->IsWebGL1OrES2Context() ?
       feature_info_->feature_flags().ext_draw_buffers : true;
 }
 
 bool GLES2DecoderImpl::ValidateAndAdjustDrawBuffers(const char* func_name) {
   if (!SupportsDrawBuffers()) {
     return true;
   }
   Framebuffer* framebuffer = framebuffer_state_.bound_draw_framebuffer.get();
@@ skipping 10080 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