Validate fbo color image format and fragment shader output variable type.

gpu/command_buffer/service/framebuffer_manager.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/framebuffer_manager.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include "base/logging.h"
@@ skipping 358 matching lines @@
     FramebufferManager* manager, GLuint service_id)
     : manager_(manager),
       deleted_(false),
       service_id_(service_id),
       has_been_bound_(false),
       framebuffer_complete_state_count_id_(0),
       read_buffer_(GL_COLOR_ATTACHMENT0) {
   manager->StartTracking(this);
   DCHECK_GT(manager->max_draw_buffers_, 0u);
   draw_buffers_.reset(new GLenum[manager->max_draw_buffers_]);
+  adjusted_draw_buffers_.reset(new GLenum[manager->max_draw_buffers_]);
   draw_buffers_[0] = GL_COLOR_ATTACHMENT0;
-  for (uint32_t i = 1; i < manager->max_draw_buffers_; ++i)
-    draw_buffers_[i] = GL_NONE;
+  adjusted_draw_buffers_[0] = GL_COLOR_ATTACHMENT0;
+  for (uint32_t ii = 1; ii < manager->max_draw_buffers_; ++ii) {
+    draw_buffers_[ii] = GL_NONE;
+    adjusted_draw_buffers_[ii] = GL_NONE;
+  }
+
+  color_attachment_base_types_.reset(
+      new ShaderVariableBaseType[manager->max_draw_buffers_]);
+  ResetColorAttachmentBaseTypes();
 }
 
 Framebuffer::~Framebuffer() {
   if (manager_) {
     if (manager_->have_context_) {
       GLuint id = service_id();
       glDeleteFramebuffersEXT(1, &id);
     }
     manager_->StopTracking(this);
     manager_ = NULL;
   }
 }
 
+void Framebuffer::ResetColorAttachmentBaseTypes() {
+  for (size_t ii = 1; ii < manager_->max_draw_buffers_; ++ii) {
+    color_attachment_base_types_[ii] = SHADER_VARIABLE_UNDEFINED_TYPE;
+  }
+}
+
 bool Framebuffer::HasUnclearedAttachment(
     GLenum attachment) const {
   AttachmentMap::const_iterator it =
       attachments_.find(attachment);
   if (it != attachments_.end()) {
     const Attachment* attachment = it->second.get();
     return !attachment->cleared();
   }
   return false;
 }
@@ skipping 58 matching lines @@
       case GL_SRGB_EXT:
       case GL_SRGB_ALPHA_EXT:
         return true;
       default:
         break;
     }
   }
   return false;
 }
 
-bool Framebuffer::PrepareDrawBuffersForClear() const {
+bool Framebuffer::PrepareDrawBuffersForClearingUninitializedAttachments(
+    ) const {
   std::unique_ptr<GLenum[]> buffers(new GLenum[manager_->max_draw_buffers_]);
   for (uint32_t i = 0; i < manager_->max_draw_buffers_; ++i)
     buffers[i] = GL_NONE;
   for (AttachmentMap::const_iterator it = attachments_.begin();
        it != attachments_.end(); ++it) {
     if (it->first >= GL_COLOR_ATTACHMENT0 &&
         it->first < GL_COLOR_ATTACHMENT0 + manager_->max_draw_buffers_ &&
         !it->second->cleared()) {
       // There should be no partially cleared images, uncleared int/3d images.
       // This is because ClearUnclearedIntOr3DImagesOrPartiallyClearedImages()
       // is called before this.
       DCHECK(!GLES2Util::IsIntegerFormat(it->second->internal_format()));
       DCHECK(!it->second->IsPartiallyCleared());
       DCHECK(!it->second->Is3D());
       buffers[it->first - GL_COLOR_ATTACHMENT0] = it->first;
     }
   }
   bool different = false;
   for (uint32_t i = 0; i < manager_->max_draw_buffers_; ++i) {
-    if (buffers[i] != draw_buffers_[i]) {
+    if (buffers[i] != adjusted_draw_buffers_[i]) {
       different = true;
       break;
     }
   }
   if (different)
     glDrawBuffersARB(manager_->max_draw_buffers_, buffers.get());
   return different;
 }
 
-void Framebuffer::RestoreDrawBuffersAfterClear() const {
-  glDrawBuffersARB(manager_->max_draw_buffers_, draw_buffers_.get());
+void Framebuffer::RestoreDrawBuffers() const {
+  glDrawBuffersARB(manager_->max_draw_buffers_, adjusted_draw_buffers_.get());
+}
+
+bool Framebuffer::ValidateAndAdjustDrawBuffers(
+    const ShaderVariableBaseType* fragment_output_base_types) {
+  bool adjust = false;
+  for (size_t ii = 0; ii < manager_->max_draw_buffers_; ++ii) {

[piman, 2016/07/13 22:35:55]

This loop seems kind of expensive...
Truth is, we don't really handle more than 16 draw buffers in practice (since
validation disallows anything beyond GL_DRAW_BUFFER15), and there actually isn't
currently any GPU that handles more than 8 (according to
http://delphigl.de/glcapsviewer/gl_stats_caps_single.php?listreportsbycap=GL_...
)

I think we can make this significantly cheaper by coalescing the information
into bitfields, 2 bits per buffer (so 32 bits for the 16 buffers we support), so
that we can use bitwise logic operations to find whether the shader matches the
FBO:
- on the FBO side, one fbo_buffer_bound_mask for which the bit value is either
0x3 if the draw buffer is bound, 0x0 otherwise, and one fbo_buffer_type_mask
that contains the type (only relevant if the buffer is bound)
- on the shader side, similarly, shader_buffer_written_mask and
shader_buffer_type_mask

Then the logic for draw can be simplified to:
if (fbo_buffer_bound_mask & ~shader_buffer_written_mask)
  adjust = true;
if ((fbo_buffer_bound_mask & fbo_buffer_type_mask) != (fbo_buffer_bound_mask &
shader_buffer_type_mask))
  return false;

(and something equivalent for the clear case)

Then we only need a loop to generate the proper array for glDrawBuffers, in the
adjust case (which should be rare).

+    if (draw_buffers_[ii] == GL_NONE) {
+      DCHECK_EQ(static_cast<GLenum>(GL_NONE), adjusted_draw_buffers_[ii]);
+      continue;
+    }
+    if (color_attachment_base_types_[ii] == SHADER_VARIABLE_UNDEFINED_TYPE) {
+      if (adjusted_draw_buffers_[ii] != GL_NONE)
+        adjust = true;
+      continue;
+    }
+    if (fragment_output_base_types) {  // Draw call cases.
+      if (fragment_output_base_types[ii] == SHADER_VARIABLE_UNDEFINED_TYPE) {
+        if (adjusted_draw_buffers_[ii] != GL_NONE)
+          adjust = true;
+        continue;
+      }
+      if (fragment_output_base_types[ii] != color_attachment_base_types_[ii]) {
+        return false;
+      }
+    } else {  // Clear call cases.
+      if (color_attachment_base_types_[ii] != SHADER_VARIABLE_FLOAT) {
+        if (adjusted_draw_buffers_[ii] != GL_NONE)
+          adjust = true;
+        continue;
+      }
+    }
+  }
+  if (adjust) {
+    // This won't be reached in every draw/clear call - only when framebuffer
+    // or program has changed.
+    for (size_t ii = 0; ii < manager_->max_draw_buffers_; ++ii) {
+      if (draw_buffers_[ii] == GL_NONE)
+        continue;
+      if (color_attachment_base_types_[ii] == SHADER_VARIABLE_UNDEFINED_TYPE) {
+        adjusted_draw_buffers_[ii] = GL_NONE;
+        continue;
+      }
+      if (fragment_output_base_types) {  // Draw call cases.
+        if (fragment_output_base_types[ii] == SHADER_VARIABLE_UNDEFINED_TYPE) {
+          adjusted_draw_buffers_[ii] = GL_NONE;
+        }
+      } else {  // Clear call cases.
+        if (color_attachment_base_types_[ii] != SHADER_VARIABLE_FLOAT) {
+          adjusted_draw_buffers_[ii] = GL_NONE;
+        }
+      }
+    }
+    glDrawBuffersARB(manager_->max_draw_buffers_, adjusted_draw_buffers_.get());
+  }
+  return true;
 }
 
 void Framebuffer::ClearUnclearedIntOr3DTexturesOrPartiallyClearedTextures(
-    GLES2Decoder* decoder,
-    TextureManager* texture_manager) {
+    GLES2Decoder* decoder, TextureManager* texture_manager) {
   for (AttachmentMap::const_iterator it = attachments_.begin();
        it != attachments_.end(); ++it) {
     if (!it->second->IsTextureAttachment() || it->second->cleared())
       continue;
     TextureAttachment* attachment =
         reinterpret_cast<TextureAttachment*>(it->second.get());
     if (attachment->IsPartiallyCleared() || attachment->Is3D() ||
         GLES2Util::IsIntegerFormat(attachment->internal_format())) {
       texture_manager->ClearTextureLevel(decoder,
                                          attachment->texture(),
                                          attachment->target(),
                                          attachment->level());
     }
   }
 }
 
 void Framebuffer::MarkAttachmentAsCleared(
-      RenderbufferManager* renderbuffer_manager,
-      TextureManager* texture_manager,
-      GLenum attachment,
-      bool cleared) {
+    RenderbufferManager* renderbuffer_manager,
+    TextureManager* texture_manager,
+    GLenum attachment,
+    bool cleared) {
   AttachmentMap::iterator it = attachments_.find(attachment);
   if (it != attachments_.end()) {
     Attachment* a = it->second.get();
     if (a->cleared() != cleared) {
       a->SetCleared(renderbuffer_manager,
                     texture_manager,
                     cleared);
     }
   }
 }
 
 void Framebuffer::MarkAttachmentsAsCleared(
-      RenderbufferManager* renderbuffer_manager,
-      TextureManager* texture_manager,
-      bool cleared) {
+    RenderbufferManager* renderbuffer_manager,
+    TextureManager* texture_manager,
+    bool cleared) {
   for (AttachmentMap::iterator it = attachments_.begin();
        it != attachments_.end(); ++it) {
     Attachment* attachment = it->second.get();
     if (attachment->cleared() != cleared) {
       attachment->SetCleared(renderbuffer_manager, texture_manager, cleared);
     }
   }
 }
 
 bool Framebuffer::HasColorAttachment(int index) const {
@@ skipping 182 matching lines @@
 GLenum Framebuffer::GetDrawBuffer(GLenum draw_buffer) const {
   GLsizei index = static_cast<GLsizei>(
       draw_buffer - GL_DRAW_BUFFER0_ARB);
   CHECK(index >= 0 &&
         index < static_cast<GLsizei>(manager_->max_draw_buffers_));
   return draw_buffers_[index];
 }
 
 void Framebuffer::SetDrawBuffers(GLsizei n, const GLenum* bufs) {
   DCHECK(n <= static_cast<GLsizei>(manager_->max_draw_buffers_));
-  for (GLsizei i = 0; i < n; ++i)
+  for (GLsizei i = 0; i < n; ++i) {
     draw_buffers_[i] = bufs[i];
+    adjusted_draw_buffers_[i] = bufs[i];
+  }
 }
 
 bool Framebuffer::HasAlphaMRT() const {
   for (uint32_t i = 0; i < manager_->max_draw_buffers_; ++i) {
     if (draw_buffers_[i] != GL_NONE) {
       const Attachment* attachment = GetAttachment(draw_buffers_[i]);
       if (!attachment)
         continue;
       if ((GLES2Util::GetChannelsForFormat(
                attachment->internal_format()) & 0x0008) != 0)
@@ skipping 51 matching lines @@
         // TODO(gman): manually detach texture.
         // glFramebufferTexture2DEXT(target, it->first, GL_TEXTURE_2D, 0, 0);
         AttachTexture(it->first, NULL, GL_TEXTURE_2D, 0, 0);
         done = false;
         break;
       }
     }
   } while (!done);
 }
 
+void Framebuffer::UpdateColorAttachmentBaseTypes() {
+  ResetColorAttachmentBaseTypes();
+  for (AttachmentMap::const_iterator it = attachments_.begin();
+       it != attachments_.end(); ++it) {
+    if (it->first < GL_COLOR_ATTACHMENT0 ||
+        it->first >= GL_COLOR_ATTACHMENT0 + manager_->max_draw_buffers_) {

[piman, 2016/07/13 22:35:55]

this should be manager_->max_color_attachments_

+      continue;
+    }
+    size_t index = it->first - GL_COLOR_ATTACHMENT0;

[piman, 2016/07/13 22:35:55]

I think there's confusion about how color_attachment_base_types_ is indexed...
Here it's setup indexed by the color attachment index, but in
ValidateAndAdjustDrawBuffers,
it's looked up by draw buffer index.

I think it would make sense to keep the cache relative to the draw buffer index
- it would make the loop at draw time a bit cheaper. It means the loop in this
function needs to go over the draw buffers rather than the attachments. It means
that we have to run this again from SetDrawBuffers, but I think that's ok - I
prefer paying the cost at glDrawBuffers time rather than every draw call.

[piman, 2016/07/13 23:16:36]

Ok, so I now understand, this works in ES because it has an extra constraint
that draw_buffers[i] can only be GL_COLOR_ATTACHMENTi or GL_NONE (as opposed to
desktop GL where no such constraint exist).

Still, looping over the draw buffers instead of the attachments is a bit
simpler, and lets us do some of the processing (the GL_NONE case) ahead of time.

+    Attachment* attachment = it->second.get();
+    GLenum internal_format = attachment->internal_format();
+    if (GLES2Util::IsSignedIntegerFormat(internal_format)) {
+      color_attachment_base_types_[index] = SHADER_VARIABLE_INT;
+    } else if (GLES2Util::IsUnsignedIntegerFormat(internal_format)) {
+      color_attachment_base_types_[index] = SHADER_VARIABLE_UINT;
+    } else {
+      color_attachment_base_types_[index] = SHADER_VARIABLE_FLOAT;
+    }
+  }
+}
+
 Framebuffer* FramebufferManager::GetFramebuffer(
     GLuint client_id) {
   FramebufferMap::iterator it = framebuffers_.find(client_id);
   return it != framebuffers_.end() ? it->second.get() : NULL;
 }
 
 void FramebufferManager::RemoveFramebuffer(GLuint client_id) {
   FramebufferMap::iterator it = framebuffers_.find(client_id);
   if (it != framebuffers_.end()) {
     it->second->MarkAsDeleted();
@@ skipping 103 matching lines @@
 
 bool FramebufferManager::IsComplete(
     Framebuffer* framebuffer) {
   DCHECK(framebuffer);
   return framebuffer->framebuffer_complete_state_count_id() ==
       framebuffer_state_change_count_;
 }
 
 }  // namespace gles2
 }  // namespace gpu