Plumb drawArc to SkDevice

Plumb drawArc to SkDevice.

Plumbs the drawArc canvas method down to SkDevice without converting to a path. Plumbs through the various recording canvas classes.

BUG=skia:5227
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2257023003

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

[bsalomon, 2016-08-18 21:12:08]

Description was changed from

==========
Add drawArc to SkDevice

BUG=skia:
==========

to

==========
Add drawArc to SkDevice

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2257023003
==========

[bsalomon, 2016-08-19 15:19:12]

Description was changed from

==========
Add drawArc to SkDevice

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2257023003
==========

to

==========
Plumb drawArc to SkDevice.

Plumbs the drawArc canvas method down to SkDevice without converting to a path.
Plumbs through the various recording canvas classes.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2257023003
==========

[bsalomon, 2016-08-19 15:19:12]

bsalomon@google.com changed reviewers:
+ mtklein@google.com, reed@google.com

[bsalomon, 2016-08-19 15:19:29]

More canvas classes please!

[reed1, 2016-08-19 15:55:30]

https://codereview.chromium.org/2257023003/diff/60001/include/core/SkDrawFilt...
File include/core/SkDrawFilter.h (right):

https://codereview.chromium.org/2257023003/diff/60001/include/core/SkDrawFilt...
include/core/SkDrawFilter.h:36: kArc_Type,
Is this necessary, given that we're trying to kill this guy anyway. Notice we
don't have image distinct from bitmap here (nor textblob, nor vertices, etc.).

https://codereview.chromium.org/2257023003/diff/60001/src/core/SkCanvas.cpp
File src/core/SkCanvas.cpp (right):

https://codereview.chromium.org/2257023003/diff/60001/src/core/SkCanvas.cpp#n...
src/core/SkCanvas.cpp:3103: TRACE_EVENT0("disabled-by-default-skia",
"SkCanvas::drawArc()");
How will we document/define what happens when oval is empty? if sweepAngle is 0?
either angle is non-finite?

https://codereview.chromium.org/2257023003/diff/60001/src/core/SkDevice.cpp
File src/core/SkDevice.cpp (right):

https://codereview.chromium.org/2257023003/diff/60001/src/core/SkDevice.cpp#n...
src/core/SkDevice.cpp:78: SkPath  path;
can/should we assert that at this level, sweepAngle is strictly < 360? (so that
path.arcTo will behave the same)

[mtklein, 2016-08-19 15:59:08]

All the recording stuff (anything that says picture,lite,record,etc.) looked
good to me as I watched over Mike's shoulder as he flipped through the CL.

[reed1, 2016-08-19 16:07:53]

lgtm

[robertphillips, 2016-08-19 16:13:51]

robertphillips@google.com changed reviewers:
+ robertphillips@google.com

[robertphillips, 2016-08-19 16:13:52]

lgtm

https://codereview.chromium.org/2257023003/diff/80001/src/core/SkCanvas.cpp
File src/core/SkCanvas.cpp (right):

https://codereview.chromium.org/2257023003/diff/80001/src/core/SkCanvas.cpp#n...
src/core/SkCanvas.cpp:2278: const SkPaint& paint) {
drawArc -> onDrawArc ?

https://codereview.chromium.org/2257023003/diff/80001/src/core/SkCanvas.cpp#n...
src/core/SkCanvas.cpp:2282: if (paint.canComputeFastBounds()) {
Can storage be moved here ?

https://codereview.chromium.org/2257023003/diff/80001/src/utils/SkDumpCanvas.cpp
File src/utils/SkDumpCanvas.cpp (right):

https://codereview.chromium.org/2257023003/diff/80001/src/utils/SkDumpCanvas....
src/utils/SkDumpCanvas.cpp:315: this->dump(kDrawArc_Verb, &paint, "drawArc(%s,
%g, %g, %d)", str.c_str(), startAngle,
Maybe: "useCenter ? "useCenter" : "" and switch the %d to a %s
?

[bsalomon, 2016-08-19 16:14:34]

https://codereview.chromium.org/2257023003/diff/60001/include/core/SkDrawFilt...
File include/core/SkDrawFilter.h (right):

https://codereview.chromium.org/2257023003/diff/60001/include/core/SkDrawFilt...
include/core/SkDrawFilter.h:36: kArc_Type,
On 2016/08/19 15:55:30, reed1 wrote:
> Is this necessary, given that we're trying to kill this guy anyway. Notice we
> don't have image distinct from bitmap here (nor textblob, nor vertices, etc.).

Happy to kill it.

https://codereview.chromium.org/2257023003/diff/60001/src/core/SkCanvas.cpp
File src/core/SkCanvas.cpp (right):

https://codereview.chromium.org/2257023003/diff/60001/src/core/SkCanvas.cpp#n...
src/core/SkCanvas.cpp:3103: TRACE_EVENT0("disabled-by-default-skia",
"SkCanvas::drawArc()");
On 2016/08/19 15:55:30, reed1 wrote:
> How will we document/define what happens when oval is empty? if sweepAngle is
0?
> either angle is non-finite?

Will add some docs to SkCanvas.h

https://codereview.chromium.org/2257023003/diff/60001/src/core/SkDevice.cpp
File src/core/SkDevice.cpp (right):

https://codereview.chromium.org/2257023003/diff/60001/src/core/SkDevice.cpp#n...
src/core/SkDevice.cpp:78: SkPath  path;
On 2016/08/19 15:55:30, reed1 wrote:
> can/should we assert that at this level, sweepAngle is strictly < 360? (so
that
> path.arcTo will behave the same)

Will add docs to SkDevice.h and asserts here. Full circle and 0 sweep should
already be handled by here.

[bsalomon, 2016-08-19 16:37:03]

https://codereview.chromium.org/2257023003/diff/60001/src/core/SkCanvas.cpp
File src/core/SkCanvas.cpp (right):

https://codereview.chromium.org/2257023003/diff/60001/src/core/SkCanvas.cpp#n...
src/core/SkCanvas.cpp:3103: TRACE_EVENT0("disabled-by-default-skia",
"SkCanvas::drawArc()");
On 2016/08/19 16:14:34, bsalomon wrote:
> On 2016/08/19 15:55:30, reed1 wrote:
> > How will we document/define what happens when oval is empty? if sweepAngle
is
> 0?
> > either angle is non-finite?
> 
> Will add some docs to SkCanvas.h

As discussed offline, this CL doesn't change behavior. drawArc already exists.
It's implementation just moves to a new virtual on SkDevice, which currently has
no overrides.

SkCanvas.h already specs this. Leaving revisions to either the docs or behavior
for the future. This *mostly* behaves as:

    SkPath path;
    if (useCenter) {
        path.moveTo(oval.centerX(), oval.centerY());
    }
    path.arcTo(oval, startAngle, sweepAngle, !useCenter);
    if (useCenter) {
        path.close();
    }
    canvas->drawPath(path, ...);

The exception is when the abs(sweepAngle) is >= 360. Then this behaves as
canvas->drawOval(oval, ...); Note that in this case useCenter is ignored, which
matters depending on the paint's style.

https://codereview.chromium.org/2257023003/diff/80001/src/core/SkCanvas.cpp
File src/core/SkCanvas.cpp (right):

https://codereview.chromium.org/2257023003/diff/80001/src/core/SkCanvas.cpp#n...
src/core/SkCanvas.cpp:2278: const SkPaint& paint) {
On 2016/08/19 16:13:52, robertphillips wrote:
> drawArc -> onDrawArc ?

None of the other onDrawFoo methods use "on".

https://codereview.chromium.org/2257023003/diff/80001/src/core/SkCanvas.cpp#n...
src/core/SkCanvas.cpp:2282: if (paint.canComputeFastBounds()) {
On 2016/08/19 16:13:52, robertphillips wrote:
> Can storage be moved here ?

Done.

[bsalomon, 2016-08-19 17:04:18]

Description was changed from

==========
Plumb drawArc to SkDevice.

Plumbs the drawArc canvas method down to SkDevice without converting to a path.
Plumbs through the various recording canvas classes.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2257023003
==========

to

==========
Plumb drawArc to SkDevice.

Plumbs the drawArc canvas method down to SkDevice without converting to a path.
Plumbs through the various recording canvas classes.

BUG=skia:5227
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2257023003
==========

[bsalomon, 2016-08-19 17:56:18]

The CQ bit was checked by bsalomon@google.com

[bsalomon, 2016-08-19 17:56:19]

The patchset sent to the CQ was uploaded after l-g-t-m from
robertphillips@google.com, reed@google.com
Link to the patchset: https://codereview.chromium.org/2257023003/#ps100001
(title: "Address comments")

[commit-bot: I haz the power, 2016-08-19 17:56:29]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-19 18:25:23]

Description was changed from

==========
Plumb drawArc to SkDevice.

Plumbs the drawArc canvas method down to SkDevice without converting to a path.
Plumbs through the various recording canvas classes.

BUG=skia:5227
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2257023003
==========

to

==========
Plumb drawArc to SkDevice.

Plumbs the drawArc canvas method down to SkDevice without converting to a path.
Plumbs through the various recording canvas classes.

BUG=skia:5227
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2257023003

Committed:
https://skia.googlesource.com/skia/+/ac3aa245acc7b469aa2f0d0078e53401d78ac8b9
==========

[commit-bot: I haz the power, 2016-08-19 18:25:24]

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