Fixing several issues with rendering blurred bitmaps with Ganesh

[junov1, 2011-06-28 21:18:01]

PTAL

[junov1, 2011-06-28 21:18:48]

bug: http://code.google.com/p/skia/issues/detail?id=305

[reed1, 2011-06-28 21:27:02]

ack! a read-back inside ganesh? lets chat when brian is back before we go
forward.

[junov, 2011-06-28 21:34:48]

Yeah... but it only happens in the case where the bitmap is previously used
for a hardware render-op, and then used as a filtered mask. Also, all this
code will go away once we get a HW implementation for blur in Ganesh.

On Tue, Jun 28, 2011 at 5:27 PM, <reed@google.com> wrote:

> ack! a read-back inside ganesh? lets chat when brian is back before we
> go forward.
>
>
>
http://codereview.appspot.com/**4630073/<http://codereview.appspot.com/4630073/>
>

[junov1, 2011-06-29 17:59:50]

Correction to my previous comment.  The readback only happens when rendering
across devices. The use case is: the device bitmap from a source device is being
drawn to another device. Therefore the pixel data resides in an FBO and needs to
be read back in order to push it through the software rendering path to apply
the mask filter. Until we have a GPU blur that is good to go, I don't see a
better solution.

On 2011/06/28 21:34:48, junov wrote:
> Yeah... but it only happens in the case where the bitmap is previously used
> for a hardware render-op, and then used as a filtered mask. Also, all this
> code will go away once we get a HW implementation for blur in Ganesh.
> 
> On Tue, Jun 28, 2011 at 5:27 PM, <mailto:reed@google.com> wrote:
> 
> > ack! a read-back inside ganesh? lets chat when brian is back before we
> > go forward.
> >
> >
> >
>
http://codereview.appspot.com/**4630073/%3Chttp://codereview.appspot.com/4630...>
> >

[reed1, 2011-06-29 19:08:15]

I think I need to support this sort of thing, but Gpu ->
Raster_device, so perhaps I'll revive my attempt at that and put you
on the CL. If that passes, I think we can avoid the need for a
special-case inside GpuDevice.

On Wed, Jun 29, 2011 at 1:59 PM,  <junov@chromium.org> wrote:
> Correction to my previous comment.  The readback only happens when
> rendering across devices. The use case is: the device bitmap from a
> source device is being drawn to another device. Therefore the pixel data
> resides in an FBO and needs to be read back in order to push it through
> the software rendering path to apply the mask filter. Until we have a
> GPU blur that is good to go, I don't see a better solution.
>
> On 2011/06/28 21:34:48, junov wrote:
>>
>> Yeah... but it only happens in the case where the bitmap is previously
>
> used
>>
>> for a hardware render-op, and then used as a filtered mask. Also, all
>
> this
>>
>> code will go away once we get a HW implementation for blur in Ganesh.
>
>> On Tue, Jun 28, 2011 at 5:27 PM, <mailto:reed@google.com> wrote:
>
>> > ack! a read-back inside ganesh? lets chat when brian is back before
>
> we
>>
>> > go forward.
>> >
>> >
>> >
>
>
http://codereview.appspot.com/**4630073/%3Chttp://codereview.appspot.com/4630...>
>>
>> >
>
>
>
> http://codereview.appspot.com/4630073/
>

[junov1, 2011-06-30 21:45:45]

I just submitted a new patch that removes the readback in the case of texture
backed bitmaps.  That problem can be fix later with the more general approach
that Mike proposed.  I still want to submit the rest of the fix to resolve
problems with sub-rect mapping and the IgnoreTransform flag.

[junov1, 2011-07-19 18:54:30]

Sorry about the hasty submission I did before.  I saw this change got rolled
back. The issues raised in the above review comments were already addressed and
no additional feedback was provided.  Anyone care to comment?

[bsalomon, 2011-07-19 18:59:56]

On 2011/07/19 18:54:30, junov1 wrote:
> Sorry about the hasty submission I did before.  I saw this change got rolled
> back. The issues raised in the above review comments were already addressed
and
> no additional feedback was provided.  Anyone care to comment?

The change caused a regression in a layout test or a real world page but I can't
recall the details. Mike looked at it...

[junov, 2011-07-19 19:04:32]

Ok, I will launch a batch of try jobs with this change in hope of finding
what the problem might have been.

On Tue, Jul 19, 2011 at 2:59 PM, <bsalomon@google.com> wrote:

> On 2011/07/19 18:54:30, junov1 wrote:
>
>> Sorry about the hasty submission I did before.  I saw this change got
>>
> rolled
>
>> back. The issues raised in the above review comments were already
>>
> addressed and
>
>> no additional feedback was provided.  Anyone care to comment?
>>
>
> The change caused a regression in a layout test or a real world page but
> I can't recall the details. Mike looked at it...
>
>
>
>
>
>
http://codereview.appspot.com/**4630073/<http://codereview.appspot.com/4630073/>
>

[bsalomon, 2011-07-19 19:12:17]

BTW Mike made changes such that lockPixels/getPixels should always succeed.
It'll do a readback if the SkBitmap is texture-backed.

[junov1, 2011-07-19 19:45:53]

On 2011/07/19 19:12:17, bsalomon wrote:
> BTW Mike made changes such that lockPixels/getPixels should always succeed.
> It'll do a readback if the SkBitmap is texture-backed.

Good to know, I just uploaded a new patch that takes that into account.  Thanks.

[junov1, 2011-07-19 19:52:37]

Ok, so as a matter of fact there are 4 layout test regressions. I am looking
into that now.

[junov1, 2011-07-21 18:43:14]

Abandoning this CL.  The test I was trying to fix was using an invalid use case
(applying a mask filter to an RGBA image).  I will fix the test and log new
issues if necessary.

On 2011/07/19 19:52:37, junov1 wrote:
> Ok, so as a matter of fact there are 4 layout test regressions. I am looking
> into that now.