Add SkColor4f serialization

Add SkColor4f serialization

Adjusted usage in color shader, and will also be using this
in gradients, soon.

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

Committed: https://skia.googlesource.com/skia/+/97bbf8211fa35bbb0811d5835cb0eaaf37cc679e

[Brian Osman, 2016-09-13 19:51:29]

Description was changed from

==========
Add SkColor4f serialization

Upcoming changes to gradients are going to need something like this.
One odd hole with this implementation is that no one other than color4f
needs explicit float support in reader/writer, so it's not in the base
class buffer interfaces. But the implementation of validating read buffer
requires it, (while the non-validating read buffer forwards to the
reader). So the implementation of readFloat in validating read buffer is
non-virtual. Obviously, I could expand out the API to include float and
floatArray in readBuffer and writeBuffer. Or this whole thing could be a
waste of time, if we decide that SkScalar will forever be float. Thoughts?

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

to

==========
Add SkColor4f serialization

Upcoming changes to gradients are going to need something like this.
One odd hole with this implementation is that no one other than color4f
needs explicit float support in reader/writer, so it's not in the base
class buffer interfaces. But the implementation of validating read buffer
requires it, (while the non-validating read buffer forwards to the
reader). So the implementation of readFloat in validating read buffer is
non-virtual. Obviously, I could expand out the API to include float and
floatArray in readBuffer and writeBuffer. Or this whole thing could be a
waste of time, if we decide that SkScalar will forever be float. Thoughts?

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

[Brian Osman, 2016-09-13 19:52:43]

brianosman@google.com changed reviewers:
+ mtklein@google.com, reed@google.com

[Brian Osman, 2016-09-13 20:04:52]

Description was changed from

==========
Add SkColor4f serialization

Upcoming changes to gradients are going to need something like this.
One odd hole with this implementation is that no one other than color4f
needs explicit float support in reader/writer, so it's not in the base
class buffer interfaces. But the implementation of validating read buffer
requires it, (while the non-validating read buffer forwards to the
reader). So the implementation of readFloat in validating read buffer is
non-virtual. Obviously, I could expand out the API to include float and
floatArray in readBuffer and writeBuffer. Or this whole thing could be a
waste of time, if we decide that SkScalar will forever be float. Thoughts?

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

to

==========
Add SkColor4f serialization

Upcoming changes to gradients are going to need something like this.
One odd hole with this implementation is that no one other than color4f
needs explicit float support in reader/writer, so it's not in the base
class buffer interfaces. But the implementation of validating read buffer
requires it, (while the non-validating read buffer forwards to the
reader). So the implementation of readFloat in validating read buffer is
non-virtual. Obviously, I could expand out the API to include float and
floatArray in readBuffer and writeBuffer. Or this whole thing could be a
waste of time, if we decide that SkScalar will forever be float. Thoughts?

Edit: Version two just uses the existing scalar API on reader/writer,
and uses static_assert to ensure that we fix things if that ever changes.

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

[Brian Osman, 2016-09-13 20:06:28]

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

[commit-bot: I haz the power, 2016-09-13 20:06:43]

Dry run: 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-09-13 20:13:10]

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

[commit-bot: I haz the power, 2016-09-13 20:13:11]

Dry run: Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Shared-Trybot on
master.client.skia (JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)

[Brian Osman, 2016-09-13 20:23:42]

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

[commit-bot: I haz the power, 2016-09-13 20:23:50]

Dry run: 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-09-13 21:09:04]

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

[commit-bot: I haz the power, 2016-09-13 21:09:05]

Dry run: This issue passed the CQ dry run.

[reed1, 2016-09-13 23:17:03]

I weary a little of all of the one-off specializations in our "virtuals" for
read/write buffers. Sure seems redundant to have to reimplement these in
ValidatingReadBuffer when the code is (nearly) the same. Hopefully we'll come up
with something more scalable in the future.

https://codereview.chromium.org/2334123003/diff/40001/src/core/SkReadBuffer.cpp
File src/core/SkReadBuffer.cpp (right):

https://codereview.chromium.org/2334123003/diff/40001/src/core/SkReadBuffer.c...
src/core/SkReadBuffer.cpp:146: void SkReadBuffer::readColor4f(SkColor4f* color)
{
Is this any simpler/more-efficient?

memcpy(color, fReader.skip(sizeof(SkColor4f)), sizeof(SkColor4f));

We use this pattern for rects, irects etc.

[reed1, 2016-09-13 23:18:07]

https://codereview.chromium.org/2334123003/diff/40001/src/core/SkWriteBuffer.cpp
File src/core/SkWriteBuffer.cpp (right):

https://codereview.chromium.org/2334123003/diff/40001/src/core/SkWriteBuffer....
src/core/SkWriteBuffer.cpp:82: void SkBinaryWriteBuffer::writeColor4f(const
SkColor4f& color) {
I've bee using just write to make these simpler/faster.

fWriter.write(&color, sizeof(color));

[Brian Osman, 2016-09-14 12:56:43]

On 2016/09/13 23:18:07, reed1 wrote:
>
https://codereview.chromium.org/2334123003/diff/40001/src/core/SkWriteBuffer.cpp
> File src/core/SkWriteBuffer.cpp (right):
> 
>
https://codereview.chromium.org/2334123003/diff/40001/src/core/SkWriteBuffer....
> src/core/SkWriteBuffer.cpp:82: void SkBinaryWriteBuffer::writeColor4f(const
> SkColor4f& color) {
> I've bee using just write to make these simpler/faster.
> 
> fWriter.write(&color, sizeof(color));

Well, we could go the other direction and get rid of many of the one-offs:

template<class T>
void SkReadBuffer::readPod(T* value) {
  static_assert(std::is_pod<T>::value);
  memcpy(value, fReader.skip(sizeof(T)), sizeof(T));
}

Similar pattern would work in ValidatingReadBuffer, etc...

[Brian Osman, 2016-09-14 12:58:46]

On 2016/09/14 12:56:43, Brian Osman wrote:
> On 2016/09/13 23:18:07, reed1 wrote:
> >
>
https://codereview.chromium.org/2334123003/diff/40001/src/core/SkWriteBuffer.cpp
> > File src/core/SkWriteBuffer.cpp (right):
> > 
> >
>
https://codereview.chromium.org/2334123003/diff/40001/src/core/SkWriteBuffer....
> > src/core/SkWriteBuffer.cpp:82: void SkBinaryWriteBuffer::writeColor4f(const
> > SkColor4f& color) {
> > I've bee using just write to make these simpler/faster.
> > 
> > fWriter.write(&color, sizeof(color));
> 
> Well, we could go the other direction and get rid of many of the one-offs:
> 
> template<class T>
> void SkReadBuffer::readPod(T* value) {
>   static_assert(std::is_pod<T>::value);
>   memcpy(value, fReader.skip(sizeof(T)), sizeof(T));
> }
> 
> Similar pattern would work in ValidatingReadBuffer, etc...

... and although I like this pattern, we do lose semantic information in the
debugger's JSON code.

[mtklein, 2016-09-14 12:59:00]

On 2016/09/14 12:56:43, Brian Osman wrote:
> On 2016/09/13 23:18:07, reed1 wrote:
> >
>
https://codereview.chromium.org/2334123003/diff/40001/src/core/SkWriteBuffer.cpp
> > File src/core/SkWriteBuffer.cpp (right):
> > 
> >
>
https://codereview.chromium.org/2334123003/diff/40001/src/core/SkWriteBuffer....
> > src/core/SkWriteBuffer.cpp:82: void SkBinaryWriteBuffer::writeColor4f(const
> > SkColor4f& color) {
> > I've bee using just write to make these simpler/faster.
> > 
> > fWriter.write(&color, sizeof(color));
> 
> Well, we could go the other direction and get rid of many of the one-offs:
> 
> template<class T>
> void SkReadBuffer::readPod(T* value) {
>   static_assert(std::is_pod<T>::value);
>   memcpy(value, fReader.skip(sizeof(T)), sizeof(T));
> }
> 
> Similar pattern would work in ValidatingReadBuffer, etc...

Yes!  my vote is definitely to treat all POD uniformly.

[reed1, 2016-09-14 13:46:22]

I don't want to block progress : lgtm

[Brian Osman, 2016-09-25 19:41:01]

Description was changed from

==========
Add SkColor4f serialization

Upcoming changes to gradients are going to need something like this.
One odd hole with this implementation is that no one other than color4f
needs explicit float support in reader/writer, so it's not in the base
class buffer interfaces. But the implementation of validating read buffer
requires it, (while the non-validating read buffer forwards to the
reader). So the implementation of readFloat in validating read buffer is
non-virtual. Obviously, I could expand out the API to include float and
floatArray in readBuffer and writeBuffer. Or this whole thing could be a
waste of time, if we decide that SkScalar will forever be float. Thoughts?

Edit: Version two just uses the existing scalar API on reader/writer,
and uses static_assert to ensure that we fix things if that ever changes.

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

to

==========
Add SkColor4f serialization

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

[Brian Osman, 2016-09-25 19:42:03]

Description was changed from

==========
Add SkColor4f serialization

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

to

==========
Add SkColor4f serialization

Adjusted usage in color shader, and will also be using this
in gradients, soon.

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

[Brian Osman, 2016-09-25 19:42:11]

The CQ bit was checked by brianosman@google.com

[Brian Osman, 2016-09-25 19:42:11]

The patchset sent to the CQ was uploaded after l-g-t-m from reed@google.com
Link to the patchset: https://codereview.chromium.org/2334123003/#ps60001
(title: "Switched to memcpy, rebased")

[commit-bot: I haz the power, 2016-09-25 19:42:14]

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-09-25 20:16:02]

Description was changed from

==========
Add SkColor4f serialization

Adjusted usage in color shader, and will also be using this
in gradients, soon.

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

to

==========
Add SkColor4f serialization

Adjusted usage in color shader, and will also be using this
in gradients, soon.

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

Committed:
https://skia.googlesource.com/skia/+/97bbf8211fa35bbb0811d5835cb0eaaf37cc679e
==========

[commit-bot: I haz the power, 2016-09-25 20:16:03]

Committed patchset #4 (id:60001) as
https://skia.googlesource.com/skia/+/97bbf8211fa35bbb0811d5835cb0eaaf37cc679e