add support for new bitmapshader context

add support for new bitmapshader context

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1757993002

Committed: https://skia.googlesource.com/skia/+/19cef56344b5a5f26f802d7be34c44af36b7e797

[reed1, 2016-03-02 18:49:15]

Description was changed from

==========
add support for new bitmapshader context

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

to

==========
add support for new bitmapshader context

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[reed1, 2016-03-02 21:20:34]

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

[reed1, 2016-03-02 21:55:20]

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

[commit-bot: I haz the power, 2016-03-02 21:55:28]

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

[commit-bot: I haz the power, 2016-03-02 21:59:17]

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

[commit-bot: I haz the power, 2016-03-02 21:59:18]

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

[mtklein, 2016-03-03 01:30:38]

lgtm, with a few requests for comment clarification

https://codereview.chromium.org/1757993002/diff/20001/src/core/SkBitmapProcSh...
File src/core/SkBitmapProcShader.cpp (right):

https://codereview.chromium.org/1757993002/diff/20001/src/core/SkBitmapProcSh...
src/core/SkBitmapProcShader.cpp:125: // The context takes ownership of the
state. It will call its destructor
Having read this code, or at least in a while, it's unclear what "state" refers
to here, and which part of this takes ownership.  I guess it's the INHERITED
constructor?

https://codereview.chromium.org/1757993002/diff/20001/src/core/SkBitmapProcSh...
src/core/SkBitmapProcShader.cpp:140: // get memcpy performance/semantics
remind me (or the reader) why this is okay?

[reed1, 2016-03-03 14:05:25]

https://codereview.chromium.org/1757993002/diff/20001/src/core/SkBitmapProcSh...
File src/core/SkBitmapProcShader.cpp (right):

https://codereview.chromium.org/1757993002/diff/20001/src/core/SkBitmapProcSh...
src/core/SkBitmapProcShader.cpp:125: // The context takes ownership of the
state. It will call its destructor
On 2016/03/03 01:30:37, mtklein wrote:
> Having read this code, or at least in a while, it's unclear what "state"
refers
> to here, and which part of this takes ownership.  I guess it's the INHERITED
> constructor?

Done.

https://codereview.chromium.org/1757993002/diff/20001/src/core/SkBitmapProcSh...
src/core/SkBitmapProcShader.cpp:140: // get memcpy performance/semantics
On 2016/03/03 01:30:37, mtklein wrote:
> remind me (or the reader) why this is okay?

Done.

[reed1, 2016-03-03 14:05:30]

The CQ bit was checked by reed@google.com

[reed1, 2016-03-03 14:05:30]

The patchset sent to the CQ was uploaded after l-g-t-m from mtklein@google.com
Link to the patchset: https://codereview.chromium.org/1757993002/#ps60001
(title: "update dox")

[commit-bot: I haz the power, 2016-03-03 14:05:41]

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

[commit-bot: I haz the power, 2016-03-03 14:15:19]

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

[commit-bot: I haz the power, 2016-03-03 14:15:20]

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

[reed1, 2016-03-03 16:44:19]

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

[commit-bot: I haz the power, 2016-03-03 16:44:24]

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

[commit-bot: I haz the power, 2016-03-03 16:56:28]

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

[commit-bot: I haz the power, 2016-03-03 16:56:29]

Dry run: This issue passed the CQ dry run.

[reed1, 2016-03-03 17:05:21]

more places needed 16bit alignment (e.g. compose-shader), but seems to all work
now

I have manually disabled calling the new shader in the L32 dst case, even though
it "works". At least for now, it shows slight differences when we accidentally
feed it an sRGB tagged src, and I'm not ready to decide if that's what we want
to do or not.

Should definitely (and only) test the new shader when the Dst is sRGB or F16

[reed1, 2016-03-03 17:09:29]

The CQ bit was checked by reed@google.com

[reed1, 2016-03-03 17:09:30]

The patchset sent to the CQ was uploaded after l-g-t-m from mtklein@google.com
Link to the patchset: https://codereview.chromium.org/1757993002/#ps80001
(title: "enforce 16byte alignment in more places")

[commit-bot: I haz the power, 2016-03-03 17:09:39]

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

[reed1, 2016-03-03 17:09:58]

The CQ bit was unchecked by reed@google.com

[reed1, 2016-03-03 17:14:49]

The CQ bit was checked by reed@google.com

[commit-bot: I haz the power, 2016-03-03 17:14:58]

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

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

Description was changed from

==========
add support for new bitmapshader context

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
add support for new bitmapshader context

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/19cef56344b5a5f26f802d7be34c44af36b7e797
==========

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

Committed patchset #5 (id:80001) as
https://skia.googlesource.com/skia/+/19cef56344b5a5f26f802d7be34c44af36b7e797

[reed1, 2016-03-03 17:36:31]

A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.chromium.org/1763743002/ by reed@google.com.

The reason for reverting is: oops, need to update bench to know about the large
size needed for the new shader.