Bundle SkShader::asFragmentProcessor arguments in a struct

Bundle SkShader::asFragmentProcessor arguments in a struct

The signature of this thing keeps changing (and is about to change again).
This just makes maintenance much easier.

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

Committed: https://skia.googlesource.com/skia/+/839345d63466a4209a0985654ec033624821fd78

[Brian Osman, 2016-07-22 17:02:53]

Description was changed from

==========
Bundle SkShader::asFragmentProcessor arguments in a struct

The signature of this thing keeps changing (and is about to change again).
This just makes maintenance much easier.

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

to

==========
Bundle SkShader::asFragmentProcessor arguments in a struct

The signature of this thing keeps changing (and is about to change again).
This just makes maintenance much easier.

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

[Brian Osman, 2016-07-22 17:06:12]

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

[commit-bot: I haz the power, 2016-07-22 17:06:20]

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

[Brian Osman, 2016-07-22 17:19:25]

brianosman@google.com changed reviewers:
+ bsalomon@google.com, egdaniel@google.com

[Brian Osman, 2016-07-22 17:19:26]

https://codereview.chromium.org/2175563003/diff/1/include/core/SkShader.h
File include/core/SkShader.h (right):

https://codereview.chromium.org/2175563003/diff/1/include/core/SkShader.h#new...
include/core/SkShader.h:327: const SkMatrix*        fViewMatrix;
This is the only (slightly) interesting change - we used to take viewmatrix by
const&, and localmatrix by pointer (due to required/optional). But it seems like
we really shy away from storing refs inside structs/classes, and I didn't want
to be copying the view matrix by value, so I converted them both to pointers.
That leads to some de-referencing in the implementation of asFP...

[egdaniel, 2016-07-22 17:27:59]

https://codereview.chromium.org/2175563003/diff/1/include/core/SkShader.h
File include/core/SkShader.h (right):

https://codereview.chromium.org/2175563003/diff/1/include/core/SkShader.h#new...
include/core/SkShader.h:327: const SkMatrix*        fViewMatrix;
On 2016/07/22 17:19:26, Brian Osman wrote:
> This is the only (slightly) interesting change - we used to take viewmatrix by
> const&, and localmatrix by pointer (due to required/optional). But it seems
like
> we really shy away from storing refs inside structs/classes, and I didn't want
> to be copying the view matrix by value, so I converted them both to pointers.
> That leads to some de-referencing in the implementation of asFP...

personally I think the use of a ref in this use case is fine, but either is fine
with me. I'll let someone who cares more make the decision.

https://codereview.chromium.org/2175563003/diff/1/src/core/SkNormalSource.cpp
File src/core/SkNormalSource.cpp (right):

https://codereview.chromium.org/2175563003/diff/1/src/core/SkNormalSource.cpp...
src/core/SkNormalSource.cpp:177: const SkShader::AsFPArgs& args) const {
same comment as below with alignment

https://codereview.chromium.org/2175563003/diff/1/src/core/SkNormalSource.cpp...
src/core/SkNormalSource.cpp:408: const SkShader::AsFPArgs&) const {
This line seems like it either needs to be spaced all the way to 100 (the robert
phillips spacing rules), or just indented 8 spaces

[commit-bot: I haz the power, 2016-07-22 17:32:42]

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

[commit-bot: I haz the power, 2016-07-22 17:32:43]

Dry run: This issue passed the CQ dry run.

[bsalomon, 2016-07-22 17:38:11]

lgtm

https://codereview.chromium.org/2175563003/diff/1/include/core/SkShader.h
File include/core/SkShader.h (right):

https://codereview.chromium.org/2175563003/diff/1/include/core/SkShader.h#new...
include/core/SkShader.h:327: const SkMatrix*        fViewMatrix;
On 2016/07/22 17:27:59, egdaniel wrote:
> On 2016/07/22 17:19:26, Brian Osman wrote:
> > This is the only (slightly) interesting change - we used to take viewmatrix
by
> > const&, and localmatrix by pointer (due to required/optional). But it seems
> like
> > we really shy away from storing refs inside structs/classes, and I didn't
want
> > to be copying the view matrix by value, so I converted them both to
pointers.
> > That leads to some de-referencing in the implementation of asFP...
> 
> personally I think the use of a ref in this use case is fine, but either is
fine
> with me. I'll let someone who cares more make the decision.

I'm not opposed to ref but I think ptr is more consistent with how other similar
args struct work (e.g. path renderers).

[Brian Osman, 2016-07-22 17:44:17]

The CQ bit was checked by brianosman@google.com

[Brian Osman, 2016-07-22 17:44:17]

The patchset sent to the CQ was uploaded after l-g-t-m from bsalomon@google.com
Link to the patchset: https://codereview.chromium.org/2175563003/#ps20001
(title: "Fix indentation to be less arbitrary")

[commit-bot: I haz the power, 2016-07-22 17:44:23]

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-07-22 18:04:56]

Description was changed from

==========
Bundle SkShader::asFragmentProcessor arguments in a struct

The signature of this thing keeps changing (and is about to change again).
This just makes maintenance much easier.

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

to

==========
Bundle SkShader::asFragmentProcessor arguments in a struct

The signature of this thing keeps changing (and is about to change again).
This just makes maintenance much easier.

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

Committed:
https://skia.googlesource.com/skia/+/839345d63466a4209a0985654ec033624821fd78
==========

[commit-bot: I haz the power, 2016-07-22 18:04:57]

Committed patchset #2 (id:20001) as
https://skia.googlesource.com/skia/+/839345d63466a4209a0985654ec033624821fd78