Altered complexclip GM to test out saveLayer w/ a bound and clipping

This feature appears to be currently untested and I would like to be able to catch regressions when GrClip is replaced with SkClipStack.

[bsalomon, 2012-07-12 13:40:11]

LGTM


On Thu, Jul 12, 2012 at 9:32 AM, <robertphillips@google.com> wrote:

> Reviewers: bsalomon, reed1,
>
> Description:
> This feature appears to be currently untested and I would like to be
> able to catch regressions when GrClip is replaced with SkClipStack.
>
> Please review this at
http://codereview.appspot.com/**6344108/<http://codereview.appspot.com/6344108/>
>
> Affected files:
>   M     gm/complexclip.cpp
>
>
> Index: gm/complexclip.cpp
> ==============================**==============================**=======
> --- gm/complexclip.cpp  (revision 4559)
> +++ gm/complexclip.cpp  (working copy)
> @@ -19,8 +19,11 @@
>
>  class ComplexClipGM : public GM {
>      bool fDoAAClip;
> +    bool fSaveLayer;
>  public:
> -       ComplexClipGM(bool aaclip) : fDoAAClip(aaclip) {
> +    ComplexClipGM(bool aaclip, bool saveLayer)
> +    : fDoAAClip(aaclip)
> +    , fSaveLayer(saveLayer) {
>          this->setBGColor(0xFFDDDDDD);
>  //        this->setBGColor(**SkColorSetRGB(0xB0,0xDD,0xB0))**;
>      }
> @@ -29,7 +32,9 @@
>
>      SkString onShortName() {
>          SkString str;
> -        str.printf("complexclip_%s", fDoAAClip ? "aa" : "bw");
> +        str.printf("complexclip_%s%s",
> +                   fDoAAClip ? "aa" : "bw",
> +                   fSaveLayer ? "_layer" : "");
>          return str;
>      }
>
> @@ -90,6 +95,15 @@
>          canvas->translate(**SkIntToScalar(20), SkIntToScalar(20));
>          canvas->scale(3 * SK_Scalar1 / 4, 3 * SK_Scalar1 / 4);
>
> +        if (fSaveLayer) {
> +            SkRect bounds = SkRect::MakeXYWH(100, 100, 1000, 750);
> +            SkPaint boundPaint;
> +            boundPaint.setColor(SK_**ColorRED);
> +            boundPaint.setStyle(SkPaint::**kStroke_Style);
> +            canvas->drawRect(bounds, boundPaint);
> +            canvas->saveLayer(&bounds, NULL);
> +        }
> +
>          for (int invBits = 0; invBits < 4; ++invBits) {
>              canvas->save();
>              for (size_t op = 0; op < SK_ARRAY_COUNT(gOps); ++op) {
> @@ -129,6 +143,10 @@
>              canvas->restore();
>              canvas->translate(0, SkIntToScalar(250));
>          }
> +
> +        if (fSaveLayer) {
> +            canvas->restore();
> +        }
>      }
>  private:
>      void drawHairlines(SkCanvas* canvas, const SkPath& path,
> @@ -154,10 +172,17 @@
>
>  //////////////////////////////**//////////////////////////////**
> //////////////////
>
> -static GM* gFact0(void*) { return new ComplexClipGM(false); }
> -static GM* gFact1(void*) { return new ComplexClipGM(true); }
> +// aliased and anti-aliased w/o a layer
> +static GM* gFact0(void*) { return new ComplexClipGM(false, false); }
> +static GM* gFact1(void*) { return new ComplexClipGM(true, false); }
>
> +// aliased and anti-aliased w/ a layer
> +static GM* gFact2(void*) { return new ComplexClipGM(false, true); }
> +static GM* gFact3(void*) { return new ComplexClipGM(true, true); }
> +
>  static GMRegistry gReg0(gFact0);
>  static GMRegistry gReg1(gFact1);
> +static GMRegistry gReg2(gFact2);
> +static GMRegistry gReg3(gFact3);
>
>  }
>
>
>

[robertphillips, 2012-07-12 14:16:30]

committed as r4561

[reed1, 2012-07-12 15:00:04]

lgtm w/ clarity suggestions

http://codereview.appspot.com/6344108/diff/1/gm/complexclip.cpp
File gm/complexclip.cpp (right):

http://codereview.appspot.com/6344108/diff/1/gm/complexclip.cpp#newcode25
gm/complexclip.cpp:25: : fDoAAClip(aaclip)
rename to fDoSaveLayer, to match fDoAAClip?

http://codereview.appspot.com/6344108/diff/1/gm/complexclip.cpp#newcode98
gm/complexclip.cpp:98: if (fSaveLayer) {
minor, but you can query the current size, and just inset from that, instead of
hard-coding these values. (getISize)

[robertphillips, 2012-07-12 19:22:35]

http://codereview.appspot.com/6344108/diff/1/gm/complexclip.cpp
File gm/complexclip.cpp (right):

http://codereview.appspot.com/6344108/diff/1/gm/complexclip.cpp#newcode25
gm/complexclip.cpp:25: : fDoAAClip(aaclip)
On 2012/07/12 15:00:04, reed1 wrote:
> rename to fDoSaveLayer, to match fDoAAClip?

Done.

http://codereview.appspot.com/6344108/diff/1/gm/complexclip.cpp#newcode98
gm/complexclip.cpp:98: if (fSaveLayer) {
Done - it gets a bit ugly b.c. of the scale and translate which is why I
originally opted for the hard coded version.

[reed1, 2012-07-13 14:41:58]

lgtm

[robertphillips, 2012-07-13 14:55:48]

committed as r4596

[robertphillips, 2012-07-13 15:18:46]

Fixed bug (calling SkIntToScalar on a float) in r4597