Add asADash entry point into SkPathEffect to allow extracting Dash info from PathEffects

Add asADash entry point into SkPathEffect to allow extracting Dash info from PathEffects

BUG=skia:

Committed: http://code.google.com/p/skia/source/detail?r=14297

[reed1, 2014-03-27 15:56:07]

As a starting point, I would have guessed that the asA dude would just return
the parameters that were passed to the dashing factory:

intervals[]
count
phase
scale_to_fit

Why does your interface return more data, and why does it have a tri-state enum
instead of just two (none or dash)?

https://codereview.chromium.org/212103010/diff/20001/include/core/SkPathEffect.h
File include/core/SkPathEffect.h (right):

https://codereview.chromium.org/212103010/diff/20001/include/core/SkPathEffec...
include/core/SkPathEffect.h:110: *  it will return Other. If a non NULL info is
passed in the various DashInfo will be filled
s/Other/MultipleInterval

[egdaniel, 2014-03-28 14:28:54]

This version is depended on https://codereview.chromium.org/214413008/ to land
first for access to pictureVersion().

Still need to figure out what version we want to branch on for
NEW_FLATTEN_VERSION

Added test class to show use

[reed1, 2014-03-28 14:52:52]

https://codereview.chromium.org/212103010/diff/90001/include/core/SkPathEffect.h
File include/core/SkPathEffect.h (right):

https://codereview.chromium.org/212103010/diff/90001/include/core/SkPathEffec...
include/core/SkPathEffect.h:116: *  None:
This part of the dox could be attached/associated directly with each enum below.

eg.

kNone_DashType,   //!< ignores the info parameter
kDash_DashType,   //!< fills in all of the info parameter

https://codereview.chromium.org/212103010/diff/90001/tests/AsADashTest.cpp
File tests/AsADashTest.cpp (right):

https://codereview.chromium.org/212103010/diff/90001/tests/AsADashTest.cpp#ne...
tests/AsADashTest.cpp:38: SkPathEffect::DashInfo info;
since we "read" fIntervals, and only that field, perhaps we can have the
constructor for info set that to zero?

[bsalomon, 2014-03-28 14:57:22]

https://codereview.chromium.org/212103010/diff/90001/include/core/SkPathEffect.h
File include/core/SkPathEffect.h (right):

https://codereview.chromium.org/212103010/diff/90001/include/core/SkPathEffec...
include/core/SkPathEffect.h:107: /**
Maybe I'm alone in this but I'd feel better about the comment if it didn't
reference a specific subclass but rather used language like "If the path effect
can be represented as a dash pattern"

https://codereview.chromium.org/212103010/diff/90001/include/core/SkPathEffec...
include/core/SkPathEffect.h:114: *  asAGradient again with the same info and
fIntervals will get copied in.
asAGradient -> asADash

https://codereview.chromium.org/212103010/diff/90001/include/core/SkPathEffec...
include/core/SkPathEffect.h:133: bool        fScaleToFit;
Of all the fields, this one needs a comment :) I'm not particularly familiar
with the dash effect class and the others are pretty obvious while this one is
not.

https://codereview.chromium.org/212103010/diff/90001/src/effects/SkDashPathEf...
File src/effects/SkDashPathEffect.cpp (right):

https://codereview.chromium.org/212103010/diff/90001/src/effects/SkDashPathEf...
src/effects/SkDashPathEffect.cpp:562: if (buffer.pictureVersion() <
NEW_FLATTEN_VERSION) {
Do we need this? Will we ever write an old picture format? I think we only care
about reading old ones.

[egdaniel, 2014-04-21 18:14:41]

Back to dashing...changes based on previous comments from a while ago

[egdaniel, 2014-04-21 18:14:43]

Back to dashing...changes based on previous comments from a while ago

[bsalomon, 2014-04-21 18:16:17]

lgtm

[egdaniel, 2014-04-22 13:10:16]

The CQ bit was checked by egdaniel@google.com

[commit-bot: I haz the power, 2014-04-22 13:10:21]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/egdaniel@google.com/212103010/150001

[commit-bot: I haz the power, 2014-04-22 13:26:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-22 13:26:02]

Failed to commit the patch.
Sending        gyp/tests.gypi
Sending        include/core/SkPathEffect.h
Sending        include/core/SkPicture.h
Sending        include/effects/SkDashPathEffect.h
Sending        src/core/SkPathEffect.cpp
Sending        src/effects/SkDashPathEffect.cpp
Adding         tests/AsADashTest.cpp
Transmitting file data .......svn: Commit failed (details follow):
svn: Commit failed unexpectedly --- please try again later (1)

[reed1, 2014-04-22 13:49:00]

https://codereview.chromium.org/212103010/diff/150001/include/core/SkPathEffe...
File include/core/SkPathEffect.h (right):

https://codereview.chromium.org/212103010/diff/150001/include/core/SkPathEffe...
include/core/SkPathEffect.h:123: DashInfo() : fIntervals(0), fCount(0),
fPhase(0.0) {}
fPhase(0) is fine.

Sometimes saying 0.0 will make the compiler think you have a double, and then
you could get a (stupid) warning about double->float conversion.

fIntervals(NULL) is skia's convention for pointers.

[reed1, 2014-04-22 14:15:02]

https://codereview.chromium.org/212103010/diff/170001/src/effects/SkDashPathE...
File src/effects/SkDashPathEffect.cpp (right):

https://codereview.chromium.org/212103010/diff/170001/src/effects/SkDashPathE...
src/effects/SkDashPathEffect.cpp:523: if (info->fCount >= fCount) {
nit:

if (info->fCount >= fCount && NULL != info->fIntervals) {
  ..

https://codereview.chromium.org/212103010/diff/170001/src/effects/SkDashPathE...
src/effects/SkDashPathEffect.cpp:569: fPhase = 0.0;
may get a double-warning: Try just fPhase = 0;

[reed1, 2014-04-22 14:15:36]

lgtm after my previous nits

[egdaniel, 2014-04-22 15:01:27]

The CQ bit was checked by egdaniel@google.com

[commit-bot: I haz the power, 2014-04-22 15:01:39]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/egdaniel@google.com/212103010/190001

[commit-bot: I haz the power, 2014-04-22 15:21:23]

Change committed as 14297