Add a gpu driver bug workaround to count in all varyings in packing check.

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 <algorithm>
 #include <set>
 #include <utility>
 #include <vector>
@@ skipping 99 matching lines @@
       "gl_FrontFacing",
       "gl_PointCoord"
   };
   for (size_t ii = 0; ii < arraysize(kBuiltInVaryings); ++ii) {
     if (name == kBuiltInVaryings[ii])
       return true;
   }
   return false;
 }
 
+bool IsGpuDriverBugWorkaroundsDisabled() {
+  return CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kDisableGpuDriverBugWorkarounds);
+}
+
 }  // anonymous namespace.
 
 Program::UniformInfo::UniformInfo()
     : size(0),
       type(GL_NONE),
       fake_location_base(0),
       is_array(false) {
 }
 
 Program::UniformInfo::UniformInfo(
@@ skipping 416 matching lines @@
                            "not declared in vertex shader: " + conflicting_name;
     set_log_info(ProcessLogInfo(info_log).c_str());
     return false;
   }
   if (DetectGlobalNameConflicts(&conflicting_name)) {
     std::string info_log = "Name conflicts between an uniform and an "
                            "attribute: " + conflicting_name;
     set_log_info(ProcessLogInfo(info_log).c_str());
     return false;
   }
-  if (!CheckVaryingsPacking()) {
+  VaryingsPackingOption option = kCountOnlyStaticallyUsed;
+  if (!IsGpuDriverBugWorkaroundsDisabled() &&

[Ken Russell (switch to Gerrit), 2014/02/04 00:34:37]

It seems unfortunate to me that the Program object has to query the command line
and driver bug workarounds, and does so all the time. Could you make it so that
the ProgramManager makes all of these queries once, and passes down the needed
arguments to the Program object (i.e., as a new argument to LinkProgram)? It
looks like the feature_info wasn't actually previously used, so maybe you could
just add a new boolean argument indicating whether to count all varyings rather
than just the statically used ones.

[Zhenyao Mo, 2014/02/06 21:54:05]

Refactored, so done.

+      feature_info &&
+      feature_info->workarounds().count_all_in_varyings_packing)
+    option = kCountAll;
+  if (!CheckVaryingsPacking(option)) {
     set_log_info("Varyings over maximum register limit");
     return false;
   }
 
   TimeTicks before_time = TimeTicks::HighResNow();
   bool link = true;
   ProgramCache* cache = manager_->program_cache_;
   if (cache) {
     DCHECK(attached_shaders_[0]->signature_source() &&
            attached_shaders_[1]->signature_source());
@@ skipping 524 matching lines @@
     for (int ii = 0; ii < 2; ++ii) {
       if (uniforms[ii]->find(iter->first) != uniforms[ii]->end()) {
         *conflicting_name = iter->first;
         return true;
       }
     }
   }
   return false;
 }
 
-bool Program::CheckVaryingsPacking() const {
+bool Program::CheckVaryingsPacking(
+    Program::VaryingsPackingOption option) const {
   DCHECK(attached_shaders_[0] &&
          attached_shaders_[0]->shader_type() == GL_VERTEX_SHADER &&
          attached_shaders_[1] &&
          attached_shaders_[1]->shader_type() == GL_FRAGMENT_SHADER);
   const ShaderTranslator::VariableMap* vertex_varyings =
       &(attached_shaders_[0]->varying_map());
   const ShaderTranslator::VariableMap* fragment_varyings =
       &(attached_shaders_[1]->varying_map());
 
   std::map<std::string, ShVariableInfo> combined_map;
 
   for (ShaderTranslator::VariableMap::const_iterator iter =
            fragment_varyings->begin();
        iter != fragment_varyings->end(); ++iter) {
-    if (!iter->second.static_use)
+    if (!iter->second.static_use && option == kCountOnlyStaticallyUsed)
       continue;
     if (!IsBuiltInVarying(iter->first)) {
       ShaderTranslator::VariableMap::const_iterator vertex_iter =
           vertex_varyings->find(iter->first);
       if (vertex_iter == vertex_varyings->end() ||
-          !vertex_iter->second.static_use)
+          (!vertex_iter->second.static_use &&
+           option == kCountOnlyStaticallyUsed))
         continue;
     }
 
     ShVariableInfo var;
     var.type = static_cast<ShDataType>(iter->second.type);
     var.size = iter->second.size;
     combined_map[iter->first] = var;
   }
 
   if (combined_map.size() == 0)
@@ skipping 111 matching lines @@
     manager_->StopTracking(this);
     manager_ = NULL;
   }
 }
 
 
 ProgramManager::ProgramManager(ProgramCache* program_cache,
                                uint32 max_varying_vectors)
     : program_count_(0),
       have_context_(true),
-      disable_workarounds_(
-          CommandLine::ForCurrentProcess()->HasSwitch(
-              switches::kDisableGpuDriverBugWorkarounds)),
       program_cache_(program_cache),
       max_varying_vectors_(max_varying_vectors) { }
 
 ProgramManager::~ProgramManager() {
   DCHECK(programs_.empty());
 }
 
 void ProgramManager::Destroy(bool have_context) {
   have_context_ = have_context;
   programs_.clear();
@@ skipping 89 matching lines @@
     Program* program) {
   DCHECK(shader_manager);
   DCHECK(program);
   DCHECK(IsOwned(program));
   program->DecUseCount();
   RemoveProgramInfoIfUnused(shader_manager, program);
 }
 
 void ProgramManager::ClearUniforms(Program* program) {
   DCHECK(program);
-  if (!disable_workarounds_) {
+  if (!IsGpuDriverBugWorkaroundsDisabled()) {

[Ken Russell (switch to Gerrit), 2014/02/04 00:34:37]

Tying together these two bug workarounds (needing to count all varyings and
needing to zero out uniforms) makes the code difficult to reason about. Could
you please file a new CL about adding a new category for this workaround ("zero
all uniforms"), and decide in the ProgramManager's constructor whether these two
workarounds ("count all varyings", "zero all uniforms") are actually needed?

[Zhenyao Mo, 2014/02/06 21:54:05]

A previous CL fixed the ClearUniforms workaround already

     program->ClearUniforms(&zero_);
   }
 }
 
 int32 ProgramManager::MakeFakeLocation(int32 index, int32 element) {
   return index + element * 0x10000;
 }
 
 }  // namespace gles2
 }  // namespace gpu