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 354 matching lines @@
   DCHECK(result.second);
 }
 
 Framebuffer::Framebuffer(
     FramebufferManager* manager, GLuint service_id)
     : manager_(manager),
       deleted_(false),
       service_id_(service_id),
       has_been_bound_(false),
       framebuffer_complete_state_count_id_(0),
+      draw_buffer_type_mask_(0u),
+      draw_buffer_bound_mask_(0u),
       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;
+  }
 }
 
 Framebuffer::~Framebuffer() {
   if (manager_) {
     if (manager_->have_context_) {
       GLuint id = service_id();
       glDeleteFramebuffersEXT(1, &id);
     }
     manager_->StopTracking(this);
     manager_ = NULL;
@@ skipping 71 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()) {
+  for (auto const& it : attachments_) {
+    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;
+      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(
+    uint32_t fragment_output_type_mask, uint32_t fragment_output_written_mask) {
+  uint32_t mask = draw_buffer_bound_mask_ & fragment_output_written_mask;
+  if ((mask & fragment_output_type_mask) != (mask & draw_buffer_type_mask_))
+    return false;
+
+  if (draw_buffer_bound_mask_ & ~fragment_output_written_mask) {
+    // This won't be reached in every draw/clear call - only when framebuffer
+    // or program has changed.
+    for (uint32_t ii = 0; ii < manager_->max_draw_buffers_; ++ii) {
+      adjusted_draw_buffers_[ii] = draw_buffers_[ii];
+      uint32_t shift_bits = ii * 2;
+      uint32_t mask = 0x33 << shift_bits;

[piman, 2016/07/14 23:41:14]

Did you mean 0x3 instead of 0x33?

[piman, 2016/07/14 23:41:14]

nit: this |mask| hides the variable on l.515 - it would be nice to avoid.

Actually, if you do, you can combine the 2 ifs below, say:
uint32_t buffer_mask = 0x3 << shift_bits;
if ((mask & buffer_mask) == 0u)
  adjusted_draw_buffers_[ii] = GL_NONE;

(you want to set adjusted_draw_buffers_[ii] to GL_NONE if either the buffer
isn't present or not written to, and that's exactly if the corresponding bits in
the outer |mask| are 0).

[Zhenyao Mo, 2016/07/15 02:41:15]

Done.

+      if ((draw_buffer_bound_mask_ & mask) == 0u) {
+        adjusted_draw_buffers_[ii] = GL_NONE;
+        continue;
+      }
+      if ((fragment_output_written_mask & mask) == 0u) {
+        adjusted_draw_buffers_[ii] = GL_NONE;
+      }
+    }
+    glDrawBuffersARB(manager_->max_draw_buffers_, adjusted_draw_buffers_.get());
+  }
+  return true;
+}
+
+bool Framebuffer::ContainsActiveIntegerAttachments() const {
+  // SHADER_VARIABLE_FLOAT is 0x3.

[piman, 2016/07/14 23:41:14]

nit: if you want to defend against this changing, you can use
0x55555555*SHADER_VARIABLE_FLOAT  (0x55555555 will broadcast the other value to
all slots).

[Zhenyao Mo, 2016/07/15 02:41:15]

Ha, nice trick.  Thanks.

+  uint32_t mask = 0xFFFFFFFF & draw_buffer_bound_mask_;
+  return draw_buffer_type_mask_ != mask;
 }
 
 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)
-    draw_buffers_[i] = bufs[i];
+  for (GLsizei ii = 0; ii < n; ++ii) {
+    draw_buffers_[ii] = bufs[ii];
+    adjusted_draw_buffers_[ii] = bufs[ii];
+  }
+  for (uint32_t ii = n; ii < manager_->max_draw_buffers_; ++ii) {
+    draw_buffers_[ii] = GL_NONE;
+    adjusted_draw_buffers_[ii] = GL_NONE;
+  }
+  UpdateDrawBufferMasks();
 }
 
 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::UpdateDrawBufferMasks() {
+  draw_buffer_type_mask_ = 0u;
+  draw_buffer_bound_mask_ = 0u;
+  for (GLenum ii = GL_COLOR_ATTACHMENT0;
+       ii < GL_COLOR_ATTACHMENT0 + manager_->max_color_attachments_; ++ii) {

[piman, 2016/07/14 23:41:14]

nit: how about: for (size_t index = 0; index < manager_->max_draw_buffers;
++index) {
  GLenum draw_buffer = draw_buffers_[index];
  if (draw_buffer == GL_NONE)
    continue;
  auto iter = attachments_.find(draw_buffer);
  [...]
}

It reads a little simpler, with no extraneous index math, and makes it clear
everything is indexed based on the draw buffer.

[Zhenyao Mo, 2016/07/15 02:41:15]

Done.

+    size_t index = ii - GL_COLOR_ATTACHMENT0;
+    if (draw_buffers_[index] == GL_NONE)
+      continue;
+    auto iter = attachments_.find(ii);
+    if (iter == attachments_.end())
+      continue;
+    scoped_refptr<Attachment> attachment = iter->second;
+    GLenum internal_format = attachment->internal_format();
+    ShaderVariableBaseType base_type = SHADER_VARIABLE_UNDEFINED_TYPE;
+    if (GLES2Util::IsSignedIntegerFormat(internal_format)) {
+      base_type = SHADER_VARIABLE_INT;
+    } else if (GLES2Util::IsUnsignedIntegerFormat(internal_format)) {
+      base_type = SHADER_VARIABLE_UINT;
+    } else {
+      base_type = SHADER_VARIABLE_FLOAT;
+    }
+    size_t shift_bits = index * 2;
+    draw_buffer_type_mask_ |= base_type << shift_bits;
+    draw_buffer_bound_mask_ |= 0x3 << shift_bits;
+  }
+}
+
 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