Fix a bug in shader/program relationship.

gpu/command_buffer/service/program_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/program_manager.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
@@ skipping 402 matching lines @@
   for (uint32_t ii = 0; ii < vertex_input_base_type_mask_.size(); ++ii) {
     vertex_input_base_type_mask_[ii] = 0u;
     vertex_input_active_mask_[ii] = 0u;
   }
 }
 
 void Program::UpdateFragmentOutputBaseTypes() {
   fragment_output_type_mask_ = 0u;
   fragment_output_written_mask_ = 0u;
   Shader* fragment_shader =
-      attached_shaders_[ShaderTypeToIndex(GL_FRAGMENT_SHADER)].get();
+      shaders_from_last_successful_link_[ShaderTypeToIndex(GL_FRAGMENT_SHADER)]
+          .get();
   DCHECK(fragment_shader);
   for (auto const& output : fragment_shader->output_variable_list()) {
     int location = output.location;
     DCHECK(location == -1 ||
            (location >= 0 &&
             location < static_cast<int>(manager_->max_draw_buffers())));
     if (location == -1)
       location = 0;
     if (ProgramManager::HasBuiltInPrefix(output.name)) {
       if (output.name != "gl_FragColor" && output.name != "gl_FragData")
@@ skipping 61 matching lines @@
 
 void Program::SetUniformBlockBinding(GLuint index, GLuint binding) {
   DCHECK_GT(uniform_block_size_info_.size(), index);
   uniform_block_size_info_[index].binding = binding;
 }
 
 void Program::UpdateTransformFeedbackInfo() {
   effective_transform_feedback_buffer_mode_ = transform_feedback_buffer_mode_;
   effective_transform_feedback_varyings_ = transform_feedback_varyings_;
 
-  Shader* vertex_shader = attached_shaders_[0].get();
+  Shader* vertex_shader = shaders_from_last_successful_link_[0].get();
   DCHECK(vertex_shader);
 
   if (effective_transform_feedback_buffer_mode_ == GL_INTERLEAVED_ATTRIBS) {
     transform_feedback_data_size_per_vertex_.resize(1);
   } else {
     transform_feedback_data_size_per_vertex_.resize(
         effective_transform_feedback_varyings_.size());
   }
 
   base::CheckedNumeric<GLsizeiptr> total = 0;
@@ skipping 303 matching lines @@
     if (!ProgramManager::HasBuiltInPrefix(service_name)) {
       service_location =
           glGetUniformLocation(service_id_, service_name.c_str());
     }
 
     // Determine the client name of the uniform and whether it is an array
     // or not.
     bool is_array = false;
     std::string client_name;
     for (size_t i = 0; i < kMaxAttachedShaders && client_name.empty(); ++i) {
-      const auto& shader = attached_shaders_[i];
+      const auto& shader = shaders_from_last_successful_link_[i];
       if (!shader)
         continue;
       const sh::ShaderVariable* info = nullptr;
       const sh::Uniform* uniform = shader->GetUniformInfo(service_name);
       if (uniform &&
           uniform->findInfoByMappedName(service_name, &info, &client_name)) {
         DCHECK(!client_name.empty());
         is_array = info->arraySize > 0;
         type = info->type;
         size = std::max(1u, info->arraySize);
@@ skipping 130 matching lines @@
     return;
 
   GLint max_len = 0;
   glGetProgramInterfaceiv(service_id_, GL_FRAGMENT_INPUT_NV, GL_MAX_NAME_LENGTH,
                           &max_len);
   DCHECK(max_len > 0);
 
   std::unique_ptr<char[]> name_buffer(new char[max_len]);
 
   Shader* fragment_shader =
-      attached_shaders_[ShaderTypeToIndex(GL_FRAGMENT_SHADER)].get();
+      shaders_from_last_successful_link_[ShaderTypeToIndex(GL_FRAGMENT_SHADER)]
+          .get();
 
   const GLenum kQueryProperties[] = {GL_LOCATION, GL_TYPE, GL_ARRAY_SIZE};
 
   std::vector<size_t> client_location_indices;
   for (GLint ii = 0; ii < num_fragment_inputs; ++ii) {
     GLsizei name_length = 0;
     glGetProgramResourceName(service_id_, GL_FRAGMENT_INPUT_NV, ii, max_len,
                              &name_length, name_buffer.get());
     DCHECK(name_length < max_len);
     DCHECK(name_length == 0 || name_buffer[name_length] == '\0');
@@ skipping 86 matching lines @@
         &fragment_input_infos_[i]);
   }
 }
 
 void Program::UpdateProgramOutputs() {
   if (!feature_info().gl_version_info().is_es3_capable ||
       feature_info().disable_shader_translator())
     return;
 
   Shader* fragment_shader =
-      attached_shaders_[ShaderTypeToIndex(GL_FRAGMENT_SHADER)].get();
+      shaders_from_last_successful_link_[ShaderTypeToIndex(GL_FRAGMENT_SHADER)]
+          .get();
 
   for (auto const& output_var : fragment_shader->output_variable_list()) {
     const std::string& service_name = output_var.mappedName;
     // A fragment shader can have gl_FragColor, gl_SecondaryFragColor, etc
     // built-ins as its output, as well as custom varyings. We are interested
     // only in custom varyings, client is allowed to bind only them.
     if (ProgramManager::HasBuiltInPrefix(service_name))
       continue;
 
     std::string client_name = output_var.name;
@@ skipping 45 matching lines @@
 void Program::ExecuteBindAttribLocationCalls() {
   for (const auto& key_value : bind_attrib_location_map_) {
     const std::string* mapped_name = GetAttribMappedName(key_value.first);
     if (mapped_name)
       glBindAttribLocation(service_id_, key_value.second, mapped_name->c_str());
   }
 }
 
 bool Program::ExecuteTransformFeedbackVaryingsCall() {
   if (!transform_feedback_varyings_.empty()) {
     Shader* vertex_shader = attached_shaders_[0].get();

[Ken Russell (switch to Gerrit), 2017/06/26 22:56:20]

Here and in several more places, there are some methods which are called before
or during program linking, and that's why attached_shaders_ is used instead of
shaders_from_last_successful_link_. This is a little subtle; is it worth adding
a comment like:

  // This is called before program linking, so refer to attached_shaders_.

at each of these sites?

     if (!vertex_shader) {
       set_log_info("TransformFeedbackVaryings: missing vertex shader");
       return false;
     }
 
     std::vector<const char*> mapped_names;
     mapped_names.reserve(transform_feedback_varyings_.size());
     for (StringVector::const_iterator it =
              transform_feedback_varyings_.begin();
          it != transform_feedback_varyings_.end(); ++it) {
@@ skipping 14 matching lines @@
 
   return true;
 }
 
 void Program::ExecuteProgramOutputBindCalls() {
   if (feature_info().disable_shader_translator()) {
     return;
   }
 
   Shader* fragment_shader =
       attached_shaders_[ShaderTypeToIndex(GL_FRAGMENT_SHADER)].get();

[Ken Russell (switch to Gerrit), 2017/06/26 22:56:20]

Same here.

   DCHECK(fragment_shader && fragment_shader->valid());
 
   if (fragment_shader->shader_version() != 100) {
     // ES SL 1.00 does not have mechanism for introducing variables that could
     // be bound. This means that ES SL 1.00 binding calls would be to
     // non-existing variable names.  Binding calls are only executed with ES SL
     // 3.00 and higher.
     for (auto const& output_var : fragment_shader->output_variable_list()) {
       size_t count = std::max(output_var.arraySize, 1u);
       bool is_array = output_var.arraySize > 0;
@@ skipping 184 matching lines @@
       glProgramParameteri(service_id(),
                           PROGRAM_BINARY_RETRIEVABLE_HINT,
                           GL_TRUE);
     }
     glLinkProgram(service_id());
   }
 
   GLint success = 0;
   glGetProgramiv(service_id(), GL_LINK_STATUS, &success);
   if (success == GL_TRUE) {
+    for (size_t ii = 0; ii < kMaxAttachedShaders; ++ii)
+      shaders_from_last_successful_link_[ii] = attached_shaders_[ii];
     Update();
     if (link) {
       // ANGLE updates the translated shader sources on link.
       for (auto shader : attached_shaders_) {
         shader->RefreshTranslatedShaderSource();
       }
       if (cache) {
         cache->SaveLinkedProgram(
             service_id(), attached_shaders_[0].get(),
             attached_shaders_[1].get(), &bind_attrib_location_map_,
@@ skipping 102 matching lines @@
     return true;
   size_t location_index =
       GetUniformLocationIndexFromFakeLocation(fake_location);
   if (location_index >= uniform_locations_.size())
     return false;
   return uniform_locations_[location_index].IsInactive();
 }
 
 const std::string* Program::GetAttribMappedName(
     const std::string& original_name) const {
   for (auto shader : attached_shaders_) {

[Ken Russell (switch to Gerrit), 2017/06/26 22:56:20]

Same here.

     if (shader) {
       const std::string* mapped_name =
           shader->GetAttribMappedName(original_name);
       if (mapped_name)
         return mapped_name;
     }
   }
   return nullptr;
 }
 
 const std::string* Program::GetUniformMappedName(
     const std::string& original_name) const {
   for (auto shader : attached_shaders_) {

[Ken Russell (switch to Gerrit), 2017/06/26 22:56:20]

Same here.

     if (shader) {
       const std::string* mapped_name =
           shader->GetUniformMappedName(original_name);
       if (mapped_name)
         return mapped_name;
     }
   }
   return nullptr;
 }
 
 const std::string* Program::GetOriginalNameFromHashedName(
     const std::string& hashed_name) const {
   for (auto shader : attached_shaders_) {

[Ken Russell (switch to Gerrit), 2017/06/26 22:56:20]

Same here. There are more places, too.

     if (shader) {
       const std::string* original_name =
           shader->GetOriginalNameFromHashedName(hashed_name);
       if (original_name)
         return original_name;
     }
   }
   return nullptr;
 }
 
 const sh::Varying* Program::GetVaryingInfo(
     const std::string& hashed_name) const {
-  for (auto shader : attached_shaders_) {
+  for (auto shader : shaders_from_last_successful_link_) {
     if (shader) {
       const sh::Varying* info = shader->GetVaryingInfo(hashed_name);
       if (info)
         return info;
     }
   }
   return nullptr;
 }
 
 const sh::InterfaceBlock* Program::GetInterfaceBlockInfo(
     const std::string& hashed_name) const {
-  for (auto shader : attached_shaders_) {
+  for (auto shader : shaders_from_last_successful_link_) {
     if (shader) {
       const sh::InterfaceBlock* info =
           shader->GetInterfaceBlockInfo(hashed_name);
       if (info)
         return info;
     }
   }
   return nullptr;
 }
 
@@ skipping 57 matching lines @@
   bind_program_output_location_index_map_[name] =
       std::make_pair(color_name, index);
   bind_program_output_location_index_map_[name + "[0]"] =
       std::make_pair(color_name, index);
 }
 
 void Program::GetVertexAttribData(
     const std::string& name, std::string* original_name, GLenum* type) const {
   DCHECK(original_name);
   DCHECK(type);
-  Shader* shader = attached_shaders_[ShaderTypeToIndex(GL_VERTEX_SHADER)].get();
+  Shader* shader =
+      shaders_from_last_successful_link_[ShaderTypeToIndex(GL_VERTEX_SHADER)]
+          .get();
   if (shader) {
     // Vertex attributes can not be arrays or structs (GLSL ES 3.00.4, section
     // 4.3.4, "Input Variables"), so the top level sh::Attribute returns the
     // information we need.
     const sh::Attribute* info = shader->GetAttribInfo(name);
     if (info) {
       *original_name = info->name;
       *type = info->type;
       return;
     }
@@ skipping 1022 matching lines @@
   DCHECK(program);
   program->ClearUniforms(&zero_);
 }
 
 int32_t ProgramManager::MakeFakeLocation(int32_t index, int32_t element) {
   return index + element * 0x10000;
 }
 
 }  // namespace gles2
 }  // namespace gpu