Add nine patch type to SkRRect.

Add nine patch type to SkRRect.

BUG=skia:2181
Committed: http://code.google.com/p/skia/source/detail?r=13833

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

[robertphillips, 2014-03-14 16:22:59]

https://codereview.chromium.org/196343015/diff/20001/src/core/SkRRect.cpp
File src/core/SkRRect.cpp (right):

https://codereview.chromium.org/196343015/diff/20001/src/core/SkRRect.cpp#new...
src/core/SkRRect.cpp:57: 
I think we need to scale the x & y radii separately?

https://codereview.chromium.org/196343015/diff/20001/src/core/SkRRect.cpp#new...
src/core/SkRRect.cpp:75: fType = kOval_Type;
don't we also need "&& 0 == topRad"?

https://codereview.chromium.org/196343015/diff/20001/src/core/SkRRect.cpp#new...
src/core/SkRRect.cpp:449: }
maybe a radii_make_nine_patch predicate to share code with above?

[bsalomon, 2014-03-14 16:30:19]

https://codereview.chromium.org/196343015/diff/20001/src/core/SkRRect.cpp
File src/core/SkRRect.cpp (right):

https://codereview.chromium.org/196343015/diff/20001/src/core/SkRRect.cpp#new...
src/core/SkRRect.cpp:57: 
On 2014/03/14 16:22:59, robertphillips wrote:
> I think we need to scale the x & y radii separately?

We don't in the other setters (which I just stole from).

https://codereview.chromium.org/196343015/diff/20001/src/core/SkRRect.cpp#new...
src/core/SkRRect.cpp:75: fType = kOval_Type;
On 2014/03/14 16:22:59, robertphillips wrote:
> don't we also need "&& 0 == topRad"?

I don't think so... if the top radius is 0 then so is the bottom, which means
all the corners are square, right?

Maybe I should be jamming the them to 0 here so we store them that way?

https://codereview.chromium.org/196343015/diff/20001/src/core/SkRRect.cpp#new...
src/core/SkRRect.cpp:449: }
On 2014/03/14 16:22:59, robertphillips wrote:
> maybe a radii_make_nine_patch predicate to share code with above?

I'm not sure what you mean.

[robertphillips, 2014-03-14 16:52:07]

https://codereview.chromium.org/196343015/diff/20001/src/core/SkRRect.cpp
File src/core/SkRRect.cpp (right):

https://codereview.chromium.org/196343015/diff/20001/src/core/SkRRect.cpp#new...
src/core/SkRRect.cpp:57: 
Sigh - that is correct
(http://www.w3.org/TR/2010/WD-css3-background-20100612/#the-border-radius --
Overlapping Curves).

https://codereview.chromium.org/196343015/diff/20001/src/core/SkRRect.cpp#new...
src/core/SkRRect.cpp:75: fType = kOval_Type;
You're right - probably both a comment and jamming them all to zero is in order

https://codereview.chromium.org/196343015/diff/20001/src/core/SkRRect.cpp#new...
src/core/SkRRect.cpp:449: }
Add a helper function?

[robertphillips, 2014-03-14 16:52:18]

lgtm

[reed1, 2014-03-14 17:10:13]

lgtm

[bsalomon, 2014-03-14 17:15:31]

https://codereview.chromium.org/196343015/diff/20001/src/core/SkRRect.cpp
File src/core/SkRRect.cpp (right):

https://codereview.chromium.org/196343015/diff/20001/src/core/SkRRect.cpp#new...
src/core/SkRRect.cpp:449: }
On 2014/03/14 16:52:07, robertphillips wrote:
> Add a helper function?

gotcha! will do. I thought you meant incorporating into the above loop.

[reed1, 2014-03-14 17:32:07]

Is this valuable to be public? At some level, the more diverse we make the
"type", the harder it will be for less-picky clients to make use of it.
rect/oval are very clear to me. simple-vs-complex less so, and ninepatch is even
more subtle/obscure a distinction.

[bsalomon, 2014-03-14 17:56:28]

On 2014/03/14 17:32:07, reed1 wrote:
> Is this valuable to be public? At some level, the more diverse we make the
> "type", the harder it will be for less-picky clients to make use of it.
> rect/oval are very clear to me. simple-vs-complex less so, and ninepatch is
even
> more subtle/obscure a distinction.

It's valuable to at least two parts of the GPU code. If I don't have this flag I
have to either rededuce it several times for the same rrect or pass around and
store a sidecar variable. It seems intuitively useful to me to know if the rrect
can be nine-patched (for mask filter, rendering, etc).

[reed1, 2014-03-14 18:24:47]

On 2014/03/14 17:56:28, bsalomon wrote:
> On 2014/03/14 17:32:07, reed1 wrote:
> > Is this valuable to be public? At some level, the more diverse we make the
> > "type", the harder it will be for less-picky clients to make use of it.
> > rect/oval are very clear to me. simple-vs-complex less so, and ninepatch is
> even
> > more subtle/obscure a distinction.
> 
> It's valuable to at least two parts of the GPU code. If I don't have this flag
I
> have to either rededuce it several times for the same rrect or pass around and
> store a sidecar variable. It seems intuitively useful to me to know if the
rrect
> can be nine-patched (for mask filter, rendering, etc).

I'd be fine if we cache this sort of thing, but perhaps that setting/enum can be
private.

[bsalomon, 2014-03-14 20:01:21]

On 2014/03/14 18:24:47, reed1 wrote:
> On 2014/03/14 17:56:28, bsalomon wrote:
> > On 2014/03/14 17:32:07, reed1 wrote:
> > > Is this valuable to be public? At some level, the more diverse we make the
> > > "type", the harder it will be for less-picky clients to make use of it.
> > > rect/oval are very clear to me. simple-vs-complex less so, and ninepatch
is
> > even
> > > more subtle/obscure a distinction.
> > 
> > It's valuable to at least two parts of the GPU code. If I don't have this
flag
> I
> > have to either rededuce it several times for the same rrect or pass around
and
> > store a sidecar variable. It seems intuitively useful to me to know if the
> rrect
> > can be nine-patched (for mask filter, rendering, etc).
> 
> I'd be fine if we cache this sort of thing, but perhaps that setting/enum can
be
> private.

Ok, how about we make Type private and just have isRect() and isOval() public?

[reed1, 2014-03-14 20:05:28]

On 2014/03/14 20:01:21, bsalomon wrote:
> On 2014/03/14 18:24:47, reed1 wrote:
> > On 2014/03/14 17:56:28, bsalomon wrote:
> > > On 2014/03/14 17:32:07, reed1 wrote:
> > > > Is this valuable to be public? At some level, the more diverse we make
the
> > > > "type", the harder it will be for less-picky clients to make use of it.
> > > > rect/oval are very clear to me. simple-vs-complex less so, and ninepatch
> is
> > > even
> > > > more subtle/obscure a distinction.
> > > 
> > > It's valuable to at least two parts of the GPU code. If I don't have this
> flag
> > I
> > > have to either rededuce it several times for the same rrect or pass around
> and
> > > store a sidecar variable. It seems intuitively useful to me to know if the
> > rrect
> > > can be nine-patched (for mask filter, rendering, etc).
> > 
> > I'd be fine if we cache this sort of thing, but perhaps that setting/enum
can
> be
> > private.
> 
> Ok, how about we make Type private and just have isRect() and isOval() public?

works for me!

[bsalomon, 2014-03-14 20:31:59]

On 2014/03/14 20:05:28, reed1 wrote:
> On 2014/03/14 20:01:21, bsalomon wrote:
> > On 2014/03/14 18:24:47, reed1 wrote:
> > > On 2014/03/14 17:56:28, bsalomon wrote:
> > > > On 2014/03/14 17:32:07, reed1 wrote:
> > > > > Is this valuable to be public? At some level, the more diverse we make
> the
> > > > > "type", the harder it will be for less-picky clients to make use of
it.
> > > > > rect/oval are very clear to me. simple-vs-complex less so, and
ninepatch
> > is
> > > > even
> > > > > more subtle/obscure a distinction.
> > > > 
> > > > It's valuable to at least two parts of the GPU code. If I don't have
this
> > flag
> > > I
> > > > have to either rededuce it several times for the same rrect or pass
around
> > and
> > > > store a sidecar variable. It seems intuitively useful to me to know if
the
> > > rrect
> > > > can be nine-patched (for mask filter, rendering, etc).
> > > 
> > > I'd be fine if we cache this sort of thing, but perhaps that setting/enum
> can
> > be
> > > private.
> > 
> > Ok, how about we make Type private and just have isRect() and isOval()
public?
> 
> works for me!

The list of friends is long and requires #including at least on header from src
in order to declare a friend function that takes a private enum type in its
signature.

[reed1, 2014-03-14 20:43:36]

On 2014/03/14 20:31:59, bsalomon wrote:
> On 2014/03/14 20:05:28, reed1 wrote:
> > On 2014/03/14 20:01:21, bsalomon wrote:
> > > On 2014/03/14 18:24:47, reed1 wrote:
> > > > On 2014/03/14 17:56:28, bsalomon wrote:
> > > > > On 2014/03/14 17:32:07, reed1 wrote:
> > > > > > Is this valuable to be public? At some level, the more diverse we
make
> > the
> > > > > > "type", the harder it will be for less-picky clients to make use of
> it.
> > > > > > rect/oval are very clear to me. simple-vs-complex less so, and
> ninepatch
> > > is
> > > > > even
> > > > > > more subtle/obscure a distinction.
> > > > > 
> > > > > It's valuable to at least two parts of the <span
id="abbrex-spnAbbr_17917" class="abbrex-Abbr abbrex-Abbr_GPU abbrex-AbbrUnknown
abbrex-AbbrUnknownVisible">GPU</span> code. If I don't have
> this
> > > flag
> > > > I
> > > > > have to either rededuce it several times for the same rrect or pass
> around
> > > and
> > > > > store a sidecar variable. It seems intuitively useful to me to know if
> the
> > > > rrect
> > > > > can be nine-patched (for mask filter, rendering, etc).
> > > > 
> > > > I'd be fine if we cache this sort of thing, but perhaps that
setting/enum
> > can
> > > be
> > > > private.
> > > 
> > > Ok, how about we make Type private and just have isRect() and isOval()
> public?
> > 
> > works for me!
> 
> The list of friends is long and requires #including at least on header from
src
> in order to declare a friend function that takes a private enum type in its
> signature.

Hmmm, ok, lets land as is. I'd like us to find a longer-term way to hide this,
as I think it is a class case of impl requirements leaking out into the open,
but I agree that I don't see how to do it easily right now.

[bsalomon, 2014-03-14 20:47:43]

The CQ bit was checked by bsalomon@google.com

[bsalomon, 2014-03-14 20:47:50]

The CQ bit was unchecked by bsalomon@google.com

[commit-bot: I haz the power, 2014-03-14 20:47:51]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/bsalomon@google.com/196343015/20001

[bsalomon, 2014-03-17 14:32:12]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2014-03-17 14:32:18]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/bsalomon@google.com/196343015/80001

[commit-bot: I haz the power, 2014-03-17 15:39:05]

Change committed as 13833

[bsalomon, 2014-03-17 19:47:24]

A revert of this CL has been created in
https://codereview.chromium.org/202163004/ by bsalomon@google.com.

The reason for reverting is: causes tests to have infinite loop.

[bsalomon, 2014-03-17 21:01:46]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2014-03-17 21:01:49]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/bsalomon@google.com/196343015/40002

[commit-bot: I haz the power, 2014-03-17 21:17:36]

Change committed as 13839