Calculate Primary and Secondary output types in the GrOptDrawState

Calculate Primary and Secondary output types in the GrOptDrawState

Follow up CL to https://codereview.chromium.org/545693004/

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/c06482494d2be6481a3666a66254b6c66890f994

[egdaniel, 2014-09-16 21:21:03]

egdaniel@google.com changed reviewers:
+ bsalomon@google.com

[egdaniel, 2014-09-17 19:02:27]

ptal

[bsalomon, 2014-09-17 19:47:42]

https://codereview.chromium.org/554833002/diff/60001/src/gpu/GrDrawState.cpp
File src/gpu/GrDrawState.cpp (right):

https://codereview.chromium.org/554833002/diff/60001/src/gpu/GrDrawState.cpp#...
src/gpu/GrDrawState.cpp:16: GrOptDrawState* GrDrawState::createOptState(const
GrDrawTargetCaps& caps) const {
this seems a little weird... why not make the GrDrawState take a GrDTC in its
cons?

https://codereview.chromium.org/554833002/diff/60001/src/gpu/GrOptDrawState.h
File src/gpu/GrOptDrawState.h (right):

https://codereview.chromium.org/554833002/diff/60001/src/gpu/GrOptDrawState.h...
src/gpu/GrOptDrawState.h:32: /// @name Stage Output Types
This is begging for the XP!

https://codereview.chromium.org/554833002/diff/60001/src/gpu/GrOptDrawState.h...
src/gpu/GrOptDrawState.h:114: * blend coeffs will represent those used by GL.
can you say "backend API" instead of "GL"?

https://codereview.chromium.org/554833002/diff/60001/src/gpu/GrOptDrawState.h...
src/gpu/GrOptDrawState.h:133: // Secondary color output info
comment seems wrong... "Fragment shader color outputs"?

https://codereview.chromium.org/554833002/diff/60001/tests/GLProgramsTest.cpp
File tests/GLProgramsTest.cpp (right):

https://codereview.chromium.org/554833002/diff/60001/tests/GLProgramsTest.cpp...
tests/GLProgramsTest.cpp:161: GrOptDrawState::PrimaryOutputType primaryOutput;
At some point this has to turn into build a random GrDS or GrODS and than build
a desc from it...

[egdaniel, 2014-09-18 15:24:37]

https://codereview.chromium.org/554833002/diff/60001/src/gpu/GrDrawState.cpp
File src/gpu/GrDrawState.cpp (right):

https://codereview.chromium.org/554833002/diff/60001/src/gpu/GrDrawState.cpp#...
src/gpu/GrDrawState.cpp:16: GrOptDrawState* GrDrawState::createOptState(const
GrDrawTargetCaps& caps) const {
On 2014/09/17 19:47:42, bsalomon wrote:
> this seems a little weird... why not make the GrDrawState take a GrDTC in its
> cons?

I feel like caps are not really part of our current DS and it feels weird
forcing them into the ctor. Also the end goal will be to have a createOS
(possibly static) which just takes in all necessary data (DS, caps, Geom Info,
etc) to create its object. So for now I will keep caps being passed in and
simply do a light weight check to see if they may have changed and thus need to
recompute an ODS.

https://codereview.chromium.org/554833002/diff/60001/src/gpu/GrOptDrawState.h
File src/gpu/GrOptDrawState.h (right):

https://codereview.chromium.org/554833002/diff/60001/src/gpu/GrOptDrawState.h...
src/gpu/GrOptDrawState.h:114: * blend coeffs will represent those used by GL.
On 2014/09/17 19:47:42, bsalomon wrote:
> can you say "backend API" instead of "GL"?

Done.

https://codereview.chromium.org/554833002/diff/60001/src/gpu/GrOptDrawState.h...
src/gpu/GrOptDrawState.h:133: // Secondary color output info
On 2014/09/17 19:47:42, bsalomon wrote:
> comment seems wrong... "Fragment shader color outputs"?

Done.

https://codereview.chromium.org/554833002/diff/60001/tests/GLProgramsTest.cpp
File tests/GLProgramsTest.cpp (right):

https://codereview.chromium.org/554833002/diff/60001/tests/GLProgramsTest.cpp...
tests/GLProgramsTest.cpp:161: GrOptDrawState::PrimaryOutputType primaryOutput;
On 2014/09/17 19:47:42, bsalomon wrote:
> At some point this has to turn into build a random GrDS or GrODS and than
build
> a desc from it...

Yes I definitely agree. Let's wait till we finalize all our architectural
changes then go back and make this much more sane.

[bsalomon, 2014-09-18 17:03:45]

https://codereview.chromium.org/554833002/diff/100001/src/gpu/GrDrawState.h
File src/gpu/GrDrawState.h (right):

https://codereview.chromium.org/554833002/diff/100001/src/gpu/GrDrawState.h#n...
src/gpu/GrDrawState.h:565: mutable SkAutoTUnref<const GrDrawTargetCaps>
fCachedCaps;
Let's put a unique ID on GrDrawTargetCaps rather than hold a ref here.

*Or*

Since GrODS already has a caps can't you just check that fCachedOptState's caps
is the one passed in?

[egdaniel, 2014-09-22 19:00:41]

updated with unique ID on caps

https://codereview.chromium.org/554833002/diff/100001/src/gpu/GrDrawState.h
File src/gpu/GrDrawState.h (right):

https://codereview.chromium.org/554833002/diff/100001/src/gpu/GrDrawState.h#n...
src/gpu/GrDrawState.h:565: mutable SkAutoTUnref<const GrDrawTargetCaps>
fCachedCaps;
Added unique ID to caps. We don't store the caps on the ODS since they are only
used in building it, thus options 2 would not work

On 2014/09/18 17:03:45, bsalomon wrote:
> Let's put a unique ID on GrDrawTargetCaps rather than hold a ref here.
> 
> *Or*
> 
> Since GrODS already has a caps can't you just check that fCachedOptState's
caps
> is the one passed in?

[bsalomon, 2014-09-22 19:21:36]

lgtm

[egdaniel, 2014-09-22 20:07:01]

The CQ bit was checked by egdaniel@google.com

[commit-bot: I haz the power, 2014-09-22 20:07:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/554833002/140001

[commit-bot: I haz the power, 2014-09-22 20:17:07]

Committed patchset #8 (id:140001) as c06482494d2be6481a3666a66254b6c66890f994