Make SkFunction copyable so it can go in containers.

Make SkFunction copyable so it can go in containers.

This totally overhauls the implementation to use ordinary inheritance-based type erasure.  I give up for now getting my manual vtable shenanigans to work with MSVC.  Still those same "expected ; before ), also expected ) before ;" errors.

I added support for uninitialized SkFunctions and operator=(), because it was fairly straightforward with this implementation.

The main downside here is that I've removed the inline implementation.  All SkFunctions involve a heap allocation, even when just wrapping function pointers.

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/0d992db1cf5003340409ed02993266004cc0a57c

[mtklein_C, 2015-04-01 21:30:42]

mtklein@chromium.org changed reviewers:
+ bungeman@google.com

[mtklein_C, 2015-04-01 21:30:47]

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

[commit-bot: I haz the power, 2015-04-01 21:31:15]

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

[mtklein_C, 2015-04-01 21:49:35]

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

[commit-bot: I haz the power, 2015-04-01 21:49:58]

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

[mtklein, 2015-04-01 21:55:41]

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

[commit-bot: I haz the power, 2015-04-01 21:56:01]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1056673002/60001

[commit-bot: I haz the power, 2015-04-01 22:15:20]

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

[commit-bot: I haz the power, 2015-04-01 22:15:20]

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

[mtklein_C, 2015-04-01 22:48:25]

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

[commit-bot: I haz the power, 2015-04-01 22:48:42]

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

[commit-bot: I haz the power, 2015-04-01 22:50:14]

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

[commit-bot: I haz the power, 2015-04-01 22:50:17]

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...)
  Build-Ubuntu-Clang-x86_64-Debug-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-Clang-x...)

[mtklein_C, 2015-04-01 22:52:22]

Patchset #5 (id:80001) has been deleted

[mtklein, 2015-05-05 20:16:47]

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

[commit-bot: I haz the power, 2015-05-05 20:17:11]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1056673002/60001

[commit-bot: I haz the power, 2015-05-05 20:20:50]

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

[commit-bot: I haz the power, 2015-05-05 20:20:51]

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

[mtklein, 2015-05-05 22:13:56]

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

[commit-bot: I haz the power, 2015-05-05 22:14:24]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1056673002/100001

[commit-bot: I haz the power, 2015-05-05 22:18:38]

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

[commit-bot: I haz the power, 2015-05-05 22:18:40]

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...)

[mtklein_C, 2015-05-06 13:27:02]

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

[commit-bot: I haz the power, 2015-05-06 13:27:21]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1056673002/120001

[commit-bot: I haz the power, 2015-05-06 13:33:22]

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

[commit-bot: I haz the power, 2015-05-06 13:33:24]

Dry run: This issue passed the CQ dry run.

[mtklein, 2015-05-06 13:54:49]

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

[commit-bot: I haz the power, 2015-05-06 13:55:03]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1056673002/180001

[commit-bot: I haz the power, 2015-05-06 14:00:52]

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

[commit-bot: I haz the power, 2015-05-06 14:00:53]

Dry run: This issue passed the CQ dry run.

[mtklein, 2015-05-06 14:04:10]

Righto Ben, PTAL if you can.  I've given up fighting MSVC and fallen back to a
simpler implementation that it likes.

[bungeman-skia, 2015-05-06 14:16:31]

https://codereview.chromium.org/1056673002/diff/180001/src/core/SkFunction.h
File src/core/SkFunction.h (right):

https://codereview.chromium.org/1056673002/diff/180001/src/core/SkFunction.h#...
src/core/SkFunction.h:24: SkFunction(const Fn& fn) :
fFunction(SkNEW_ARGS(LambdaImpl<Fn>, (fn))) {}
It's been a while now, but not having the small function optimization makes
these rather less useful from a performance standpoint doesn't it? In some cases
it doesn't matter, but in some places it seemed like it did.

https://codereview.chromium.org/1056673002/diff/180001/src/core/SkFunction.h#...
src/core/SkFunction.h:29: SkFunction& operator=(const SkFunction& other) {
Check for self assignment?

[bungeman-skia, 2015-05-06 14:18:26]

https://codereview.chromium.org/1056673002/diff/180001/src/core/SkFunction.h
File src/core/SkFunction.h (right):

https://codereview.chromium.org/1056673002/diff/180001/src/core/SkFunction.h#...
src/core/SkFunction.h:30: if (other.fFunction) {
If the other guy doesn't have a function, shouldn't this not have a function as
well?

[mtklein, 2015-05-06 14:22:10]

https://codereview.chromium.org/1056673002/diff/180001/src/core/SkFunction.h
File src/core/SkFunction.h (right):

https://codereview.chromium.org/1056673002/diff/180001/src/core/SkFunction.h#...
src/core/SkFunction.h:24: SkFunction(const Fn& fn) :
fFunction(SkNEW_ARGS(LambdaImpl<Fn>, (fn))) {}
On 2015/05/06 14:16:31, bungeman1 wrote:
> It's been a while now, but not having the small function optimization makes
> these rather less useful from a performance standpoint doesn't it? In some
cases
> it doesn't matter, but in some places it seemed like it did.

Yes, for sure.  But, not being able to copy SkFunctions makes them even less
useful.

https://codereview.chromium.org/1056673002/diff/180001/src/core/SkFunction.h#...
src/core/SkFunction.h:29: SkFunction& operator=(const SkFunction& other) {
On 2015/05/06 14:16:31, bungeman1 wrote:
> Check for self assignment?

Done.

https://codereview.chromium.org/1056673002/diff/180001/src/core/SkFunction.h#...
src/core/SkFunction.h:30: if (other.fFunction) {
On 2015/05/06 14:18:26, bungeman1 wrote:
> If the other guy doesn't have a function, shouldn't this not have a function
as
> well?

Ooh, yes, done.  As you might imagine, that's a tricky situation to unit test.

[bungeman-skia, 2015-05-06 14:31:32]

Maybe with vc++14 something sane will be possible for the small function
optimization.

lgtm

[mtklein, 2015-05-06 14:34:31]

On 2015/05/06 14:31:32, bungeman1 wrote:
> Maybe with vc++14 something sane will be possible for the small function
> optimization.
> 
> lgtm

Yeah, I'm still hopeful I can think of something, but I don't want to gate using
this on that, at least in non-perf-critical spots.  I was just looking to pilot
it in a few spost in DM to start.

[mtklein, 2015-05-06 14:34:39]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2015-05-06 14:34:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1056673002/200001

[commit-bot: I haz the power, 2015-05-06 14:40:29]

Committed patchset #10 (id:200001) as
https://skia.googlesource.com/skia/+/0d992db1cf5003340409ed02993266004cc0a57c