4f linear gradient shader blitters

4f linear gradient shader blitters

Add F16 specializations to support writing to half-float dests.

Add color profile template arg across the board to support writing
to sRGB dests.

R=reed@google.com,mtklein@google.com,herb@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1808963005

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

[f(malita), 2016-03-17 21:41:01]

Description was changed from

==========
4f linear gradient shader blitters
==========

to

==========
4f linear gradient shader blitters
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[f(malita), 2016-03-17 21:47:33]

Description was changed from

==========
4f linear gradient shader blitters
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
4f linear gradient shader blitters

Add F16 specializations to support writing to half-float dests.

Add color profile template arg across the board to support writing
to sRGB dests.

R=reed@google.com,mtklein@google.com,herb@google.com
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[f(malita), 2016-03-17 21:47:33]

fmalita@chromium.org changed reviewers:
+ herb@google.com, mtklein@google.com, reed@google.com

[f(malita), 2016-03-17 22:04:29]

fmalita@chromium.org changed reviewers:
- herb@google.com, mtklein@google.com

[f(malita), 2016-03-17 22:04:29]

A hack to slap shader blitters on top of the current 4f linear impl.

Since I know Mike is anxious for some numbers:

* using SampleApp --slide GM:gradient_many_stops_4f, L32 as a baseline (.51ms)
* overhead without blitters - S32: ~75% (.89ms), F16: ~15% (60ms)
* overhead with blitters - S32: ~31% (.67ms), F16: ~13% (.58ms)


So it's a significant improvement for S32, but we're still 30% slower and the
current hack makes some optimistic assumptions which will have to be addressed
(annoyingly, computing the gradient in premul space now gets in the way as now
we'll need to undo the premul for srgb conversion).

[f(malita), 2016-03-17 22:04:41]

The CQ bit was checked by fmalita@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-17 22:04:43]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1808963005/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1808963005/20001

[commit-bot: I haz the power, 2016-03-17 22:27:37]

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

[commit-bot: I haz the power, 2016-03-17 22:27:38]

Dry run: This issue passed the CQ dry run.

[reed1, 2016-03-18 14:28:59]

https://codereview.chromium.org/1808963005/diff/20001/src/effects/gradients/S...
File src/effects/gradients/Sk4fGradientPriv.h (right):

https://codereview.chromium.org/1808963005/diff/20001/src/effects/gradients/S...
src/effects/gradients/Sk4fGradientPriv.h:43: inline void store<SkPM4f,
kLinear_SkColorProfileType, false>(const Sk4f& c, SkPM4f* dst) {
Getting hard to remember what true and false mean in all of these
specializations, Perhaps an enum? { kApplyPremul_True, False }

https://codereview.chromium.org/1808963005/diff/20001/src/effects/gradients/S...
src/effects/gradients/Sk4fGradientPriv.h:71: *dst =
SkPreMultiplyARGB(SkGetPackedA32(s32), SkGetPackedR32(s32),
why not Sk4f_toS32(premul_4f(c)) ?

https://codereview.chromium.org/1808963005/diff/20001/src/effects/gradients/S...
File src/effects/gradients/Sk4fLinearGradient.cpp (right):

https://codereview.chromium.org/1808963005/diff/20001/src/effects/gradients/S...
src/effects/gradients/Sk4fLinearGradient.cpp:58: while (n > 0) {
sk_memset64() ?

[f(malita), 2016-03-18 16:54:11]

https://codereview.chromium.org/1808963005/diff/20001/src/effects/gradients/S...
File src/effects/gradients/Sk4fGradientPriv.h (right):

https://codereview.chromium.org/1808963005/diff/20001/src/effects/gradients/S...
src/effects/gradients/Sk4fGradientPriv.h:43: inline void store<SkPM4f,
kLinear_SkColorProfileType, false>(const Sk4f& c, SkPM4f* dst) {
On 2016/03/18 14:28:59, reed1 wrote:
> Getting hard to remember what true and false mean in all of these
> specializations, Perhaps an enum? { kApplyPremul_True, False }

Done.

https://codereview.chromium.org/1808963005/diff/20001/src/effects/gradients/S...
src/effects/gradients/Sk4fGradientPriv.h:71: *dst =
SkPreMultiplyARGB(SkGetPackedA32(s32), SkGetPackedR32(s32),
On 2016/03/18 14:28:59, reed1 wrote:
> why not Sk4f_toS32(premul_4f(c)) ?

Done.

https://codereview.chromium.org/1808963005/diff/20001/src/effects/gradients/S...
File src/effects/gradients/Sk4fLinearGradient.cpp (right):

https://codereview.chromium.org/1808963005/diff/20001/src/effects/gradients/S...
src/effects/gradients/Sk4fLinearGradient.cpp:58: while (n > 0) {
On 2016/03/18 14:28:59, reed1 wrote:
> sk_memset64() ?

added/done.

[f(malita), 2016-03-18 16:54:16]

The CQ bit was checked by fmalita@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-18 16:54:19]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1808963005/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1808963005/40001

[reed1, 2016-03-18 16:56:09]

Any feel how this restructuring of your templates affects your speed in the
shadeSpan4f case?

[reed1, 2016-03-18 16:58:05]

lgtm

https://codereview.chromium.org/1808963005/diff/40001/src/effects/gradients/S...
File src/effects/gradients/Sk4fGradientPriv.h (right):

https://codereview.chromium.org/1808963005/diff/40001/src/effects/gradients/S...
src/effects/gradients/Sk4fGradientPriv.h:45: inline void store<SkPM4f,
kLinear_SkColorProfileType, ApplyPremul::False>
just a question: can the "store" functions be written to return their value,
rather than take a dst* argument? Just seems like it might read easier.

[commit-bot: I haz the power, 2016-03-18 16:58:09]

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

[commit-bot: I haz the power, 2016-03-18 16:58:09]

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

[f(malita), 2016-03-18 17:03:34]

On 2016/03/18 16:56:09, reed1 wrote:
> Any feel how this restructuring of your templates affects your speed in the
> shadeSpan4f case?

There's a super tiny improvement due to refactoring dst_component_scale ->
scale_for_dest (which is now a no-op for everything except linear), but
essentially no impact.

https://codereview.chromium.org/1808963005/diff/40001/src/effects/gradients/S...
File src/effects/gradients/Sk4fGradientPriv.h (right):

https://codereview.chromium.org/1808963005/diff/40001/src/effects/gradients/S...
src/effects/gradients/Sk4fGradientPriv.h:45: inline void store<SkPM4f,
kLinear_SkColorProfileType, ApplyPremul::False>
On 2016/03/18 16:58:05, reed1 wrote:
> just a question: can the "store" functions be written to return their value,
> rather than take a dst* argument? Just seems like it might read easier.

Yeah, I was just thinking about that.  I'll be doing several cleanup passes on
this code, and will try it out.

[f(malita), 2016-03-18 17:10:30]

The CQ bit was checked by fmalita@chromium.org

[f(malita), 2016-03-18 17:10:30]

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/1808963005/#ps60001
(title: "rename trunc_from_255 to quiet confused msvc")

[commit-bot: I haz the power, 2016-03-18 17:10:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1808963005/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1808963005/60001

[f(malita), 2016-03-18 17:14:42]

On 2016/03/18 17:03:34, f(malita) wrote:
> On 2016/03/18 16:56:09, reed1 wrote:
> > Any feel how this restructuring of your templates affects your speed in the
> > shadeSpan4f case?
> 
> There's a super tiny improvement due to refactoring dst_component_scale ->
> scale_for_dest (which is now a no-op for everything except linear), but
> essentially no impact.

BTW, I'm not sure which of the changes is responsible, but now the F16 overhead
is
down to something negligible with blitter: ~2% \o/

[reed1, 2016-03-18 17:18:00]

Sweet.

Makes some sense, since the orig code also was paying for float->byte, and F16
is just float->16bit int

[commit-bot: I haz the power, 2016-03-18 17:28:26]

Description was changed from

==========
4f linear gradient shader blitters

Add F16 specializations to support writing to half-float dests.

Add color profile template arg across the board to support writing
to sRGB dests.

R=reed@google.com,mtklein@google.com,herb@google.com
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
4f linear gradient shader blitters

Add F16 specializations to support writing to half-float dests.

Add color profile template arg across the board to support writing
to sRGB dests.

R=reed@google.com,mtklein@google.com,herb@google.com
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

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

[commit-bot: I haz the power, 2016-03-18 17:28:26]

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