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

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

This is to workaround a Mesa driver bug.

BUG=333885
TEST=gpu_unittests, test as mentioned in the bug report
R=bajones@chromium.org, kbr@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=249547

[Zhenyao Mo, 2014-02-03 21:56:26]

kbr, bajones: please take a look!

[bajones, 2014-02-03 22:06:39]

On 2014/02/03 21:56:26, Zhenyao Mo wrote:
> kbr, bajones: please take a look!

LGTM

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

https://codereview.chromium.org/153173002/diff/1/gpu/command_buffer/service/p...
File gpu/command_buffer/service/program_manager.cc (right):

https://codereview.chromium.org/153173002/diff/1/gpu/command_buffer/service/p...
gpu/command_buffer/service/program_manager.cc:562: if
(!IsGpuDriverBugWorkaroundsDisabled() &&
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.

https://codereview.chromium.org/153173002/diff/1/gpu/command_buffer/service/p...
gpu/command_buffer/service/program_manager.cc:1375: if
(!IsGpuDriverBugWorkaroundsDisabled()) {
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?

https://codereview.chromium.org/153173002/diff/1/gpu/config/gpu_driver_bug_li...
File gpu/config/gpu_driver_bug_list_json.cc (right):

https://codereview.chromium.org/153173002/diff/1/gpu/config/gpu_driver_bug_li...
gpu/config/gpu_driver_bug_list_json.cc:729: "description": "Mesa drivers handle
varyings without static use incorrectly",
The duplication here is unfortunate. Could you please either make the
description more precise ("Mesa drivers on Linux", "Mesa drivers on Chrome OS")
or figure out a way to merge the two entries?

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

https://codereview.chromium.org/153173002/diff/1/gpu/command_buffer/service/p...
File gpu/command_buffer/service/program_manager.cc (right):

https://codereview.chromium.org/153173002/diff/1/gpu/command_buffer/service/p...
gpu/command_buffer/service/program_manager.cc:562: if
(!IsGpuDriverBugWorkaroundsDisabled() &&
On 2014/02/04 00:34:37, Ken Russell wrote:
> 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.

Refactored, so done.

https://codereview.chromium.org/153173002/diff/1/gpu/command_buffer/service/p...
gpu/command_buffer/service/program_manager.cc:1375: if
(!IsGpuDriverBugWorkaroundsDisabled()) {
On 2014/02/04 00:34:37, Ken Russell wrote:
> 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?

A previous CL fixed the ClearUniforms workaround already

https://codereview.chromium.org/153173002/diff/1/gpu/config/gpu_driver_bug_li...
File gpu/config/gpu_driver_bug_list_json.cc (right):

https://codereview.chromium.org/153173002/diff/1/gpu/config/gpu_driver_bug_li...
gpu/config/gpu_driver_bug_list_json.cc:729: "description": "Mesa drivers handle
varyings without static use incorrectly",
On 2014/02/04 00:34:37, Ken Russell wrote:
> The duplication here is unfortunate. Could you please either make the
> description more precise ("Mesa drivers on Linux", "Mesa drivers on Chrome
OS")
> or figure out a way to merge the two entries?

Done.

[Zhenyao Mo, 2014-02-06 21:54:24]

kbr: revised. please take another look.

[Ken Russell (switch to Gerrit), 2014-02-06 22:13:30]

LGTM

https://codereview.chromium.org/153173002/diff/190001/gpu/command_buffer/serv...
File gpu/command_buffer/service/program_manager.cc (right):

https://codereview.chromium.org/153173002/diff/190001/gpu/command_buffer/serv...
gpu/command_buffer/service/program_manager.cc:122: }
This function's unused and must be deleted (the unused function breaks the Mac
build).

[Zhenyao Mo, 2014-02-06 23:42:40]

There is one failure on android: CompositingModeAsExpected, but I don't think
it's related to this CL.

Landing this.

https://codereview.chromium.org/153173002/diff/190001/gpu/command_buffer/serv...
File gpu/command_buffer/service/program_manager.cc (right):

https://codereview.chromium.org/153173002/diff/190001/gpu/command_buffer/serv...
gpu/command_buffer/service/program_manager.cc:122: }
On 2014/02/06 22:13:30, Ken Russell wrote:
> This function's unused and must be deleted (the unused function breaks the Mac
> build).

Done.

[Zhenyao Mo, 2014-02-07 00:11:23]

Committed patchset #4 manually as r249547 (presubmit successful).