Dash batch

Dash batch

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/4f569beac777afde2f2dd9079a52535f8e7519b3

[joshualitt, 2015-02-24 22:04:08]

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

[egdaniel, 2015-02-27 15:13:18]

https://codereview.chromium.org/925673002/diff/80001/src/gpu/effects/GrDashin...
File src/gpu/effects/GrDashingEffect.cpp (right):

https://codereview.chromium.org/925673002/diff/80001/src/gpu/effects/GrDashin...
src/gpu/effects/GrDashingEffect.cpp:347: calc_dash_scaling(&parallelScale,
&perpScale, args.fViewMatrix, args.fPtsRot);
do we really want recalculate this for all the dashes (it is already calculated
when we make the original DrawDashLine call. Is this just to save the 2 scalars
from being in geometry? It isn't the cheapest calculation.

https://codereview.chromium.org/925673002/diff/80001/src/gpu/effects/GrDashin...
src/gpu/effects/GrDashingEffect.cpp:592: const GrIndexBuffer* quadIndexBuffer =
batchTarget->quadIndexBuffer();
rename from quad

https://codereview.chromium.org/925673002/diff/80001/src/gpu/effects/GrDashin...
src/gpu/effects/GrDashingEffect.cpp:602:
drawInfo.setIndexBuffer(quadIndexBuffer);
quad

https://codereview.chromium.org/925673002/diff/80001/src/gpu/effects/GrDashin...
src/gpu/effects/GrDashingEffect.cpp:604: int maxInstancesPerDraw =
quadIndexBuffer->maxQuads();
quads

https://codereview.chromium.org/925673002/diff/80001/src/gpu/effects/GrDashin...
src/gpu/effects/GrDashingEffect.cpp:700: //SkScalar srcPhase = info.fPhase;
remove line?

https://codereview.chromium.org/925673002/diff/80001/src/gpu/effects/GrDashin...
src/gpu/effects/GrDashingEffect.cpp:722: SkScalar endInterval =
info.fIntervals[1] * parallelScale;
can this be off interval? end interval doesn't really make sense since we
theoretically can start on an off interval

https://codereview.chromium.org/925673002/diff/80001/src/gpu/effects/GrDashin...
src/gpu/effects/GrDashingEffect.cpp:731: bool fullDash = endInterval > 0.f ||
useAA;
Add todo to do a real rect call in case where fullDash is false (no off interval
and not using AA).

https://codereview.chromium.org/925673002/diff/80001/src/gpu/effects/GrDashin...
src/gpu/effects/GrDashingEffect.cpp:743: //SkDEBUGCODE(fDevBounds;)
delete?

[joshualitt, 2015-02-27 15:39:10]

On 2015/02/27 15:13:18, egdaniel wrote:
>
https://codereview.chromium.org/925673002/diff/80001/src/gpu/effects/GrDashin...
> File src/gpu/effects/GrDashingEffect.cpp (right):
> 
>
https://codereview.chromium.org/925673002/diff/80001/src/gpu/effects/GrDashin...
> src/gpu/effects/GrDashingEffect.cpp:347: calc_dash_scaling(&parallelScale,
> &perpScale, args.fViewMatrix, args.fPtsRot);
> do we really want recalculate this for all the dashes (it is already
calculated
> when we make the original DrawDashLine call. Is this just to save the 2
scalars
> from being in geometry? It isn't the cheapest calculation.
> 
>
https://codereview.chromium.org/925673002/diff/80001/src/gpu/effects/GrDashin...
> src/gpu/effects/GrDashingEffect.cpp:592: const GrIndexBuffer* quadIndexBuffer
=
> batchTarget->quadIndexBuffer();
> rename from quad
> 
>
https://codereview.chromium.org/925673002/diff/80001/src/gpu/effects/GrDashin...
> src/gpu/effects/GrDashingEffect.cpp:602:
> drawInfo.setIndexBuffer(quadIndexBuffer);
> quad
> 
>
https://codereview.chromium.org/925673002/diff/80001/src/gpu/effects/GrDashin...
> src/gpu/effects/GrDashingEffect.cpp:604: int maxInstancesPerDraw =
> quadIndexBuffer->maxQuads();
> quads
> 
>
https://codereview.chromium.org/925673002/diff/80001/src/gpu/effects/GrDashin...
> src/gpu/effects/GrDashingEffect.cpp:700: //SkScalar srcPhase = info.fPhase;
> remove line?
> 
>
https://codereview.chromium.org/925673002/diff/80001/src/gpu/effects/GrDashin...
> src/gpu/effects/GrDashingEffect.cpp:722: SkScalar endInterval =
> info.fIntervals[1] * parallelScale;
> can this be off interval? end interval doesn't really make sense since we
> theoretically can start on an off interval
> 
>
https://codereview.chromium.org/925673002/diff/80001/src/gpu/effects/GrDashin...
> src/gpu/effects/GrDashingEffect.cpp:731: bool fullDash = endInterval > 0.f ||
> useAA;
> Add todo to do a real rect call in case where fullDash is false (no off
interval
> and not using AA).
> 
>
https://codereview.chromium.org/925673002/diff/80001/src/gpu/effects/GrDashin...
> src/gpu/effects/GrDashingEffect.cpp:743: //SkDEBUGCODE(fDevBounds;)
> delete?

feedback incorporated

[egdaniel, 2015-02-27 18:32:57]

lgtm

[joshualitt, 2015-02-27 19:34:09]

The CQ bit was checked by joshualitt@google.com

[commit-bot: I haz the power, 2015-02-27 19:34:55]

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

[commit-bot: I haz the power, 2015-02-27 19:41:55]

Committed patchset #6 (id:100001) as
https://skia.googlesource.com/skia/+/4f569beac777afde2f2dd9079a52535f8e7519b3