Don't look at current vertex layout when vertex source hasn't been set

[bsalomon, 2012-03-29 18:24:48]

We have some canDoSuchAndSuch() helper functions on GrDrawTarget that consider
the capabilities of the the underlying API and the current state. They
internally call getBlendOpts(). getBlendOpts() wants to know the current vertex
layout because stages are considered disabled unless the vertex layout provides
them with texture coordinates.

However, some of the call sites necessarily occur before the caller has
specified a vertex source (and along with it the vertex layout).

This change makes getBlendOpts assume that all stages that have a non-NULL
texture will appear in the vertex layout when considering what blend
optimizations are allowed.

This is still a bit messy. I'd consider a deeper cleanup but I'm hoping that as
the custom shader work comes online we'll have a path foward towards removing
vertex layouts entirely.

[TomH, 2012-03-29 18:30:28]

On 2012/03/29 18:24:48, bsalomon wrote:
> This change makes getBlendOpts assume that all stages that have a non-NULL
> texture will appear in the vertex layout when considering what blend
> optimizations are allowed.

Over-conservative? Is there a perf hit?

[bsalomon, 2012-03-29 18:41:34]

On 2012/03/29 18:30:28, TomH wrote:
> On 2012/03/29 18:24:48, bsalomon wrote:
> > This change makes getBlendOpts assume that all stages that have a non-NULL
> > texture will appear in the vertex layout when considering what blend
> > optimizations are allowed.
> 
> Over-conservative? Is there a perf hit?

In general we don't set textures on stages unless we intend to use them. I don't
expect a perf hit. I just think its ugly.

[TomH, 2012-03-29 18:43:12]

LGTM.

http://codereview.appspot.com/5959047/diff/4001/src/gpu/GrDrawTarget.h
File src/gpu/GrDrawTarget.h (left):

http://codereview.appspot.com/5959047/diff/4001/src/gpu/GrDrawTarget.h#oldcod...
src/gpu/GrDrawTarget.h:126: bool drawWillReadDst() const;
Doesn't it still make sense to encapsulate the logic behind this decision? Or
was it too often wrong?

http://codereview.appspot.com/5959047/diff/4001/src/gpu/GrDrawTarget.h
File src/gpu/GrDrawTarget.h (right):

http://codereview.appspot.com/5959047/diff/4001/src/gpu/GrDrawTarget.h#newcod...
src/gpu/GrDrawTarget.h:134: *    3. If a vertex source has not yet been
specified then any stages with
nit: "all" is probably more explicit than "any"
tangent: now I want to document a project in 18th century dialect

http://codereview.appspot.com/5959047/diff/4001/src/gpu/GrDrawTarget.h#newcod...
src/gpu/GrDrawTarget.h:999: }
Couldn't this (and StagePosAs...) be a static helper somewhere not listed in a
header? I see this not being used elsewhere, although perhaps StagePosAs...()
is.

http://codereview.appspot.com/5959047/diff/4001/src/gpu/GrGpu.cpp
File src/gpu/GrGpu.cpp (right):

http://codereview.appspot.com/5959047/diff/4001/src/gpu/GrGpu.cpp#newcode914
src/gpu/GrGpu.cpp:914: fVertexPool->appendVertices(this->getVertexLayout,
Doesn't this need ()?

[bsalomon, 2012-03-29 20:31:13]

Closed with r3545.

http://codereview.appspot.com/5959047/diff/4001/src/gpu/GrDrawTarget.h
File src/gpu/GrDrawTarget.h (left):

http://codereview.appspot.com/5959047/diff/4001/src/gpu/GrDrawTarget.h#oldcod...
src/gpu/GrDrawTarget.h:126: bool drawWillReadDst() const;
On 2012/03/29 18:43:12, TomH wrote:
> Doesn't it still make sense to encapsulate the logic behind this decision? Or
> was it too often wrong?

There were no longer any callers.

http://codereview.appspot.com/5959047/diff/4001/src/gpu/GrDrawTarget.h
File src/gpu/GrDrawTarget.h (right):

http://codereview.appspot.com/5959047/diff/4001/src/gpu/GrDrawTarget.h#newcod...
src/gpu/GrDrawTarget.h:134: *    3. If a vertex source has not yet been
specified then any stages with
On 2012/03/29 18:43:12, TomH wrote:
> nit: "all" is probably more explicit than "any"
> tangent: now I want to document a project in 18th century dialect

Done. (x3)

http://codereview.appspot.com/5959047/diff/4001/src/gpu/GrDrawTarget.h#newcod...
src/gpu/GrDrawTarget.h:999: }
On 2012/03/29 18:43:12, TomH wrote:
> Couldn't this (and StagePosAs...) be a static helper somewhere not listed in a
> header? I see this not being used elsewhere, although perhaps StagePosAs...()
> is.

Good point. I'll move this to the cpp. StagePosAs... is used by GrDrawTarget
clients to create their vertex layouts and thus must remain int he header.

http://codereview.appspot.com/5959047/diff/4001/src/gpu/GrGpu.cpp
File src/gpu/GrGpu.cpp (right):

http://codereview.appspot.com/5959047/diff/4001/src/gpu/GrGpu.cpp#newcode914
src/gpu/GrGpu.cpp:914: fVertexPool->appendVertices(this->getVertexLayout,
On 2012/03/29 18:43:12, TomH wrote:
> Doesn't this need ()?

Done. (upload.pyed too early)