Make invalidateFramebuffer no-op for DEPTH_STENCIL attachment

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 5659 matching lines @@
         "glDisableVertexAttribArray", "index out of range");
   }
 }
 
 void GLES2DecoderImpl::InvalidateFramebufferImpl(
     GLenum target, GLsizei count, const GLenum* attachments,
     GLint x, GLint y, GLsizei width, GLsizei height,
     const char* function_name, FramebufferOperation op) {
   Framebuffer* framebuffer = GetFramebufferInfoForTarget(GL_FRAMEBUFFER);
 
+  // Because of performance issues, no-op if the format of the attachment is
+  // DEPTH_STENCIL and only one part is intended to be invalidated.
+  bool has_depth_stencil_format = framebuffer &&
+      framebuffer->HasDepthStencilFormatAttachment(GL_DEPTH_ATTACHMENT) &&
+      framebuffer->HasDepthStencilFormatAttachment(GL_STENCIL_ATTACHMENT);
+  bool invalidate_depth = false;
+  bool invalidate_stencil = false;
+  std::unique_ptr<GLenum[]> validated_attachments(new GLenum[count]);
+  GLsizei validated_count = 0;
+
   // Validates the attachments. If one of them fails, the whole command fails.
   GLenum thresh0 = GL_COLOR_ATTACHMENT0 + group_->max_color_attachments();
   GLenum thresh1 = GL_COLOR_ATTACHMENT15;
   for (GLsizei i = 0; i < count; ++i) {
     if (framebuffer) {
       if (attachments[i] >= thresh0 && attachments[i] <= thresh1) {
         LOCAL_SET_GL_ERROR(
             GL_INVALID_OPERATION, function_name, "invalid attachment");
         return;
       }
       if (!validators_->attachment.IsValid(attachments[i])) {
         LOCAL_SET_GL_ERROR_INVALID_ENUM(
             function_name, attachments[i], "attachments");
         return;
       }
+      if (has_depth_stencil_format) {
+        switch(attachments[i]) {
+          case GL_DEPTH_ATTACHMENT:
+            invalidate_depth = true;
+            continue;
+          case GL_STENCIL_ATTACHMENT:
+            invalidate_stencil = true;
+            continue;
+        }
+      }
     } else {
       if (!validators_->backbuffer_attachment.IsValid(attachments[i])) {
         LOCAL_SET_GL_ERROR_INVALID_ENUM(
             function_name, attachments[i], "attachments");
         return;
       }
     }
+    validated_attachments[validated_count++] = attachments[i];
+  }
+  if (invalidate_depth && invalidate_stencil) {
+    validated_attachments[validated_count++] = GL_DEPTH_STENCIL_ATTACHMENT;
   }
 
   // If the default framebuffer is bound but we are still rendering to an
   // FBO, translate attachment names that refer to default framebuffer
   // channels to corresponding framebuffer attachments.
-  std::unique_ptr<GLenum[]> translated_attachments(new GLenum[count]);
-  for (GLsizei i = 0; i < count; ++i) {
-    GLenum attachment = attachments[i];
+  std::unique_ptr<GLenum[]> translated_attachments(new GLenum[validated_count]);
+  for (GLsizei i = 0; i < validated_count; ++i) {
+    GLenum attachment = validated_attachments[i];
     if (!framebuffer && GetBackbufferServiceId()) {
       switch (attachment) {
         case GL_COLOR_EXT:
           attachment = GL_COLOR_ATTACHMENT0;
           break;
         case GL_DEPTH_EXT:
           attachment = GL_DEPTH_ATTACHMENT;
           break;
         case GL_STENCIL_EXT:
           attachment = GL_STENCIL_ATTACHMENT;
           break;
         default:
           NOTREACHED();
           return;
       }
     }
     translated_attachments[i] = attachment;
   }
 
   bool dirty = false;
   switch (op) {
     case kFramebufferDiscard:
       if (feature_info_->gl_version_info().is_es3) {
         glInvalidateFramebuffer(
-            target, count, translated_attachments.get());
+            target, validated_count, translated_attachments.get());
       } else {
         glDiscardFramebufferEXT(
-            target, count, translated_attachments.get());
+            target, validated_count, translated_attachments.get());
       }
       dirty = true;
       break;
     case kFramebufferInvalidate:
       if (feature_info_->gl_version_info().IsLowerThanGL(4, 3)) {
         // no-op since the function isn't supported.
       } else {
         glInvalidateFramebuffer(
-            target, count, translated_attachments.get());
+            target, validated_count, translated_attachments.get());
         dirty = true;
       }
       break;
     case kFramebufferInvalidateSub:
       // Make it an no-op because we don't have a mechanism to mark partial
       // pixels uncleared yet.
       // TODO(zmo): Revisit this.
       break;
   }
 
   if (!dirty)
     return;
 
   // Marks each one of them as not cleared.
-  for (GLsizei i = 0; i < count; ++i) {
+  for (GLsizei i = 0; i < validated_count; ++i) {
     if (framebuffer) {
-      framebuffer->MarkAttachmentAsCleared(renderbuffer_manager(),
-                                           texture_manager(),
-                                           attachments[i],
-                                           false);
+      if (validated_attachments[i] == GL_DEPTH_STENCIL_ATTACHMENT) {
+        framebuffer->MarkAttachmentAsCleared(renderbuffer_manager(),
+                                             texture_manager(),
+                                             GL_DEPTH_ATTACHMENT,
+                                             false);
+        framebuffer->MarkAttachmentAsCleared(renderbuffer_manager(),
+                                             texture_manager(),
+                                             GL_STENCIL_ATTACHMENT,
+                                             false);
+      } else {
+        framebuffer->MarkAttachmentAsCleared(renderbuffer_manager(),
+                                             texture_manager(),
+                                             validated_attachments[i],
+                                             false);
+      }
     } else {
-      switch (attachments[i]) {
+      switch (validated_attachments[i]) {
         case GL_COLOR_EXT:
           backbuffer_needs_clear_bits_ |= GL_COLOR_BUFFER_BIT;
           break;
         case GL_DEPTH_EXT:
           backbuffer_needs_clear_bits_ |= GL_DEPTH_BUFFER_BIT;
           break;
         case GL_STENCIL_EXT:
           backbuffer_needs_clear_bits_ |= GL_STENCIL_BUFFER_BIT;
           break;
         default:
@@ skipping 1216 matching lines @@
     glFramebufferRenderbufferEXT(
         target, GL_DEPTH_ATTACHMENT, renderbuffertarget, service_id);
     glFramebufferRenderbufferEXT(
         target, GL_STENCIL_ATTACHMENT, renderbuffertarget, service_id);
   } else {
     glFramebufferRenderbufferEXT(
         target, attachment, renderbuffertarget, service_id);
   }
   GLenum error = LOCAL_PEEK_GL_ERROR("glFramebufferRenderbuffer");
   if (error == GL_NO_ERROR) {
     framebuffer->AttachRenderbuffer(attachment, renderbuffer);

[qiankun, 2016/07/20 18:54:57]

If attachment is GL_DEPTH_STENCIL_ATTACHMENT, framebuffer will attach
renderbuffer to GL_DEPTH_STENCIL_ATTACHMENT here. has_depth_stencil_format check
in line 5682 will fail though framebuffer has attachment in depth_stencil
format.

[Zhenyao Mo, 2016/07/20 19:08:20]

Actually I think somewhere before reaching here, we already split DEPTH_STENCIL
into DEPTH and STENCIL. We should DCHECK in AttachRenderbuffer or AttachTexture
to make sure of that, but that's for another CL.  Let me dig a bit and find the
code.

[Zhenyao Mo, 2016/07/20 19:09:51]

Yeah, see GLES2DecoderImpl::DoFramebufferRenderbuffer

[Zhenyao Mo, 2016/07/20 19:23:54]

You are correct.  On blink side, we only split DEPTH_STENCIL into DEPTH and
STENCIL if it's WebGL2. Here we should split, and add a DCHECK in Framebuffer,
making sure DEPTH_STENCIL never reaches there.

   }
   if (framebuffer == framebuffer_state_.bound_draw_framebuffer.get()) {
     framebuffer_state_.clear_state_dirty = true;
   }
   OnFboChanged();
 }
 
 void GLES2DecoderImpl::DoDisable(GLenum cap) {
   if (SetCapabilityState(cap, false)) {
     if (cap == GL_PRIMITIVE_RESTART_FIXED_INDEX &&
@@ skipping 10439 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