change the hairline detection threshold to <=1.0 from <1.0

The actual change is the one-liner in SkDraw.cpp. The rest of the patch is scaffolding to the benchmark to measure the impact. The existing lines bench test used only sizes 7 and 0 and wouldn't be affected by this change. I added -strokeWidth=[float] to benchmain.cpp which overrides the default widths.

The gm suit passes.

Examples of benchmark output AFTER the change:

$ ./out/bench/bench -match lines -repeat 20 -strokeWidth 0
running bench [640 480]            lines  8888: msecs =    4.65, fps =  215.05   565: msecs =    3.55, fps =  281.69
$ ./out/bench/bench -match lines -repeat 20 -strokeWidth 0.99
running bench [640 480]            lines  8888: msecs =    6.30, fps =  158.73   565: msecs =    3.60, fps =  277.78
$ ./out/bench/bench -match lines -repeat 20 -strokeWidth 1.00
running bench [640 480]            lines  8888: msecs =    5.25, fps =  190.48   565: msecs =    3.90, fps =  256.41
$ ./out/bench/bench -match lines -repeat 20 -strokeWidth 1.01
running bench [640 480]            lines  8888: msecs =   19.55, fps =   51.15   565: msecs =   17.80, fps =   56.18
$ ./out/bench/bench -match lines -repeat 20 -strokeWidth 7
running bench [640 480]            lines  8888: msecs =   24.40, fps =   40.98   565: msecs =   22.10, fps =   45.25

Demonstrating the difference with the benchmark output BEFORE the change to SkDraw.cpp:

$ ./out/bench/bench -match lines -repeat 20 -strokeWidth 0.99
running bench [640 480]            lines  8888: msecs =    6.60, fps =  151.52   565: msecs =    3.65, fps =  273.97
$ ./out/bench/bench -match lines -repeat 20 -strokeWidth 1.00
running bench [640 480]            lines  8888: msecs =   20.25, fps =   49.38   565: msecs =   18.45, fps =   54.20
$ ./out/bench/bench -match lines -repeat 20 -strokeWidth 1.01
running bench [640 480]            lines  8888: msecs =   19.30, fps =   51.81   565: msecs =   17.70, fps =   56.50

[reed1, 2010-04-07 15:42:38]

Is that antialiased? Can you run the bench in both modes, to see both sets
of timings? (since the bw and aa hairline cases are different).

On Wed, Apr 7, 2010 at 11:21 AM, <anatoly@google.com> wrote:

> Reviewers: reed_google.com,
>
> Description:
> The actual change is the one-liner in SkDraw.cpp. The rest of the patch
> is scaffolding to the benchmark to measure the impact. The existing
> lines bench test used only sizes 7 and 0 and wouldn't be affected by
> this change. I added -strokeWidth=[float] to benchmain.cpp which
> overrides the default widths.
>
> The gm suit passes.
>
> Examples of benchmark output AFTER the change:
>
> $ ./out/bench/bench -match lines -repeat 20 -strokeWidth 0
> running bench [640 480]            lines  8888: msecs =    4.65, fps =
> 215.05   565: msecs =    3.55, fps =  281.69
> $ ./out/bench/bench -match lines -repeat 20 -strokeWidth 0.99
> running bench [640 480]            lines  8888: msecs =    6.30, fps =
> 158.73   565: msecs =    3.60, fps =  277.78
> $ ./out/bench/bench -match lines -repeat 20 -strokeWidth 1.00
> running bench [640 480]            lines  8888: msecs =    5.25, fps =
> 190.48   565: msecs =    3.90, fps =  256.41
> $ ./out/bench/bench -match lines -repeat 20 -strokeWidth 1.01
> running bench [640 480]            lines  8888: msecs =   19.55, fps =
> 51.15   565: msecs =   17.80, fps =   56.18
> $ ./out/bench/bench -match lines -repeat 20 -strokeWidth 7
> running bench [640 480]            lines  8888: msecs =   24.40, fps =
> 40.98   565: msecs =   22.10, fps =   45.25
>
> Demonstrating the difference with the benchmark output BEFORE the change
> to SkDraw.cpp:
>
> $ ./out/bench/bench -match lines -repeat 20 -strokeWidth 0.99
> running bench [640 480]            lines  8888: msecs =    6.60, fps =
> 151.52   565: msecs =    3.65, fps =  273.97
> $ ./out/bench/bench -match lines -repeat 20 -strokeWidth 1.00
> running bench [640 480]            lines  8888: msecs =   20.25, fps =
> 49.38   565: msecs =   18.45, fps =   54.20
> $ ./out/bench/bench -match lines -repeat 20 -strokeWidth 1.01
> running bench [640 480]            lines  8888: msecs =   19.30, fps =
> 51.81   565: msecs =   17.70, fps =   56.50
>
>
> Please review this at http://codereview.appspot.com/883047/show
>
> Affected files:
>  bench/RectBench.cpp
>  bench/SkBenchmark.cpp
>  bench/SkBenchmark.h
>  bench/benchmain.cpp
>  src/core/SkDraw.cpp
>
>
>

[anatoly, 2010-04-07 19:23:45]

This was antialiased. Here's the numbers with -forceAA 0, and -repeat 50 to
improve stability:

$ ./out/bench/bench -repeat 50 -strokeWidth 0.99 -match lines -forceAA 0
running bench [640 480]            lines  8888: msecs =    2.38, fps =
 420.17   565: msecs =    1.92, fps =  520.83
$ ./out/bench/bench -repeat 50 -strokeWidth 1.00 -match lines -forceAA 0
running bench [640 480]            lines  8888: msecs =    2.08, fps =
 480.77   565: msecs =    1.48, fps =  675.68
$ ./out/bench/bench -repeat 50 -strokeWidth 1.01 -match lines -forceAA 0
running bench [640 480]            lines  8888: msecs =    4.86, fps =
 205.76   565: msecs =    4.60, fps =  217.39

Before the change:

$ ./out/bench/bench -repeat 50 -strokeWidth 0.99 -match lines -forceAA 0
running bench [640 480]            lines  8888: msecs =    2.44, fps =
 409.84   565: msecs =    1.94, fps =  515.46
$ ./out/bench/bench -repeat 50 -strokeWidth 1.00 -match lines -forceAA 0
running bench [640 480]            lines  8888: msecs =    5.10, fps =
 196.08   565: msecs =    4.56, fps =  219.30
$ ./out/bench/bench -repeat 50 -strokeWidth 1.01 -match lines -forceAA 0
running bench [640 480]            lines  8888: msecs =    4.98, fps =
 200.80   565: msecs =    4.62, fps =  216.45


On Wed, Apr 7, 2010 at 6:40 PM, Mike Reed <reed@google.com> wrote:

> Is that antialiased? Can you run the bench in both modes, to see both sets
> of timings? (since the bw and aa hairline cases are different).
>
>
> On Wed, Apr 7, 2010 at 11:21 AM, <anatoly@google.com> wrote:
>
>> Reviewers: reed_google.com,
>>
>> Description:
>> The actual change is the one-liner in SkDraw.cpp. The rest of the patch
>> is scaffolding to the benchmark to measure the impact. The existing
>> lines bench test used only sizes 7 and 0 and wouldn't be affected by
>> this change. I added -strokeWidth=[float] to benchmain.cpp which
>> overrides the default widths.
>>
>> The gm suit passes.
>>
>> Examples of benchmark output AFTER the change:
>>
>> $ ./out/bench/bench -match lines -repeat 20 -strokeWidth 0
>> running bench [640 480]            lines  8888: msecs =    4.65, fps =
>> 215.05   565: msecs =    3.55, fps =  281.69
>> $ ./out/bench/bench -match lines -repeat 20 -strokeWidth 0.99
>> running bench [640 480]            lines  8888: msecs =    6.30, fps =
>> 158.73   565: msecs =    3.60, fps =  277.78
>> $ ./out/bench/bench -match lines -repeat 20 -strokeWidth 1.00
>> running bench [640 480]            lines  8888: msecs =    5.25, fps =
>> 190.48   565: msecs =    3.90, fps =  256.41
>> $ ./out/bench/bench -match lines -repeat 20 -strokeWidth 1.01
>> running bench [640 480]            lines  8888: msecs =   19.55, fps =
>> 51.15   565: msecs =   17.80, fps =   56.18
>> $ ./out/bench/bench -match lines -repeat 20 -strokeWidth 7
>> running bench [640 480]            lines  8888: msecs =   24.40, fps =
>> 40.98   565: msecs =   22.10, fps =   45.25
>>
>> Demonstrating the difference with the benchmark output BEFORE the change
>> to SkDraw.cpp:
>>
>> $ ./out/bench/bench -match lines -repeat 20 -strokeWidth 0.99
>> running bench [640 480]            lines  8888: msecs =    6.60, fps =
>> 151.52   565: msecs =    3.65, fps =  273.97
>> $ ./out/bench/bench -match lines -repeat 20 -strokeWidth 1.00
>> running bench [640 480]            lines  8888: msecs =   20.25, fps =
>> 49.38   565: msecs =   18.45, fps =   54.20
>> $ ./out/bench/bench -match lines -repeat 20 -strokeWidth 1.01
>> running bench [640 480]            lines  8888: msecs =   19.30, fps =
>> 51.81   565: msecs =   17.70, fps =   56.50
>>
>>
>> Please review this at http://codereview.appspot.com/883047/show
>>
>> Affected files:
>>  bench/RectBench.cpp
>>  bench/SkBenchmark.cpp
>>  bench/SkBenchmark.h
>>  bench/benchmain.cpp
>>  src/core/SkDraw.cpp
>>
>>
>>
>


-- 
Anatoly Vorobey
Software Engineer,
Google Israel

[reed, 2010-04-08 14:29:22]

http://codereview.appspot.com/883047/diff/1/2
File bench/benchmain.cpp (right):

http://codereview.appspot.com/883047/diff/1/2#newcode203
bench/benchmain.cpp:203: const char* matchStr = NULL;
for clarity in this section, lets rename these to include the word stroke. e.g.
hasStrokeWidth, etc.

[anatoly, 2010-04-08 14:45:09]

http://codereview.appspot.com/883047/diff/1/2
File bench/benchmain.cpp (right):

http://codereview.appspot.com/883047/diff/1/2#newcode203
bench/benchmain.cpp:203: const char* matchStr = NULL;

Done and I'm uploading a new patch set with this (I assume this is how it's done
here, haven't used Rietveld before).

[anatoly, 2010-04-08 14:45:45]

fix width->strokeWidth etc.

[reed, 2010-04-09 16:41:19]

LGTM