fixing shadow support for drawing bitmap and sprites with skia gpu device

This fixes chrome bug:
http://code.google.com/p/chromium/issues/detail?id=83440

[junov1, 2011-05-25 15:25:42]

PTAL

[junov1, 2011-05-26 17:20:58]

On 2011/05/25 15:25:42, junov1 wrote:
> PTAL

ping!

[reed1, 2011-05-26 17:30:21]

looking now...

[reed1, 2011-05-26 17:45:35]

This is tricky to get right. I think we should create a Sample that exercises a
few of the variables here:

- with and without a srcRectPtr (your code doesn't check for NULL)
- with and without filtering in the paint (will we get bleed w/ filtering when
there is a srcRect?)

Also, your current version is leaking the shader. Typical fix is to say:
    paint.setShader(SkShader::Create....)->unref();

As a possible path to follow, here is how this is handled for the raster device
(cribbed from SkDevice.cpp and SkDraw.cpp):

1. Handle the srcRectPtr.

    SkBitmap        tmp;    // storage if we need a subset of bitmap
    const SkBitmap* bitmapPtr = &bitmap;

    if (srcRect) {
        if (!bitmap.extractSubset(&tmp, *srcRect)) {
            return;     // extraction failed
        }
        bitmapPtr = &tmp;
    }

Then handle the shader.

        // preconcat the matrix onto our stack

        SkPaint shaderPaint(paint);
        shaderPaint.setShader(..*bitmapPtr..)->unref();
        SkRect  r;
        r.set(0, 0, SkIntToScalar(bitmapPtr->width()),
SkIntToScalar(bitmapPtr->height()));
        this->drawRect(r, shaderPaint);

You can alternatively (as you started) explicitly convert to a path and apply
the matrix. That has the disadvantage of always making us take the slower
path-case, since if the matrix is just scale/translate, rects will stay rects,
and we may draw faster.

[junov1, 2011-05-27 03:51:20]

> You can alternatively (as you started) explicitly convert to a path and apply
> the matrix. That has the disadvantage of always making us take the slower
> path-case, since if the matrix is just scale/translate, rects will stay rects,
> and we may draw faster.

I think I should stick with the path approach because the whole point of this
fix is to divert to the slow path in the event that we have a mask filter. I
realize this duplicates some of the logic that is already in other places, but
it avoids adding overhead to the accelerated path (copying skPaint, extract
subRect, creating a shader).

[reed1, 2011-05-27 19:27:38]

I don't see why you prefer drawPath over drawRect. This does not allow for us to
take advantage of our rect special cases. True today rects themselves turn into
paths for maskfilters, but that is a detail known only to the rect impl. If we
convert to paths here, we have to remember that, and undo this code later
if/when we make rects yet-more efficient.

That aside, we still need a test case for this inside skia. At least a Sample
that exercises some of the permutations that I mentioned earlier.

[junov1, 2011-06-09 23:40:23]

Now that the M13 rush is mostly over, I resurrected this old code review. I
think the latest upload covers all of the concerns you expressed in previous
reviews.  

Added Brian as reviewer now that he is back.

[bsalomon, 2011-06-13 17:20:55]

LGTM. (Sorry for not noticing this earlier!)


On 2011/06/09 23:40:23, junov1 wrote:
> Now that the M13 rush is mostly over, I resurrected this old code review. I
> think the latest upload covers all of the concerns you expressed in previous
> reviews.  
> 
> Added Brian as reviewer now that he is back.

[junov1, 2011-06-16 15:57:45]

Pinging Mike.  PTAL

[reed1, 2011-06-16 20:12:41]

LGTM