more zero-length changes for svg compatibility

more zero-length changes for svg compatibility

If a path contains a moveTo followed by a line or curve,
even if the line or curve has no length, SVG expects
the end caps to draw if the cap style is round or square.

Fredrik Söderquist attached a patch to the chrome bug
(slightly modified here) that fixes layout test failures
resulting from deleting special-case code in SVG
dealing with zero-length path segments.

R=reed@google.com,fs@opera.com
BUG=22974
Committed: https://skia.googlesource.com/skia/+/62fb1ba1786863e545c89839b5706ad5151cec15

[caryclark, 2015-09-03 15:41:48]

The CQ bit was checked by caryclark@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-09-03 15:42:01]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1330623003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1330623003/1

[commit-bot: I haz the power, 2015-09-03 15:43:51]

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

[commit-bot: I haz the power, 2015-09-03 15:43:52]

Dry run: Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Shared-Trybot on client.skia
(JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)

[fs, 2015-09-03 15:49:24]

On 2015/09/03 15:41:58, caryclark wrote:

Thanks for picking it (the patch sketch) up! Hopefully the isEmpty() part can be
narrowed a bit (maybe check if |bounds| too?). Let me know if you need any help
temporarily skipping layout tests in blink (or maybe someone closer to your
timezone - like fmalita.)

(P.S. I took the liberty of fixing up my name in the description =))

[caryclark, 2015-09-03 16:39:37]

The CQ bit was checked by caryclark@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-09-03 16:39:42]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1330623003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1330623003/20001

[commit-bot: I haz the power, 2015-09-03 16:40:32]

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

[commit-bot: I haz the power, 2015-09-03 16:40:33]

Dry run: Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Shared-Trybot on client.skia
(JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)
  Build-Ubuntu-Clang-x86_64-Debug-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-Clang-x...)
  Build-Ubuntu-GCC-Arm64-Debug-Android-Trybot on client.skia.compile
(JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Arm...)
  Build-Ubuntu-GCC-Arm7-Debug-Android-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Arm...)
  Build-Ubuntu-GCC-Mips-Debug-Android-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Mip...)
  Build-Ubuntu-GCC-x86_64-Release-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-x86...)

[caryclark, 2015-09-03 16:44:43]

The CQ bit was checked by caryclark@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-09-03 16:44:52]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1330623003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1330623003/40001

[commit-bot: I haz the power, 2015-09-03 16:49:33]

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

[commit-bot: I haz the power, 2015-09-03 16:49:33]

Dry run: This issue passed the CQ dry run.

[reed1, 2015-09-03 19:07:27]

https://codereview.chromium.org/1330623003/diff/40001/include/core/SkPath.h
File include/core/SkPath.h (right):

https://codereview.chromium.org/1330623003/diff/40001/include/core/SkPath.h#n...
include/core/SkPath.h:979: bool hasOnlyMoveTos() const;
Not part of this CL, but do you know why we compute this (each time), instead of
relying on SegmentMask?

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

https://codereview.chromium.org/1330623003/diff/40001/src/core/SkCanvas.cpp#n...
src/core/SkCanvas.cpp:2129: if (path.hasOnlyMoveTos()) {
Is this test for correctness or for performance?

https://codereview.chromium.org/1330623003/diff/40001/src/core/SkStroke.cpp
File src/core/SkStroke.cpp (right):

https://codereview.chromium.org/1330623003/diff/40001/src/core/SkStroke.cpp#n...
src/core/SkStroke.cpp:622: this->lineTo(pt2);
Do we add a lineTo to trigger some follow-on heuristic? Can we comment why we do
this?

[caryclark, 2015-09-03 19:37:57]

https://codereview.chromium.org/1330623003/diff/40001/include/core/SkPath.h
File include/core/SkPath.h (right):

https://codereview.chromium.org/1330623003/diff/40001/include/core/SkPath.h#n...
include/core/SkPath.h:979: bool hasOnlyMoveTos() const;
On 2015/09/03 19:07:27, reed1 wrote:
> Not part of this CL, but do you know why we compute this (each time), instead
of
> relying on SegmentMask?

I didn't know this function existed! Certainly makes sense.

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

https://codereview.chromium.org/1330623003/diff/40001/src/core/SkCanvas.cpp#n...
src/core/SkCanvas.cpp:2129: if (path.hasOnlyMoveTos()) {
On 2015/09/03 19:07:27, reed1 wrote:
> Is this test for correctness or for performance?

Correctness. Removing this test causes EmptyPath to fail.

https://codereview.chromium.org/1330623003/diff/40001/src/core/SkStroke.cpp
File src/core/SkStroke.cpp (right):

https://codereview.chromium.org/1330623003/diff/40001/src/core/SkStroke.cpp#n...
src/core/SkStroke.cpp:622: this->lineTo(pt2);
On 2015/09/03 19:07:27, reed1 wrote:
> Do we add a lineTo to trigger some follow-on heuristic? Can we comment why we
do
> this?

Added comment (done)

[caryclark, 2015-09-03 19:38:04]

The CQ bit was checked by caryclark@google.com

[commit-bot: I haz the power, 2015-09-03 19:38:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1330623003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1330623003/60001

[commit-bot: I haz the power, 2015-09-03 19:38:13]

Note for Reviewers:
The CQ is waiting for an approval.
If you believe that the CL is not ready yet, or if you would like to L-G-T-M
with comments then please uncheck the CQ checkbox.

Waiting for LGTM from valid reviewer(s) till 2015-09-04 01:38 UTC

[commit-bot: I haz the power, 2015-09-03 19:39:16]

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

[commit-bot: I haz the power, 2015-09-03 19:39:17]

Try jobs failed on following builders:
  skia_presubmit-Trybot on client.skia.fyi (JOB_FAILED,
http://build.chromium.org/p/client.skia.fyi/builders/skia_presubmit-Trybot/bu...)

[reed1, 2015-09-04 16:23:15]

suggesting we land and compare this GM
https://codereview.chromium.org/1311503008/

[caryclark, 2015-09-08 13:26:57]

The CQ bit was checked by caryclark@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-09-08 13:27:03]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1330623003/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1330623003/100001

[commit-bot: I haz the power, 2015-09-08 13:33:15]

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

[commit-bot: I haz the power, 2015-09-08 13:33:16]

Dry run: This issue passed the CQ dry run.

[caryclark, 2015-09-08 17:39:08]

Mike, is there anything more you'd like me to do on this?

[reed1, 2015-09-09 13:51:30]

lgtm

[caryclark, 2015-09-09 13:59:23]

The CQ bit was checked by caryclark@google.com

[commit-bot: I haz the power, 2015-09-09 13:59:29]

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

[commit-bot: I haz the power, 2015-09-09 14:04:37]

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

[dogben, 2015-09-09 16:32:02]

On 2015/09/09 at 14:04:37, commit-bot wrote:
> Committed patchset #6 (id:100001) as
https://skia.googlesource.com/skia/+/62fb1ba1786863e545c89839b5706ad5151cec15

It seems this caused the DEPS roll to fail:
https://codereview.chromium.org/1325843005

[caryclark, 2015-09-09 16:34:06]

A revert of this CL (patchset #6 id:100001) has been created in
https://codereview.chromium.org/1334543002/ by caryclark@google.com.

The reason for reverting is: breaks DEPS.