Add back overdraw visualization for Skia BenchmarkingCanvas

Add back overdraw visualization for Skia BenchmarkingCanvas

This was dropped during BenchmarkingCanvas refactoring. Adding back for
feature parity.

BUG=461534
R=robertphillips@google.com,reed@google.com

Committed: https://crrev.com/5fb2ffe4b404d28242d8f62fcfa6c0c1bdaddc4f
Cr-Commit-Position: refs/heads/master@{#320111}

[robertphillips, 2015-03-10 17:23:41]

lgtm

https://codereview.chromium.org/993893004/diff/20001/skia/ext/benchmarking_ca...
File skia/ext/benchmarking_canvas.cc (right):

https://codereview.chromium.org/993893004/diff/20001/skia/ext/benchmarking_ca...
skia/ext/benchmarking_canvas.cc:429: 
// TODO: seems like there should be a far faster way to do this  

if (dst.blue / 42 > 0) {
    i = 7 - dst.blue/42;
} else if (dst.green / 45 >0) {
    i = 10 - dst.green / 42;
} else if (dst.red > 128) {
    i = 10;
} else {
    i = 0;
}
SkASSERT(gTable[i] == dst);

?

https://codereview.chromium.org/993893004/diff/20001/skia/ext/benchmarking_ca...
skia/ext/benchmarking_canvas.cc:442: #endif
private:
    typedef SkXfermode INHERITED;

?

[reed1, 2015-03-10 19:02:20]

since this is "private" to a canvas subclass, I wonder if we can deliver the
effect w/o also using SkDrawFilter (which is on the chopping block of
features)...?

[f(malita), 2015-03-10 19:13:56]

On 2015/03/10 19:02:20, reed1 wrote:
> since this is "private" to a canvas subclass, I wonder if we can deliver the
> effect w/o also using SkDrawFilter (which is on the chopping block of
> features)...?

Yup, we certainly can. I thought about it and then got lazy :) Chopping block
sounds like a serious enough reason to revisit that, let me roll another
version.

[f(malita), 2015-03-10 22:28:32]

On 2015/03/10 19:13:56, f(malita) wrote:
> On 2015/03/10 19:02:20, reed1 wrote:
> > since this is "private" to a canvas subclass, I wonder if we can deliver the
> > effect w/o also using SkDrawFilter (which is on the chopping block of
> > features)...?
> 
> Yup, we certainly can. I thought about it and then got lazy :) Chopping block
> sounds like a serious enough reason to revisit that, let me roll another
> version.

Done.

https://codereview.chromium.org/993893004/diff/20001/skia/ext/benchmarking_ca...
File skia/ext/benchmarking_canvas.cc (right):

https://codereview.chromium.org/993893004/diff/20001/skia/ext/benchmarking_ca...
skia/ext/benchmarking_canvas.cc:429: 
On 2015/03/10 17:23:41, robertphillips wrote:
> // TODO: seems like there should be a far faster way to do this  
> 
> if (dst.blue / 42 > 0) {
>     i = 7 - dst.blue/42;
> } else if (dst.green / 45 >0) {
>     i = 10 - dst.green / 42;
> } else if (dst.red > 128) {
>     i = 10;
> } else {
>     i = 0;
> }
> SkASSERT(gTable[i] == dst);
> 
> ?

Refactored to use interpolation instead of table lookup.

https://codereview.chromium.org/993893004/diff/20001/skia/ext/benchmarking_ca...
skia/ext/benchmarking_canvas.cc:442: #endif
On 2015/03/10 17:23:41, robertphillips wrote:
> private:
>     typedef SkXfermode INHERITED;
> 
> ?

Done.

[robertphillips, 2015-03-11 14:07:47]

https://codereview.chromium.org/993893004/diff/60001/skia/ext/benchmarking_ca...
File skia/ext/benchmarking_canvas.cc (right):

https://codereview.chromium.org/993893004/diff/60001/skia/ext/benchmarking_ca...
skia/ext/benchmarking_canvas.cc:410: 
// Comment maybe ?
// This filter works by ...

https://codereview.chromium.org/993893004/diff/60001/skia/ext/benchmarking_ca...
skia/ext/benchmarking_canvas.cc:450: , op_params_(new base::ListValue())
What guarantees is the caller providing regarding the lifetime of the passed in
paints?

https://codereview.chromium.org/993893004/diff/60001/skia/ext/benchmarking_ca...
skia/ext/benchmarking_canvas.cc:461: 
I think even if the op has no paint you need to create a default munged paint
when in overdraw viz mode.

https://codereview.chromium.org/993893004/diff/60001/skia/ext/benchmarking_ca...
skia/ext/benchmarking_canvas.cc:465:
filtered_paint_.get()->setXfermode(canvas->overdraw_xfermode_.get());
Should disable AA in here too.

[f(malita), 2015-03-11 14:45:49]

https://codereview.chromium.org/993893004/diff/60001/skia/ext/benchmarking_ca...
File skia/ext/benchmarking_canvas.cc (right):

https://codereview.chromium.org/993893004/diff/60001/skia/ext/benchmarking_ca...
skia/ext/benchmarking_canvas.cc:410: 
On 2015/03/11 14:07:47, robertphillips wrote:
> // Comment maybe ?
> // This filter works by ...

Switched back to the color-table approach.

https://codereview.chromium.org/993893004/diff/60001/skia/ext/benchmarking_ca...
skia/ext/benchmarking_canvas.cc:450: , op_params_(new base::ListValue())
On 2015/03/11 14:07:47, robertphillips wrote:
> What guarantees is the caller providing regarding the lifetime of the passed
in
> paints?

AutoOp objects are always scoped within a given draw method, so the paint ptr is
guaranteed to be valid for their lifetime.

Added a comment.

https://codereview.chromium.org/993893004/diff/60001/skia/ext/benchmarking_ca...
skia/ext/benchmarking_canvas.cc:461: 
On 2015/03/11 14:07:47, robertphillips wrote:
> I think even if the op has no paint you need to create a default munged paint
> when in overdraw viz mode.

Good catch, done.

https://codereview.chromium.org/993893004/diff/60001/skia/ext/benchmarking_ca...
skia/ext/benchmarking_canvas.cc:465:
filtered_paint_.get()->setXfermode(canvas->overdraw_xfermode_.get());
On 2015/03/11 14:07:47, robertphillips wrote:
> Should disable AA in here too.

Done.

[robertphillips, 2015-03-11 15:19:16]

lgtm

[f(malita), 2015-03-11 15:27:08]

The CQ bit was checked by fmalita@chromium.org

[commit-bot: I haz the power, 2015-03-11 15:27:45]

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

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

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-03-11 18:10:20]

Patchset 5 (id:??) landed as
https://crrev.com/5fb2ffe4b404d28242d8f62fcfa6c0c1bdaddc4f
Cr-Commit-Position: refs/heads/master@{#320111}