Remove some validation and asserts from tessellating path renderer.

Remove some validation and asserts from tessellating path renderer.

In some cases, resolving an intersection can cause a vertex to go
slightly out-of-order with edges which have already been processed.
This doesn't cause any algorithmic errors, but it's difficult to detect
without impacting performance significantly.

Also, the GPU infrastructure fires asserts when attempting 
to allocate 0-length vertex buffers. Early-out instead, 
since there's nothing to draw.

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

[Stephen White, 2015-03-26 18:54:43]

senorblanco@chromium.org changed reviewers:
+ egdaniel@chromium.org

[Stephen White, 2015-03-26 18:55:16]

Greg: PTAL. Thanks!

[egdaniel, 2015-03-26 19:24:21]

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

[egdaniel, 2015-03-26 19:24:22]

so lgtm but some general question.

Since all this validate code only runs in Debug, do we really care about the
perf hit? Or is it so drastic that it makes running test in Debug unfeasible?

I feel like having some way of checking the math in the tessellator is of value
(outside of simply drawing) and currently these checks do some validation of
them. Maybe there is some (I'm sure it would be ugly) way to unit test them.

[Stephen White, 2015-03-26 19:29:44]

On 2015/03/26 19:24:22, egdaniel wrote:
> so lgtm but some general question.
> 
> Since all this validate code only runs in Debug, do we really care about the
> perf hit? Or is it so drastic that it makes running test in Debug unfeasible?
> 
> I feel like having some way of checking the math in the tessellator is of
value
> (outside of simply drawing) and currently these checks do some validation of
> them. Maybe there is some (I'm sure it would be ugly) way to unit test them.

The problem is that these edge cases are found by the fuzzer (and presumably
once
it hits Chrome, clusterfuzz would find them too). I'm a little disappointed to 
have to remove them, but since the algorithm does actually violate them due
to floating point inaccuracies, I can't really leave them in.

[egdaniel, 2015-03-26 19:41:28]

On 2015/03/26 19:29:44, Stephen White wrote:
> On 2015/03/26 19:24:22, egdaniel wrote:
> > so lgtm but some general question.
> > 
> > Since all this validate code only runs in Debug, do we really care about the
> > perf hit? Or is it so drastic that it makes running test in Debug
unfeasible?
> > 
> > I feel like having some way of checking the math in the tessellator is of
> value
> > (outside of simply drawing) and currently these checks do some validation of
> > them. Maybe there is some (I'm sure it would be ugly) way to unit test them.
> 
> The problem is that these edge cases are found by the fuzzer (and presumably
> once
> it hits Chrome, clusterfuzz would find them too). I'm a little disappointed to

> have to remove them, but since the algorithm does actually violate them due
> to floating point inaccuracies, I can't really leave them in.

And I'm guessing it doesn't really make sense to be testing an epsilon something
to handle these numerical issues?

[Stephen White, 2015-03-26 19:47:15]

On 2015/03/26 19:41:28, egdaniel wrote:
> On 2015/03/26 19:29:44, Stephen White wrote:
> > On 2015/03/26 19:24:22, egdaniel wrote:
> > > so lgtm but some general question.
> > > 
> > > Since all this validate code only runs in Debug, do we really care about
the
> > > perf hit? Or is it so drastic that it makes running test in Debug
> unfeasible?
> > > 
> > > I feel like having some way of checking the math in the tessellator is of
> > value
> > > (outside of simply drawing) and currently these checks do some validation
of
> > > them. Maybe there is some (I'm sure it would be ugly) way to unit test
them.
> > 
> > The problem is that these edge cases are found by the fuzzer (and presumably
> > once
> > it hits Chrome, clusterfuzz would find them too). I'm a little disappointed
to
> 
> > have to remove them, but since the algorithm does actually violate them due
> > to floating point inaccuracies, I can't really leave them in.
> 
> And I'm guessing it doesn't really make sense to be testing an epsilon
something
> to handle these numerical issues?

It's really hard to get the epsilon correct for arbitrary precision inputs; see 
https://randomascii.wordpress.com/2012/02/25/comparing-floating-point-numbers...

[egdaniel, 2015-03-26 20:01:33]

On 2015/03/26 19:47:15, Stephen White wrote:
> On 2015/03/26 19:41:28, egdaniel wrote:
> > On 2015/03/26 19:29:44, Stephen White wrote:
> > > On 2015/03/26 19:24:22, egdaniel wrote:
> > > > so lgtm but some general question.
> > > > 
> > > > Since all this validate code only runs in Debug, do we really care about
> the
> > > > perf hit? Or is it so drastic that it makes running test in Debug
> > unfeasible?
> > > > 
> > > > I feel like having some way of checking the math in the tessellator is
of
> > > value
> > > > (outside of simply drawing) and currently these checks do some
validation
> of
> > > > them. Maybe there is some (I'm sure it would be ugly) way to unit test
> them.
> > > 
> > > The problem is that these edge cases are found by the fuzzer (and
presumably
> > > once
> > > it hits Chrome, clusterfuzz would find them too). I'm a little
disappointed
> to
> > 
> > > have to remove them, but since the algorithm does actually violate them
due
> > > to floating point inaccuracies, I can't really leave them in.
> > 
> > And I'm guessing it doesn't really make sense to be testing an epsilon
> something
> > to handle these numerical issues?
> 
> It's really hard to get the epsilon correct for arbitrary precision inputs;
see 
>
https://randomascii.wordpress.com/2012/02/25/comparing-floating-point-numbers...

Interesting. Well only possible thing you may want to do is leave the code in
behind some other define so it doesn't run. You would know best but if it is
useful for general debugging of errors we could consider leaving it. Your call,
no strong opinion either way.

[Stephen White, 2015-03-26 21:47:28]

The CQ bit was checked by senorblanco@chromium.org

[commit-bot: I haz the power, 2015-03-26 21:47:42]

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

[commit-bot: I haz the power, 2015-03-26 21:52:49]

Committed patchset #1 (id:1) as
https://skia.googlesource.com/skia/+/e833107a30b240e0685b354838bf6098066da6f8