Move logic from NativeImageSkia to GraphicsContext and SkiaUtils

Move logic from NativeImageSkia to GraphicsContext and SkiaUtils

Related to https://codereview.chromium.org/358893002: in this CL, preparePaint[...] is used  in NativeImageSkia and will be used in StaticBitmapImage::draw()

To avoid having a huge CL (and because it is possible), this change is proposed as a separate CL.

BUG=344804, 170021
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=178847

[Rémi Piotaix, 2014-07-23 15:41:11]

PTAL

[jbroman, 2014-07-23 15:48:59]

just a drive-by comment

https://codereview.chromium.org/416543002/diff/1/Source/platform/graphics/Gra...
File Source/platform/graphics/GraphicsContext.h (right):

https://codereview.chromium.org/416543002/diff/1/Source/platform/graphics/Gra...
Source/platform/graphics/GraphicsContext.h:414: void preparePaint(
drive-by: please use a more descriptive name than "preparePaint". What kind of
paint is this preparing, and where should it be used? It's not obvious what
srcRect and destRect mean in this context.

[Justin Novosad, 2014-07-23 17:19:05]

https://codereview.chromium.org/416543002/diff/20001/Source/platform/graphics...
File Source/platform/graphics/GraphicsContext.h (right):

https://codereview.chromium.org/416543002/diff/20001/Source/platform/graphics...
Source/platform/graphics/GraphicsContext.h:240: SkColorFilter* colorFilter()
const;
could the return value be const as well?

[Rémi Piotaix, 2014-07-23 17:27:22]

On 2014/07/23 15:48:59, jbroman wrote:
> just a drive-by comment
> 
>
https://codereview.chromium.org/416543002/diff/1/Source/platform/graphics/Gra...
> File Source/platform/graphics/GraphicsContext.h (right):
> 
>
https://codereview.chromium.org/416543002/diff/1/Source/platform/graphics/Gra...
> Source/platform/graphics/GraphicsContext.h:414: void preparePaint(
> drive-by: please use a more descriptive name than "preparePaint". What kind of
> paint is this preparing, and where should it be used? It's not obvious what
> srcRect and destRect mean in this context.

Renamed in preparePaintForDrawRectToRect

[Rémi Piotaix, 2014-07-23 17:28:23]

https://codereview.chromium.org/416543002/diff/1/Source/platform/graphics/Gra...
File Source/platform/graphics/GraphicsContext.h (right):

https://codereview.chromium.org/416543002/diff/1/Source/platform/graphics/Gra...
Source/platform/graphics/GraphicsContext.h:414: void preparePaint(
On 2014/07/23 15:48:59, jbroman wrote:
> drive-by: please use a more descriptive name than "preparePaint". What kind of
> paint is this preparing, and where should it be used? It's not obvious what
> srcRect and destRect mean in this context.

Done.

https://codereview.chromium.org/416543002/diff/20001/Source/platform/graphics...
File Source/platform/graphics/GraphicsContext.h (right):

https://codereview.chromium.org/416543002/diff/20001/Source/platform/graphics...
Source/platform/graphics/GraphicsContext.h:240: SkColorFilter* colorFilter()
const;
On 2014/07/23 17:19:05, junov wrote:
> could the return value be const as well?

Maybe. I just added the 'const' to be able to call this method from preparePaint
which is const.

[Justin Novosad, 2014-07-23 17:49:21]

> 
>
https://codereview.chromium.org/416543002/diff/20001/Source/platform/graphics...
> Source/platform/graphics/GraphicsContext.h:240: SkColorFilter* colorFilter()
> const;
> On 2014/07/23 17:19:05, junov wrote:
> > could the return value be const as well?
> 
> Maybe. I just added the 'const' to be able to call this method from
preparePaint
> which is const.

I understand, but logically we should propagate the const to aggregated objects
that are owned by the GraphicsContext when accessed through a const accessor.
Otherwise, the const constraint loses much of its significance.

[Stephen Chennney, 2014-07-23 18:10:09]

Could you please explain in the CL the reason why this is necessary? I'm
negative on changes that just move stuff into GraphicsContext that is used by a
single external method, and a "native Skia" method at that.

[Rémi Piotaix, 2014-07-23 18:28:50]

On 2014/07/23 18:10:09, Stephen Chenney wrote:
> Could you please explain in the CL the reason why this is necessary? I'm
> negative on changes that just move stuff into GraphicsContext that is used by
a
> single external method, and a "native Skia" method at that.

Done, issue updated.

[Stephen Chennney, 2014-07-23 19:04:41]

Thanks. That addresses my concern. I'll let one of the primary commenters give
the final approval.

[Justin Novosad, 2014-07-24 14:18:18]

On 2014/07/23 19:04:41, Stephen Chenney wrote:
> Thanks. That addresses my concern. I'll let one of the primary commenters give
> the final approval.

Looks good, but Rémi is still working on some layout test regressions with this
patch that seem related to texture filtering modes.  I'll wait to see the final
version before a approving.

[Justin Novosad, 2014-07-24 14:46:41]

On 2014/07/24 14:18:18, junov wrote:
> On 2014/07/23 19:04:41, Stephen Chenney wrote:
> > Thanks. That addresses my concern. I'll let one of the primary commenters
give
> > the final approval.
> 
> Looks good, but Rémi is still working on some layout test regressions with
this
> patch that seem related to texture filtering modes.  I'll wait to see the
final
> version before a approving.

lgtm

[Rémi Piotaix, 2014-07-24 15:01:11]

The CQ bit was checked by piotaixr@chromium.org

[commit-bot: I haz the power, 2014-07-24 15:02:00]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/piotaixr@chromium.org/416543002/80001

[commit-bot: I haz the power, 2014-07-24 15:52:45]

Change committed as 178847