Add getRectCount to SkRegtion

Add getRectCount to SkRegtion

It is useful to know how many rects comprise a region,
since in some situations we can optimize code based on
the complexity of the region. For instance, if we use
SkRegion for tracking invalidation we might opt to use
the region bounds as invalidation instead of iterating
over each rect.

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

[vmpstr, 2013-07-16 19:40:45]

Please take a look.

[tomhudson, 2013-07-17 09:46:28]

Assuming this is a response to
https://code.google.com/p/chromium/issues/detail?id=260626 ?

LGTM, but I'd like Mike or his designee to sign off on it too.

Longer-term, should we be naming it getComplexity() as a nominally-abstract
measure, rather than getRectCount()? Or is that too abstract to be useful, and
we're sufficiently happy with the run-based implementation?

I was going to nit that we should SkASSERT(fRunHead) before dereferencing it,
but we aren't doing that elsewhere.
Bonus points for using 'comprise'.

[vmpstr, 2013-07-17 17:09:36]

On 2013/07/17 09:46:28, tomhudson wrote:
> Assuming this is a response to
> https://code.google.com/p/chromium/issues/detail?id=260626 ?
> 

Yeah this is really the only way for us to know how "big" the region is

> LGTM, but I'd like Mike or his designee to sign off on it too.
> 
> Longer-term, should we be naming it getComplexity() as a nominally-abstract
> measure, rather than getRectCount()? Or is that too abstract to be useful, and
> we're sufficiently happy with the run-based implementation?

I think getComplexity might be a bit abstract. Complexity is really dependent
on the calling code (ie, it's up to the client to decide what's complex), and
having complexity in some anonymous units might be difficult to reason about.

> 
> I was going to nit that we should SkASSERT(fRunHead) before dereferencing it,
> but we aren't doing that elsewhere.
> Bonus points for using 'comprise'.

:D

[robertphillips, 2013-07-17 17:35:10]

Mike is OOO right now.

[caryclark, 2013-07-17 17:52:44]

LGTM

Before you check it in, please add to the SkRegion.h comment to note that this
is experimental to discourage adoption until Mike gets back and can weigh in on
this.

[bsalomon, 2013-07-17 17:54:12]

On 2013/07/17 17:52:44, caryclark wrote:
> LGTM
> 
> Before you check it in, please add to the SkRegion.h comment to note that this
> is experimental to discourage adoption until Mike gets back and can weigh in
on
> this.

I agree with Cary. The comment on getIntervalCount() suggests that this is
correct, but I'd feel better about it if Mike reviews it when he gets back as
none among the rest of the Skia team understand this very well.

[caryclark, 2013-07-17 17:59:24]

Also, I assume that SkRegion::isComplex() is not a suitable short-term
substitute. Is that correct?

[vmpstr, 2013-07-17 18:05:24]

On 2013/07/17 17:59:24, caryclark wrote:
> Also, I assume that SkRegion::isComplex() is not a suitable short-term
> substitute. Is that correct?

That's correct. isComplex() can only distinguish between 1 rect and > 1 rect,
which isn't good enough. We would potentially end up with quite a few more
rasters if we use such a low bound on rects.

[vmpstr, 2013-07-17 18:14:05]

Added a comment about this being experimental. Are you guys OK with me
committing this?

[caryclark, 2013-07-17 18:14:49]

lgtm

[caryclark, 2013-07-17 18:16:42]

On 2013/07/17 18:14:49, caryclark wrote:
> lgtm

Mike just got in. Hold on a moment.

[reed1, 2013-07-17 19:14:36]

https://codereview.chromium.org/19366008/diff/12001/include/core/SkRegion.h
File include/core/SkRegion.h (right):

https://codereview.chromium.org/19366008/diff/12001/include/core/SkRegion.h#n...
include/core/SkRegion.h:89: *  Returns the number of rects that comprise this
region.
I'd like to make the definition a little fuzzier, so we're not married to this
returning exactly the same number of rects that Iter might return. How about...

/**
Returns a value that grows approximately linearly with the number of intervals
comprised in the region. Empty will return 0, Rect will return 1, Complex will
return some value > 1. Use this to compare two regions, where the larger count
likely indicates a more complex region.
*/

int computeRegionComplexity() const;

[vmpstr, 2013-07-17 19:59:08]

PTAL.

https://codereview.chromium.org/19366008/diff/12001/include/core/SkRegion.h
File include/core/SkRegion.h (right):

https://codereview.chromium.org/19366008/diff/12001/include/core/SkRegion.h#n...
include/core/SkRegion.h:89: *  Returns the number of rects that comprise this
region.
On 2013/07/17 19:14:36, reed1 wrote:
> I'd like to make the definition a little fuzzier, so we're not married to this
> returning exactly the same number of rects that Iter might return. How
about...
> 
> /**
> Returns a value that grows approximately linearly with the number of intervals
> comprised in the region. Empty will return 0, Rect will return 1, Complex will
> return some value > 1. Use this to compare two regions, where the larger count
> likely indicates a more complex region.
> */
> 
> int computeRegionComplexity() const;
> 

Done.

[reed1, 2013-07-17 20:55:45]

lgtm

[commit-bot: I haz the power, 2013-07-17 21:09:24]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/vmpstr@chromium.org/19366008/18001

[commit-bot: I haz the power, 2013-07-17 21:39:30]

Change committed as 10129