Rewrite SkTRegistry to take any trivially-copyable type.

Rewrite SkTRegistry to take any trivially-copyable type.

Obviously these are all currently function pointers of type T(*)(P) for various
T and P.  In bench refactoring, I'm trying to register a function pointer of
type T(*)(), which can't be done as is (passing P=void doesn't work).  This
also lets us register things like primitives, which is conceivable useful.

BUG=
R=reed@google.com, scroggo@google.com

Committed: https://code.google.com/p/skia/source/detail?r=11082

[reed1, 2013-09-04 15:15:23]

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

https://codereview.chromium.org/23453031/diff/1/include/core/SkTRegistry.h#ne...
include/core/SkTRegistry.h:22: explicit SkTRegistry(T data) {
(const T&) so we don't force the caller to make a copy? (trivial I know, but I
had to ask)

https://codereview.chromium.org/23453031/diff/1/include/core/SkTRegistry.h#ne...
include/core/SkTRegistry.h:43: Data data() const { return fData; } // fix
const Data& data() const { ... }   ?

https://codereview.chromium.org/23453031/diff/1/src/images/SkImageDecoder_lib...
File src/images/SkImageDecoder_libpng.cpp (right):

https://codereview.chromium.org/23453031/diff/1/src/images/SkImageDecoder_lib...
src/images/SkImageDecoder_libpng.cpp:1207: static
SkTRegistry<SkImageDecoder::Format(*)(SkStream*)> gFormatReg(get_format_png);
Seems like (perhaps) ImageDecoder should typedef this signature, just to make it
easy for all of its clients to "get it right".

[reed1, 2013-09-04 15:15:43]

adding scroggo for ImageDecoder question

[scroggo, 2013-09-04 15:40:21]

Minor concerns about clarity, but the code looks good.

https://codereview.chromium.org/23453031/diff/1/bench/benchmain.cpp
File bench/benchmain.cpp (right):

https://codereview.chromium.org/23453031/diff/1/bench/benchmain.cpp#newcode88
bench/benchmain.cpp:88: BenchRegistry::Data f = fBench->data();
The name f is no longer redundant, and might be better named factory.

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

https://codereview.chromium.org/23453031/diff/1/include/core/SkTRegistry.h#ne...
include/core/SkTRegistry.h:21: typedef T Data;
Would it make sense to write a partial specialization for returning a factory?
That would have the benefit that you could call a function called factory()
rather than a function called data().

https://codereview.chromium.org/23453031/diff/1/samplecode/SampleTests.cpp
File samplecode/SampleTests.cpp (right):

https://codereview.chromium.org/23453031/diff/1/samplecode/SampleTests.cpp#ne...
samplecode/SampleTests.cpp:38: TestRegistry::Data fact = fReg->data();
Again, fact might be better named factory now.

https://codereview.chromium.org/23453031/diff/1/src/images/SkImageDecoder_lib...
File src/images/SkImageDecoder_libpng.cpp (right):

https://codereview.chromium.org/23453031/diff/1/src/images/SkImageDecoder_lib...
src/images/SkImageDecoder_libpng.cpp:1207: static
SkTRegistry<SkImageDecoder::Format(*)(SkStream*)> gFormatReg(get_format_png);
On 2013/09/04 15:15:23, reed1 wrote:
> Seems like (perhaps) ImageDecoder should typedef this signature, just to make
it
> easy for all of its clients to "get it right".

That sounds reasonable to me.

[mtklein, 2013-09-04 16:36:45]

Take another look?  I added the typedefs.  Now let's see if they compile
everywhere...

https://codereview.chromium.org/23453031/diff/1/bench/benchmain.cpp
File bench/benchmain.cpp (right):

https://codereview.chromium.org/23453031/diff/1/bench/benchmain.cpp#newcode88
bench/benchmain.cpp:88: BenchRegistry::Data f = fBench->data();
On 2013/09/04 15:40:21, scroggo wrote:
> The name f is no longer redundant, and might be better named factory.

Renamed everything back to Factory/factory.

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

https://codereview.chromium.org/23453031/diff/1/include/core/SkTRegistry.h#ne...
include/core/SkTRegistry.h:22: explicit SkTRegistry(T data) {
On 2013/09/04 15:15:23, reed1 wrote:
> (const T&) so we don't force the caller to make a copy? (trivial I know, but I
> had to ask)

As we talked.

https://codereview.chromium.org/23453031/diff/1/include/core/SkTRegistry.h#ne...
include/core/SkTRegistry.h:43: Data data() const { return fData; } // fix
On 2013/09/04 15:15:23, reed1 wrote:
> const Data& data() const { ... }   ?

Done.

https://codereview.chromium.org/23453031/diff/1/samplecode/SampleTests.cpp
File samplecode/SampleTests.cpp (right):

https://codereview.chromium.org/23453031/diff/1/samplecode/SampleTests.cpp#ne...
samplecode/SampleTests.cpp:38: TestRegistry::Data fact = fReg->data();
On 2013/09/04 15:40:21, scroggo wrote:
> Again, fact might be better named factory now.

Done.

[scroggo, 2013-09-04 17:15:38]

lgtm

[reed1, 2013-09-04 17:16:15]

lgtm

[mtklein, 2013-09-04 17:20:38]

Committed patchset #2 manually as r11082 (presubmit successful).