Reimplement drawBitmapRectToRect to correctly handle fraction srcRect.

Reimplement drawBitmapRectToRect to correctly handle fraction srcRect.

The prev impl relied on drawBitmap "deducing" the destination rect by applying
the computed matrix to the bitmap's bounds. This cannot be done if the srcRect
is fractional, and therefore not representable w/ a bitmap.

The new impl computes the same matrix, but calls down to the device via drawRect
+ a bitmap_shader. This allows us to specfiy the dstRect explicitly.

The possible down-side is that we now rely on the device subclass to efficiently
handle draRect+shader, instead of calling its drawBitmap entry-point.

To give the device the chance to handle this differently, I now call through to
a new device virtual: drawBitmapRect. The default impl is to create the shader
and call drawRect, but a subclass can intercept that.

For now, the GPU override of drawBitmapRect is mimicing the old behavior (by
rounding the srcRect to an iRect). This preserves its ability to call drawBitmap
which handles very-large textures, but shows some gittering/imprecision, due to
the rounding. ... this is the same GPU behavior we have before this CL.
Committed: https://code.google.com/p/skia/source/detail?r=5663

[Steve VanDeBogart, 2012-09-24 20:40:51]

Could this be implemented by accounting for any fractional values in src rect to
enlarge the dst rect and then also applying a clip to the passed dst rect?

[robertphillips, 2012-09-25 14:22:22]

nits

https://codereview.appspot.com/6542065/diff/3001/include/core/SkDevice.h
File include/core/SkDevice.h (right):

https://codereview.appspot.com/6542065/diff/3001/include/core/SkDevice.h#newc...
include/core/SkDevice.h:253: /**
bimtap -> bitmap

https://codereview.appspot.com/6542065/diff/3001/src/core/SkDevice.cpp
File src/core/SkDevice.cpp (right):

https://codereview.appspot.com/6542065/diff/3001/src/core/SkDevice.cpp#newcod...
src/core/SkDevice.cpp:363: // Compute matrix from the two rectangles
NULL !=

https://codereview.appspot.com/6542065/diff/3001/src/core/SkDevice.cpp#newcod...
src/core/SkDevice.cpp:375: // needed (if the src was clipped). No check needed
if src==null.
NULL !=

[reed1, 2012-09-25 14:25:37]

patch #4 adds GPU override, restoring the previous behavior:
1) still has discretization problem when src is actually fractional
2) preserves calling drawBitmap, which correctly handles very-large bitmaps

I leave it to a later CL to address #1

[robertphillips, 2012-09-25 15:05:54]

LGTM

[bsalomon, 2012-09-25 15:08:00]

On 2012/09/25 15:05:54, robertphillips wrote:
> LGTM

LGTM

[Steve VanDeBogart, 2012-09-25 17:31:31]

My question was not necessarily to propose a different solution across the
board, but to understand the problem being solved....

I think this will work correctly for PDF, but it will introduce a lot of PDF
command overhead that will probably cause rendering of the PDF to be slower.

[reed1, 2012-09-25 17:38:23]

nocolorbleed_pdf now seems to draw correctly (certainly more correctly than
before) w/ the new drawRect approach. I can't tell if that's just a fluke, or
that the drawRect+shader logic in PDF is more robust.

[Steve VanDeBogart, 2012-09-25 18:47:39]

On 2012/09/25 17:38:23, reed1 wrote:
> nocolorbleed_pdf now seems to draw correctly (certainly more correctly than
> before) w/ the new drawRect approach. I can't tell if that's just a fluke, or
> that the drawRect+shader logic in PDF is more robust.

It looks like there's a scaling bug in the drawBitmap code.  It also increases
the file size from 1578 to 5500 bytes.  Is the only problem you're solving with
this change to handle fractional values in src rect?  Could this be implemented
by accounting for any fractional values in src rect to enlarge the dst rect and
then also applying a clip to the passed dst rect?  That would have minimal size
impact for PDF.

[reed1, 2012-09-25 20:02:54]

drawBitmapRect() is now a virtual on SkDevice, so SkPDFDevice is free to
implement that in whatever way is most efficient. SkGpuDevice is already moving
in that direction.