Change code to not store Sk4* in data structures.

Change code to not store Sk4* in data structures.

This allows the SkLinearBitmapPipeline system to avoid problems with alignment. This allows it to naturally fit with the rest of the system, and simplifies surrounding code.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2486523002

Committed: https://skia.googlesource.com/skia/+/93b0650084285400102f2d65d5c4411185111085

[herb_g, 2016-11-07 21:14:44]

Description was changed from

==========
Change code to not store Sk4* in data structures.

BUG=skia:
==========

to

==========
Change code to not store Sk4* in data structures.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2486523002
==========

[herb_g, 2016-11-07 21:15:55]

The CQ bit was checked by herb@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-07 21:16:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[herb_g, 2016-11-07 21:16:17]

Description was changed from

==========
Change code to not store Sk4* in data structures.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2486523002
==========

to

==========
Change code to not store Sk4* in data structures.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2486523002
==========

[herb_g, 2016-11-07 21:16:17]

herb@google.com changed reviewers:
+ mtklein@google.com

[mtklein_C, 2016-11-07 21:48:14]

mtklein@chromium.org changed reviewers:
+ mtklein@chromium.org

[mtklein_C, 2016-11-07 21:48:15]

https://codereview.chromium.org/2486523002/diff/1/src/core/SkLinearBitmapPipe...
File src/core/SkLinearBitmapPipeline.h (right):

https://codereview.chromium.org/2486523002/diff/1/src/core/SkLinearBitmapPipe...
src/core/SkLinearBitmapPipeline.h:168: alignas(SkLinearBitmapPipeline) mutable
char fPipelineStorage[sizeof(SkLinearBitmapPipeline)];
Does this really need to be mutable?  Isn't it only changed in init()?

https://codereview.chromium.org/2486523002/diff/1/src/core/SkLinearBitmapPipe...
src/core/SkLinearBitmapPipeline.h:169: bool inited {false};
fInited, or fIsInitialized to be consistent with what you wrote above?

https://codereview.chromium.org/2486523002/diff/1/src/core/SkLinearBitmapPipe...
File src/core/SkLinearBitmapPipeline_sample.h (right):

https://codereview.chromium.org/2486523002/diff/1/src/core/SkLinearBitmapPipe...
src/core/SkLinearBitmapPipeline_sample.h:77: float fTintColor[4];
This may be a good place to use SkColor4f.

[commit-bot: I haz the power, 2016-11-07 22:23:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-07 22:23:32]

Dry run: This issue passed the CQ dry run.

[herb_g, 2016-11-08 14:58:35]

Address comments

[herb_g, 2016-11-08 14:59:34]

PTRL

https://codereview.chromium.org/2486523002/diff/1/src/core/SkLinearBitmapPipe...
File src/core/SkLinearBitmapPipeline.h (right):

https://codereview.chromium.org/2486523002/diff/1/src/core/SkLinearBitmapPipe...
src/core/SkLinearBitmapPipeline.h:168: alignas(SkLinearBitmapPipeline) mutable
char fPipelineStorage[sizeof(SkLinearBitmapPipeline)];
On 2016/11/07 21:48:15, mtklein_C wrote:
> Does this really need to be mutable?  Isn't it only changed in init()?

It really does. The const on get() makes it so. In unique_ptr get() is const,
but through a pointer to the data. Here the data is in the object and needs to
be mutable to behave like a pointer to memory.

https://codereview.chromium.org/2486523002/diff/1/src/core/SkLinearBitmapPipe...
src/core/SkLinearBitmapPipeline.h:169: bool inited {false};
On 2016/11/07 21:48:15, mtklein_C wrote:
> fInited, or fIsInitialized to be consistent with what you wrote above?

Done.

https://codereview.chromium.org/2486523002/diff/1/src/core/SkLinearBitmapPipe...
File src/core/SkLinearBitmapPipeline_sample.h (right):

https://codereview.chromium.org/2486523002/diff/1/src/core/SkLinearBitmapPipe...
src/core/SkLinearBitmapPipeline_sample.h:77: float fTintColor[4];
On 2016/11/07 21:48:15, mtklein_C wrote:
> This may be a good place to use SkColor4f.

Done.

[herb_g, 2016-11-08 15:01:22]

Description was changed from

==========
Change code to not store Sk4* in data structures.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2486523002
==========

to

==========
Change code to not store Sk4* in data structures.

This allows the SkLinearBitmapPipeline system to avoid problems with alignment.
This allows it to naturally fit with the rest of the system, and simplifies
surrounding code.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2486523002
==========

[herb_g, 2016-11-08 15:29:35]

The CQ bit was checked by herb@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-08 15:29:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mtklein_C, 2016-11-08 15:33:48]

On 2016/11/08 at 14:59:34, herb wrote:
> PTRL

What's this mean?

[mtklein_C, 2016-11-08 15:38:16]

lgtm

https://codereview.chromium.org/2486523002/diff/20001/src/core/SkLinearBitmap...
File src/core/SkLinearBitmapPipeline_sample.h (right):

https://codereview.chromium.org/2486523002/diff/20001/src/core/SkLinearBitmap...
src/core/SkLinearBitmapPipeline_sample.h:69:
set_alpha(Sk4f_from_SkColor(tintColor), 1.0f).store(&fTintColor);
I was thinking something more like

fTintColor    = SkColor4f::FromColor(tintColor);
fTintColor.fA = 1.0f;

[herb_g, 2016-11-08 15:46:40]

Address comments 2

[herb_g, 2016-11-08 15:47:48]

On 2016/11/08 15:33:48, mtklein_C wrote:
> On 2016/11/08 at 14:59:34, herb wrote:
> > PTRL
> 
> What's this mean?

It means the the medicine has not kicked in yet this morning, and my hands are
working right.

[herb_g, 2016-11-08 15:48:24]

https://codereview.chromium.org/2486523002/diff/20001/src/core/SkLinearBitmap...
File src/core/SkLinearBitmapPipeline_sample.h (right):

https://codereview.chromium.org/2486523002/diff/20001/src/core/SkLinearBitmap...
src/core/SkLinearBitmapPipeline_sample.h:69:
set_alpha(Sk4f_from_SkColor(tintColor), 1.0f).store(&fTintColor);
On 2016/11/08 15:38:16, mtklein_C wrote:
> I was thinking something more like
> 
> fTintColor    = SkColor4f::FromColor(tintColor);
> fTintColor.fA = 1.0f;

Done.

[herb_g, 2016-11-08 15:49:03]

The CQ bit was checked by herb@google.com

[herb_g, 2016-11-08 15:49:03]

The patchset sent to the CQ was uploaded after l-g-t-m from mtklein@chromium.org
Link to the patchset: https://codereview.chromium.org/2486523002/#ps40001
(title: "Address comments 2")

[commit-bot: I haz the power, 2016-11-08 15:49:11]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-08 16:19:09]

Description was changed from

==========
Change code to not store Sk4* in data structures.

This allows the SkLinearBitmapPipeline system to avoid problems with alignment.
This allows it to naturally fit with the rest of the system, and simplifies
surrounding code.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2486523002
==========

to

==========
Change code to not store Sk4* in data structures.

This allows the SkLinearBitmapPipeline system to avoid problems with alignment.
This allows it to naturally fit with the rest of the system, and simplifies
surrounding code.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2486523002

Committed:
https://skia.googlesource.com/skia/+/93b0650084285400102f2d65d5c4411185111085
==========

[commit-bot: I haz the power, 2016-11-08 16:19:10]

Committed patchset #3 (id:40001) as
https://skia.googlesource.com/skia/+/93b0650084285400102f2d65d5c4411185111085