Removing GraphicsContext from ImageBuffer

Removing GraphicsContext from ImageBuffer

BUG=453113
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=197051

[Justin Novosad, 2015-06-04 17:56:53]

junov@chromium.org changed reviewers:
+ chrishtr@chromium.org, senorblanco@chromium.org

[Justin Novosad, 2015-06-04 17:56:53]

PTAL

[Stephen White, 2015-06-04 18:33:10]

LGTM!

https://codereview.chromium.org/1170523002/diff/20001/Source/core/frame/Image...
File Source/core/frame/ImageBitmap.cpp (right):

https://codereview.chromium.org/1170523002/diff/20001/Source/core/frame/Image...
Source/core/frame/ImageBitmap.cpp:87:
buffer->canvas()->drawPicture(recording.get());
chrishtr@: Not new to this patch, but why are we going through the record path
here? It looks like we always record, then immediately play back the recorded
commands. Why not just draw?

[chrishtr, 2015-06-04 19:58:00]

https://codereview.chromium.org/1170523002/diff/20001/Source/core/frame/Image...
File Source/core/frame/ImageBitmap.cpp (right):

https://codereview.chromium.org/1170523002/diff/20001/Source/core/frame/Image...
Source/core/frame/ImageBitmap.cpp:87:
buffer->canvas()->drawPicture(recording.get());
Agree that this looks wrong and we should just draw here. Justin could you fix
that? Just remove the picture builder and sub-context and drawing recorder, and
simply call clip and translate on buffer->context().

[Justin Novosad, 2015-06-05 02:20:45]

On 2015/06/04 19:58:00, chrishtr wrote:
>
https://codereview.chromium.org/1170523002/diff/20001/Source/core/frame/Image...
> File Source/core/frame/ImageBitmap.cpp (right):
> 
>
https://codereview.chromium.org/1170523002/diff/20001/Source/core/frame/Image...
> Source/core/frame/ImageBitmap.cpp:87:
> buffer->canvas()->drawPicture(recording.get());
> Agree that this looks wrong and we should just draw here. Justin could you fix
> that? Just remove the picture builder and sub-context and drawing recorder,
and
> simply call clip and translate on buffer->context().

You mean "buffer->canvas()".  Yes I can do that here, and in DragImage, but not
in LocalFrame.cpp

[chrishtr, 2015-06-05 04:59:09]

On 2015/06/05 at 02:20:45, junov wrote:
> On 2015/06/04 19:58:00, chrishtr wrote:
> >
https://codereview.chromium.org/1170523002/diff/20001/Source/core/frame/Image...
> > File Source/core/frame/ImageBitmap.cpp (right):
> > 
> >
https://codereview.chromium.org/1170523002/diff/20001/Source/core/frame/Image...
> > Source/core/frame/ImageBitmap.cpp:87:
> > buffer->canvas()->drawPicture(recording.get());
> > Agree that this looks wrong and we should just draw here. Justin could you
fix
> > that? Just remove the picture builder and sub-context and drawing recorder,
and
> > simply call clip and translate on buffer->context().
> 
> You mean "buffer->canvas()".

Yes, sorry.

>  Yes I can do that here, and in DragImage, but not in LocalFrame.cpp

Right. In LocalFrame it actually paints some HTML content.

[Justin Novosad, 2015-06-12 15:52:43]

re-review ping.

This cl now draws directly into SkCanvas instead of using SkPictureBuilder in
ImageBitmap and DragImage.  Other significant change since last review: multiple
files related to mac theme painting had to be tweaked to remove uses of
GraphicsContext, and use SkCanvas directly instead.

[chrishtr, 2015-06-12 16:08:01]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2015-06-12 16:08:02]

lgtm

[chrishtr, 2015-06-12 16:08:02]

The patchset sent to the CQ was uploaded after l-g-t-m from
senorblanco@chromium.org
Link to the patchset: https://codereview.chromium.org/1170523002/#ps100001
(title: "fix mac build for realz")

[commit-bot: I haz the power, 2015-06-12 16:08:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1170523002/100001

[Justin Novosad, 2015-06-12 16:49:58]

The CQ bit was unchecked by junov@chromium.org

[Justin Novosad, 2015-06-12 17:03:15]

The CQ bit was checked by junov@chromium.org

[Justin Novosad, 2015-06-12 17:03:15]

The patchset sent to the CQ was uploaded after l-g-t-m from
chrishtr@chromium.org, senorblanco@chromium.org
Link to the patchset: https://codereview.chromium.org/1170523002/#ps120001
(title: "fix for shape bug")

[commit-bot: I haz the power, 2015-06-12 17:03:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1170523002/120001

[commit-bot: I haz the power, 2015-06-12 18:13:21]

Committed patchset #7 (id:120001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=197051