Support tiling bitmaps outside clip bounds in SkPDFImageShader

Restyle SkPDFImageShader and support tiling bitmaps outside clip bounds

BUG=chromium:99458
Committed: http://code.google.com/p/skia/source/detail?r=10870

[ducky, 2013-08-20 00:21:56]

A lot of this CL turned out to be re-styling code and renaming functions /
variables to be (in my opinion) more intuitive, but the main new functionality
is to allow the pattern cell to be outside the clip bounds by extending the
device bounds to encompass both.

[vandebo (ex-Chrome), 2013-08-20 17:22:19]

Are there existing GMs that demonstrate the problem you are fixing here?  If not
we should add one.

https://codereview.chromium.org/22884013/diff/1/src/pdf/SkPDFShader.cpp
File src/pdf/SkPDFShader.cpp (right):

https://codereview.chromium.org/22884013/diff/1/src/pdf/SkPDFShader.cpp#newco...
src/pdf/SkPDFShader.cpp:848: const SkBitmap* image = &fState.get()->fImage;
I think you don't want to do this for clamp mode.  In clamp mode, not drawing
the things that will get clipped is the right thing to do.

https://codereview.chromium.org/22884013/diff/1/src/pdf/SkPDFShader.cpp#newco...
src/pdf/SkPDFShader.cpp:865: if (deviceBounds.left() < 0 || deviceBounds.top() <
0) {
Why do this conditionally?  What if the pattern is located off the right side or
the bottom of the canvas?

[ducky, 2013-08-21 00:44:08]

Added a test case and optimized the clamping code. Dissecting the PDF shows the
the whole image isn't included in the output PDF.

https://codereview.chromium.org/22884013/diff/1/src/pdf/SkPDFShader.cpp
File src/pdf/SkPDFShader.cpp (right):

https://codereview.chromium.org/22884013/diff/1/src/pdf/SkPDFShader.cpp#newco...
src/pdf/SkPDFShader.cpp:848: const SkBitmap* image = &fState.get()->fImage;
On 2013/08/20 17:22:19, vandebo wrote:
> I think you don't want to do this for clamp mode.  In clamp mode, not drawing
> the things that will get clipped is the right thing to do.

Done.

https://codereview.chromium.org/22884013/diff/1/src/pdf/SkPDFShader.cpp#newco...
src/pdf/SkPDFShader.cpp:865: if (deviceBounds.left() < 0 || deviceBounds.top() <
0) {
On 2013/08/20 17:22:19, vandebo wrote:
> Why do this conditionally?  What if the pattern is located off the right side
or
> the bottom of the canvas?

The bitmap origin is at (0, 0), so the joined rectangle (if the clip bounds are
at the right / bottom side) should be anchored to (0, 0).

Though with clamp mode no longer including the bitmap, the conditional has been
removed.

[vandebo (ex-Chrome), 2013-08-21 21:32:43]

LGTM after nits.

https://codereview.chromium.org/22884013/diff/6001/src/pdf/SkPDFShader.cpp
File src/pdf/SkPDFShader.cpp (right):

https://codereview.chromium.org/22884013/diff/6001/src/pdf/SkPDFShader.cpp#ne...
src/pdf/SkPDFShader.cpp:852: // or not the pattern cell is in it.
nit: for clamping mode there isn't really a pattern cell because it's not
repeated => "whether or not the main bitmap is in it."

https://codereview.chromium.org/22884013/diff/6001/src/pdf/SkPDFShader.cpp#ne...
src/pdf/SkPDFShader.cpp:883: SkScalar width = image->width();
nit: you dropped SkIntToScalar

[vandebo (ex-Chrome), 2013-08-21 21:37:08]

Oops, forgot to look at the GM...

https://codereview.chromium.org/22884013/diff/6001/gm/clippedbitmapshaders.cpp
File gm/clippedbitmapshaders.cpp (right):

https://codereview.chromium.org/22884013/diff/6001/gm/clippedbitmapshaders.cp...
gm/clippedbitmapshaders.cpp:34: pixels[0] = SkPackARGB32(0xFF, 0xFF, 0x00,
0x00);
SkPreMultiplyColor(Sk_ColorRED)?

[commit-bot: I haz the power, 2013-08-21 23:05:28]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/richardlin@chromium.org/22884013/22001

[ducky, 2013-08-21 23:05:35]

Committing

https://codereview.chromium.org/22884013/diff/6001/gm/clippedbitmapshaders.cpp
File gm/clippedbitmapshaders.cpp (right):

https://codereview.chromium.org/22884013/diff/6001/gm/clippedbitmapshaders.cp...
gm/clippedbitmapshaders.cpp:34: pixels[0] = SkPackARGB32(0xFF, 0xFF, 0x00,
0x00);
On 2013/08/21 21:37:08, vandebo wrote:
> SkPreMultiplyColor(Sk_ColorRED)?

Done.

https://codereview.chromium.org/22884013/diff/6001/src/pdf/SkPDFShader.cpp
File src/pdf/SkPDFShader.cpp (right):

https://codereview.chromium.org/22884013/diff/6001/src/pdf/SkPDFShader.cpp#ne...
src/pdf/SkPDFShader.cpp:852: // or not the pattern cell is in it.
On 2013/08/21 21:32:43, vandebo wrote:
> nit: for clamping mode there isn't really a pattern cell because it's not
> repeated => "whether or not the main bitmap is in it."

Done.

https://codereview.chromium.org/22884013/diff/6001/src/pdf/SkPDFShader.cpp#ne...
src/pdf/SkPDFShader.cpp:883: SkScalar width = image->width();
On 2013/08/21 21:32:43, vandebo wrote:
> nit: you dropped SkIntToScalar

Done.

[commit-bot: I haz the power, 2013-08-21 23:10:50]

Change committed as 10870