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

gpu/command_buffer/service/program_manager.cc

Index: gpu/command_buffer/service/program_manager.cc
diff --git a/gpu/command_buffer/service/program_manager.cc b/gpu/command_buffer/service/program_manager.cc
index 32b91403bf6fd8093d53c1944903922c7133279b..1e51d44073697e1839bcfb445268f360bdf9f100 100644
--- a/gpu/command_buffer/service/program_manager.cc
+++ b/gpu/command_buffer/service/program_manager.cc
@@ -117,6 +117,11 @@ bool IsBuiltInVarying(const std::string& name) {
   return false;
 }
 
+bool IsGpuDriverBugWorkaroundsDisabled() {
+  return CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kDisableGpuDriverBugWorkarounds);
+}
+
 }  // anonymous namespace.
 
 Program::UniformInfo::UniformInfo()
@@ -553,7 +558,12 @@ bool Program::Link(ShaderManager* manager,
     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;
   }
@@ -1098,7 +1108,8 @@ bool Program::DetectGlobalNameConflicts(std::string* conflicting_name) const {
   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] &&
@@ -1113,13 +1124,14 @@ bool Program::CheckVaryingsPacking() const {
   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;
     }
 
@@ -1251,9 +1263,6 @@ 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) { }
 
@@ -1363,7 +1372,7 @@ void ProgramManager::UnuseProgram(
 
 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_);
   }
 }