Added ctor to SkClipStack and isEmpty method

The new ctor is/will be used is GrContext::AutoClip to create temporary clip stacks while isEmpty is/will be used in GrClipMaskManager to potentially avoid clipping.

[bsalomon, 2012-07-26 12:28:38]

http://codereview.appspot.com/6444048/diff/1/include/core/SkClipStack.h
File include/core/SkClipStack.h (right):

http://codereview.appspot.com/6444048/diff/1/include/core/SkClipStack.h#newco...
include/core/SkClipStack.h:76: bool isEmpty() const;
IIRC Region uses "empty" to mean the exact opposite (no area). Same with Rect.
I'd propose naming it something else. isUnbounded()? isUnrestricted()?

[reed1, 2012-07-26 14:14:49]

http://codereview.appspot.com/6444048/diff/1/include/core/SkClipStack.h
File include/core/SkClipStack.h (right):

http://codereview.appspot.com/6444048/diff/1/include/core/SkClipStack.h#newco...
include/core/SkClipStack.h:76: bool isEmpty() const;
On 2012/07/26 12:28:38, bsalomon wrote:
> <span id="abbrex-spnAbbr_2601" class="abbrex-Abbr abbrex-Abbr_IIRC
abbrex-AbbrUnknown abbrex-AbbrUnknownVisible">IIRC</span> Region uses "empty" to
mean the exact opposite (no area). Same with Rect.
> I'd propose naming it something else. isUnbounded()? isUnrestricted()?

Agreed. isFull() or isWideOpen() are better matches to the SkRegion
API/semantics.

[robertphillips, 2012-07-26 15:37:27]

http://codereview.appspot.com/6444048/diff/1/include/core/SkClipStack.h
File include/core/SkClipStack.h (right):

http://codereview.appspot.com/6444048/diff/1/include/core/SkClipStack.h#newco...
include/core/SkClipStack.h:76: bool isEmpty() const;
On 2012/07/26 12:28:38, bsalomon wrote:
> IIRC Region uses "empty" to mean the exact opposite (no area). Same with Rect.
> I'd propose naming it something else. isUnbounded()? isUnrestricted()?

Done.

[bsalomon, 2012-07-26 15:46:51]

LGTM

http://codereview.appspot.com/6444048/diff/4/src/core/SkClipStack.cpp
File src/core/SkClipStack.cpp (right):

http://codereview.appspot.com/6444048/diff/4/src/core/SkClipStack.cpp#newcode556
src/core/SkClipStack.cpp:556: (kInsideOut_BoundsType ==
((Rec*)fDeque.back())->fFiniteBoundType &&
Does this bit need some unit testing?

[reed1, 2012-07-26 16:11:03]

unittests?

1. a newly created clipstack: is it wide-open?
2. a newly created clipstack passed an empty rect. is it wide-open?

[robertphillips, 2012-07-26 17:46:11]

Added unit test

http://codereview.appspot.com/6444048/diff/4/src/core/SkClipStack.cpp
File src/core/SkClipStack.cpp (right):

http://codereview.appspot.com/6444048/diff/4/src/core/SkClipStack.cpp#newcode556
src/core/SkClipStack.cpp:556: (kInsideOut_BoundsType ==
((Rec*)fDeque.back())->fFiniteBoundType &&
On 2012/07/26 15:46:52, bsalomon wrote:
> Does this bit need some unit testing?

Done.

[bsalomon, 2012-07-26 17:49:52]

LGTM

[reed1, 2012-07-26 17:59:58]

lgtm w/ question

http://codereview.appspot.com/6444048/diff/6001/src/core/SkClipStack.cpp
File src/core/SkClipStack.cpp (right):

http://codereview.appspot.com/6444048/diff/6001/src/core/SkClipStack.cpp#newc...
src/core/SkClipStack.cpp:553: 
minor, but should we consider caching the return value from back(), since we
reference it more than once?

[robertphillips, 2012-07-26 18:39:36]

committed as r4787

http://codereview.appspot.com/6444048/diff/6001/src/core/SkClipStack.cpp
File src/core/SkClipStack.cpp (right):

http://codereview.appspot.com/6444048/diff/6001/src/core/SkClipStack.cpp#newc...
src/core/SkClipStack.cpp:553: 
On 2012/07/26 17:59:58, reed1 wrote:
> minor, but should we consider caching the return value from back(), since we
> reference it more than once?

Done.