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

gpu/command_buffer/service/gles2_cmd_decoder.cc

Index: gpu/command_buffer/service/gles2_cmd_decoder.cc
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index 6df7f6a2f816a89bb6e37149101e0a60c27a2da4..8b9ad5bb70bf0ca55a4519189207168e4876e78d 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -1366,6 +1366,16 @@ class GLES2DecoderImpl : public GLES2Decoder, public ErrorStateClient {
   // of the draw operation are the same.
   bool CheckDrawingFeedbackLoops();
 
+  // 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; set |adjusted| to true
+  // if such adjustments happen. Return true.
+  bool ValidateAndAdjustDrawBuffers(const char* function_name, bool* adjusted);
+
   // Checks if |api_type| is valid for the given uniform
   // If the api type is not valid generates the appropriate GL
   // error. Returns true if |api_type| is valid for the uniform
@@ -7096,7 +7106,7 @@ void GLES2DecoderImpl::ClearUnclearedAttachments(
 
   if (cleared_int_renderbuffers || clear_bits) {
     if (reset_draw_buffers)
-      framebuffer->RestoreDrawBuffersAfterClear();
+      framebuffer->RestoreDrawBuffers();
     RestoreClearState();
     if (target == GL_READ_FRAMEBUFFER && draw_framebuffer != framebuffer) {
       GLuint service_id = draw_framebuffer ? draw_framebuffer->service_id() :
@@ -8026,6 +8036,58 @@ bool GLES2DecoderImpl::CheckDrawingFeedbackLoops() {
   return false;
 }
 
+bool GLES2DecoderImpl::ValidateAndAdjustDrawBuffers(
+    const char* function_name, bool* adjusted) {
+  bool validation = true;
+  bool my_adjusted = false;
+  switch (feature_info_->context_type()) {
+    case CONTEXT_TYPE_OPENGLES2:
+    case CONTEXT_TYPE_WEBGL1:
+      // Int and unsigned int formats are not available in ES2/WebGL1.
+      validation = false;
+      break;
+    default:
+      break;
+  }
+  Framebuffer* framebuffer = framebuffer_state_.bound_draw_framebuffer.get();
+  if (!state_.current_program.get() || !framebuffer) {
+    return true;
+  }
+  const std::vector<ShaderVariableBaseType>& fragment_output_base_types =
+      state_.current_program->GetFragmentOutputBaseTypes();
+  const std::vector<ShaderVariableBaseType>& color_attachment_base_types =
+      framebuffer->GetColorAttachmentBaseTypes();
+  DCHECK_EQ(fragment_output_base_types.size(),
+            color_attachment_base_types.size());
+  size_t count = color_attachment_base_types.size();
+  std::unique_ptr<GLenum[]> adjusted_draw_buffers(new GLenum[count]);
+  for (size_t ii = 0; ii < count; ++ii) {
+    adjusted_draw_buffers[ii] =
+        framebuffer->GetDrawBuffer(GL_DRAW_BUFFER0 + ii);
+    if (adjusted_draw_buffers[ii] == GL_NONE)
+      continue;
+    if (fragment_output_base_types[ii] == SHADER_VARIABLE_UNDEFINED_TYPE) {
+      adjusted_draw_buffers[ii] = GL_NONE;

[Corentin Wallez, 2016/07/13 14:50:07]

This looks like it will be doing an allocation for every draw call in WebGL2.
Can we avoid this cost, for example by lazy allocating adjusted_draw_buffers or
using dirty bits?

+      my_adjusted = true;
+      continue;
+    }
+    if (!validation)
+      continue;
+    if (color_attachment_base_types[ii] == SHADER_VARIABLE_UNDEFINED_TYPE)
+      continue;
+    if (fragment_output_base_types[ii] != color_attachment_base_types[ii]) {
+      LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, function_name,
+          "buffer format and fragment output variable type incompatible");
+      return false;
+    }
+  }
+  if (my_adjusted) {
+    glDrawBuffersARB(count, adjusted_draw_buffers.get());
+  }
+  *adjusted = my_adjusted;
+  return true;
+}
+
 bool GLES2DecoderImpl::CheckUniformForApiType(
     const Program::UniformInfo* info,
     const char* function_name,
@@ -9053,11 +9115,19 @@ error::Error GLES2DecoderImpl::DoDrawArrays(
         primcount)) {
       bool textures_set = !PrepareTexturesForRender();
       ApplyDirtyState();
+      bool adjusted = false;
+      if (!ValidateAndAdjustDrawBuffers(function_name, &adjusted)) {
+        return error::kNoError;
+      }
       if (!instanced) {
         glDrawArrays(mode, first, count);
       } else {
         glDrawArraysInstancedANGLE(mode, first, count, primcount);
       }
+      if (adjusted) {
+        DCHECK(framebuffer_state_.bound_draw_framebuffer.get());
+        framebuffer_state_.bound_draw_framebuffer->RestoreDrawBuffers();
+      }
       if (textures_set) {
         RestoreStateForTextures();
       }
@@ -9194,14 +9264,16 @@ error::Error GLES2DecoderImpl::DoDrawElements(const char* function_name,
         glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);
         indices = element_array_buffer->GetRange(offset, 0);
       }
-
+      bool adjusted = false;
+      if (!ValidateAndAdjustDrawBuffers(function_name, &adjusted)) {
+        return error::kNoError;
+      }
       if (state_.enable_flags.primitive_restart_fixed_index &&
           feature_info_->feature_flags().
               emulate_primitive_restart_fixed_index) {
         glEnable(GL_PRIMITIVE_RESTART);
         buffer_manager()->SetPrimitiveRestartFixedIndexIfNecessary(type);
       }
-
       if (!instanced) {
         glDrawElements(mode, count, type, indices);
       } else {
@@ -9212,12 +9284,14 @@ error::Error GLES2DecoderImpl::DoDrawElements(const char* function_name,
               emulate_primitive_restart_fixed_index) {
         glDisable(GL_PRIMITIVE_RESTART);
       }
-
+      if (adjusted) {
+        DCHECK(framebuffer_state_.bound_draw_framebuffer.get());
+        framebuffer_state_.bound_draw_framebuffer->RestoreDrawBuffers();
+      }
       if (used_client_side_array) {
         glBindBuffer(GL_ELEMENT_ARRAY_BUFFER,
                      element_array_buffer->service_id());
       }
-
       if (textures_set) {
         RestoreStateForTextures();
       }