Implement Fill() for incomplete decodes to RGBA_F16

Implement Fill() for incomplete decodes to RGBA_F16

Before this patch, we would hit an SkASSERT(false).

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

Committed: https://skia.googlesource.com/skia/+/f7eb6fc71abd7649d65a877e7a10d1060afc0c88

[msarett, 2016-09-13 14:05:43]

Description was changed from

==========
Fill correctly on incomplete decodes to RGBA_F16

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

to

==========
Fill correctly on incomplete decodes to RGBA_F16

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

[msarett, 2016-09-13 14:23:13]

Description was changed from

==========
Fill correctly on incomplete decodes to RGBA_F16

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

to

==========
Fill correctly on incomplete decodes to RGBA_F16

Before this patch, we would hit an SkASSERT(false).

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

[msarett, 2016-09-13 14:26:42]

Patchset #1 (id:1) has been deleted

[msarett, 2016-09-13 14:30:53]

msarett@google.com changed reviewers:
+ reed@google.com, scroggo@google.com

[msarett, 2016-09-13 14:30:54]

We probably still need a redesign of how we handle incomplete
images (Chrome always fills with transparent).

This is pretty much just a bug fix.

https://codereview.chromium.org/2335203002/diff/20001/src/codec/SkSampler.cpp
File src/codec/SkSampler.cpp (right):

https://codereview.chromium.org/2335203002/diff/20001/src/codec/SkSampler.cpp...
src/codec/SkSampler.cpp:34: sk_memset32((uint32_t*) dstRow, color, width);
There's not a big performance difference between calling sk_memset32() in a loop
vs. just one time (I measured this for a different reason for Android a few
weeks ago).

Also, skmemset_X() does not have stricter alignment requirements than our own
inline implementations.

So we are fine to just have one implementation that calls sk_memset() in a loop.

[msarett, 2016-09-13 14:31:07]

Description was changed from

==========
Fill correctly on incomplete decodes to RGBA_F16

Before this patch, we would hit an SkASSERT(false).

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

to

==========
Implement Fill() for incomplete decodes to RGBA_F16

Before this patch, we would hit an SkASSERT(false).

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

[reed1, 2016-09-13 14:41:29]

looks tricky to me
lgtm

https://codereview.chromium.org/2335203002/diff/20001/include/codec/SkCodec.h
File include/codec/SkCodec.h (right):

https://codereview.chromium.org/2335203002/diff/20001/include/codec/SkCodec.h...
include/codec/SkCodec.h:620: * kRGBA_F16_SkColorType: Transparent or Black,
depending on the src alpha type
Is this list your "default" values? Not clear from the comment.

[msarett, 2016-09-13 14:47:24]

https://codereview.chromium.org/2335203002/diff/20001/include/codec/SkCodec.h
File include/codec/SkCodec.h (right):

https://codereview.chromium.org/2335203002/diff/20001/include/codec/SkCodec.h...
include/codec/SkCodec.h:620: * kRGBA_F16_SkColorType: Transparent or Black,
depending on the src alpha type
On 2016/09/13 14:41:29, reed1 wrote:
> Is this list your "default" values? Not clear from the comment.

Improving comments.

[msarett, 2016-09-13 14:47:37]

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

[commit-bot: I haz the power, 2016-09-13 14:47:39]

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 14:57:51]

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

[commit-bot: I haz the power, 2016-09-13 14:57:52]

Dry run: Try jobs failed on following builders:
  Build-Win-MSVC-x86_64-Release-Vulkan-Trybot on master.client.skia.compile
(JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Win-MSVC-x86_6...)

[scroggo, 2016-09-13 15:03:16]

https://codereview.chromium.org/2335203002/diff/40001/gyp/codec_android.gyp
File gyp/codec_android.gyp (right):

https://codereview.chromium.org/2335203002/diff/40001/gyp/codec_android.gyp#n...
gyp/codec_android.gyp:28: '../src/core',
Is this for SkHalf.h? Which is referenced by SkCodec.cpp, that should be in
codec.gyp?

https://codereview.chromium.org/2335203002/diff/40001/src/codec/SkCodec.cpp
File src/codec/SkCodec.cpp (right):

https://codereview.chromium.org/2335203002/diff/40001/src/codec/SkCodec.cpp#n...
src/codec/SkCodec.cpp:361: uint64_t opaqueColor = ((uint64_t) SK_Half1) << 48;
Can these be constexpr?

[msarett, 2016-09-13 15:23:55]

Also uploaded a test image

https://codereview.chromium.org/2335203002/diff/40001/gyp/codec_android.gyp
File gyp/codec_android.gyp (right):

https://codereview.chromium.org/2335203002/diff/40001/gyp/codec_android.gyp#n...
gyp/codec_android.gyp:28: '../src/core',
On 2016/09/13 15:03:16, scroggo wrote:
> Is this for SkHalf.h? Which is referenced by SkCodec.cpp, that should be in
> codec.gyp?

This is for SkColorSpaceXform.h (included by SkCodecPriv.h).

https://codereview.chromium.org/2335203002/diff/40001/src/codec/SkCodec.cpp
File src/codec/SkCodec.cpp (right):

https://codereview.chromium.org/2335203002/diff/40001/src/codec/SkCodec.cpp#n...
src/codec/SkCodec.cpp:361: uint64_t opaqueColor = ((uint64_t) SK_Half1) << 48;
On 2016/09/13 15:03:16, scroggo wrote:
> Can these be constexpr?

Yes, done.

[msarett, 2016-09-13 15:30:20]

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

[commit-bot: I haz the power, 2016-09-13 15:30:24]

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

[scroggo, 2016-09-13 15:48:51]

On 2016/09/13 15:23:55, msarett wrote:
> Also uploaded a test image

LGTM

> 
> https://codereview.chromium.org/2335203002/diff/40001/gyp/codec_android.gyp
> File gyp/codec_android.gyp (right):
> 
>
https://codereview.chromium.org/2335203002/diff/40001/gyp/codec_android.gyp#n...
> gyp/codec_android.gyp:28: '../src/core',
> On 2016/09/13 15:03:16, scroggo wrote:
> > Is this for SkHalf.h? Which is referenced by SkCodec.cpp, that should be in
> > codec.gyp?
> 
> This is for SkColorSpaceXform.h (included by SkCodecPriv.h).
> 
> https://codereview.chromium.org/2335203002/diff/40001/src/codec/SkCodec.cpp
> File src/codec/SkCodec.cpp (right):
> 
>
https://codereview.chromium.org/2335203002/diff/40001/src/codec/SkCodec.cpp#n...
> src/codec/SkCodec.cpp:361: uint64_t opaqueColor = ((uint64_t) SK_Half1) << 48;
> On 2016/09/13 15:03:16, scroggo wrote:
> > Can these be constexpr?
> 
> Yes, done.

[commit-bot: I haz the power, 2016-09-13 16:00:24]

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

[commit-bot: I haz the power, 2016-09-13 16:00:25]

Dry run: This issue passed the CQ dry run.

[msarett, 2016-09-13 16:02:47]

The CQ bit was checked by msarett@google.com

[msarett, 2016-09-13 16:02:47]

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/2335203002/#ps80001
(title: "Fix win builds")

[commit-bot: I haz the power, 2016-09-13 16:02:53]

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 16:04:13]

Description was changed from

==========
Implement Fill() for incomplete decodes to RGBA_F16

Before this patch, we would hit an SkASSERT(false).

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

to

==========
Implement Fill() for incomplete decodes to RGBA_F16

Before this patch, we would hit an SkASSERT(false).

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

Committed:
https://skia.googlesource.com/skia/+/f7eb6fc71abd7649d65a877e7a10d1060afc0c88
==========

[commit-bot: I haz the power, 2016-09-13 16:04:15]

Committed patchset #4 (id:80001) as
https://skia.googlesource.com/skia/+/f7eb6fc71abd7649d65a877e7a10d1060afc0c88