Pass in ProcOptInfos into willNeedDstCopy on XPs

Pass in ProcOptInfos into willNeedDstCopy on XPs

BUG=skia:

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

[egdaniel, 2015-02-11 14:56:08]

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

[egdaniel, 2015-02-11 14:56:08]

This is a minor change as a pre-cl to landing the porter duff dst copy cl.

[bsalomon, 2015-02-11 15:38:03]

https://codereview.chromium.org/912413002/diff/20001/src/gpu/GrInOrderDrawBuf...
File src/gpu/GrInOrderDrawBuffer.cpp (right):

https://codereview.chromium.org/912413002/diff/20001/src/gpu/GrInOrderDrawBuf...
src/gpu/GrInOrderDrawBuffer.cpp:628: GrBatchOpt batchOpt;
seems weird that we're doing this here.

https://codereview.chromium.org/912413002/diff/20001/src/gpu/GrInOrderDrawBuf...
File src/gpu/GrInOrderDrawBuffer.h (right):

https://codereview.chromium.org/912413002/diff/20001/src/gpu/GrInOrderDrawBuf...
src/gpu/GrInOrderDrawBuffer.h:281: bool SK_WARN_UNUSED_RESULT
recordStateAndSetupPipeline(const GrPrimitiveProcessor*,
savePipelineAndShouldDraw?

[egdaniel, 2015-02-11 16:48:09]

https://codereview.chromium.org/912413002/diff/20001/src/gpu/GrInOrderDrawBuf...
File src/gpu/GrInOrderDrawBuffer.cpp (right):

https://codereview.chromium.org/912413002/diff/20001/src/gpu/GrInOrderDrawBuf...
src/gpu/GrInOrderDrawBuffer.cpp:628: GrBatchOpt batchOpt;
On 2015/02/11 15:38:03, bsalomon wrote:
> seems weird that we're doing this here.

So the reason we are doing this here is that we need the batch to be initiated
in order to get the color/coverage ProcOptInfos which are now needed for
setupDstReadIfNecessary. This approach was chosen so we only need to calc these
ProcOptInfos once since they are also needed to construct the pipeline.

Other alternatives would be to recreate a caching system so that we only create
the POIs once. Or we could just eat the time to create them twice. Thoughts?

[bsalomon, 2015-02-11 16:51:23]

https://codereview.chromium.org/912413002/diff/20001/src/gpu/GrInOrderDrawBuf...
File src/gpu/GrInOrderDrawBuffer.cpp (right):

https://codereview.chromium.org/912413002/diff/20001/src/gpu/GrInOrderDrawBuf...
src/gpu/GrInOrderDrawBuffer.cpp:628: GrBatchOpt batchOpt;
On 2015/02/11 16:48:08, egdaniel wrote:
> On 2015/02/11 15:38:03, bsalomon wrote:
> > seems weird that we're doing this here.
> 
> So the reason we are doing this here is that we need the batch to be initiated
> in order to get the color/coverage ProcOptInfos which are now needed for
> setupDstReadIfNecessary. This approach was chosen so we only need to calc
these
> ProcOptInfos once since they are also needed to construct the pipeline.
> 
> Other alternatives would be to recreate a caching system so that we only
create
> the POIs once. Or we could just eat the time to create them twice. Thoughts?

It seems to like everything from 628 to 643 should be done by the GrDT base
class. Maybe we should move to a model where the base class has a function that
takes the params necessary to compute a pipeline and placement news it into
memory provided by the subclass.

[egdaniel, 2015-02-12 21:27:11]

ptal

[bsalomon, 2015-02-12 21:39:26]

minor comments... really like moving away from having cases where base class
calls will call back to virtuals on the subclass

lgtm

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

https://codereview.chromium.org/912413002/diff/100001/src/gpu/GrDrawTarget.h#...
src/gpu/GrDrawTarget.h:688: PipelineInfo(GrPipelineBuilder* pipelineBuilder,
GrScissorState* scissor,
can we bury these constructors in the cpp?

https://codereview.chromium.org/912413002/diff/100001/src/gpu/GrDrawTarget.h#...
src/gpu/GrDrawTarget.h:730: private:
\n

https://codereview.chromium.org/912413002/diff/100001/src/gpu/GrInOrderDrawBu...
File src/gpu/GrInOrderDrawBuffer.h (right):

https://codereview.chromium.org/912413002/diff/100001/src/gpu/GrInOrderDrawBu...
src/gpu/GrInOrderDrawBuffer.h:196: ~SetState() {
one line?

https://codereview.chromium.org/912413002/diff/100001/src/gpu/GrInOrderDrawBu...
src/gpu/GrInOrderDrawBuffer.h:200: GrPipeline* getPipeline() { return
reinterpret_cast<GrPipeline*>(fPipeline.get()); }
pipelineLocation()? comment that this is just for constructing the pl?

https://codereview.chromium.org/912413002/diff/100001/src/gpu/GrInOrderDrawBu...
src/gpu/GrInOrderDrawBuffer.h:232: void onDraw(const GrGeometryProcessor*,
one line?

[egdaniel, 2015-02-13 14:10:43]

New patchsets have been uploaded after l-g-t-m from bsalomon@google.com

[egdaniel, 2015-02-13 14:11:37]

The CQ bit was checked by egdaniel@google.com

[commit-bot: I haz the power, 2015-02-13 14:12:16]

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

[commit-bot: I haz the power, 2015-02-13 17:00:38]

Committed patchset #7 (id:120001) as
https://skia.googlesource.com/skia/+/e36914cb205699526988127a827d1a76c9a98d39