Add factory class for generating various flavors of GrTextContext.

Add factory class for generating various flavors of GrTextContext.

This is the first pass of making Gr*TextContext more generic and easily
subclassed. The next stage will be making GrBitmapTextContext and
GrDistanceFieldTextContext more similar by moving the SkDraw loop into
each subclass.

BUG=skia:2018
Committed: http://code.google.com/p/skia/source/detail?r=13142

Committed: http://code.google.com/p/skia/source/detail?r=13165

[bsalomon, 2014-01-21 20:46:17]

https://codereview.chromium.org/144283002/diff/1/include/gpu/GrTextContext.h
File include/gpu/GrTextContext.h (right):

https://codereview.chromium.org/144283002/diff/1/include/gpu/GrTextContext.h#...
include/gpu/GrTextContext.h:50: * This class wraps the creation of a single text
context
Maybe a little more clear in the comment that it isn't a factory for a singleton
text context but that it assumes there is at most one undestroyed text context
at any given time.

https://codereview.chromium.org/144283002/diff/1/include/gpu/GrTextContext.h#...
include/gpu/GrTextContext.h:56: fAllocation = malloc(sizeof(TextContextClass));
SkMalloc/SkFree

https://codereview.chromium.org/144283002/diff/1/src/gpu/SkGpuDevice.cpp
File src/gpu/SkGpuDevice.cpp (right):

https://codereview.chromium.org/144283002/diff/1/src/gpu/SkGpuDevice.cpp#newc...
src/gpu/SkGpuDevice.cpp:1884: (GrDistanceFieldTextContext*)
fTextContextFactory->Create(fContext, grPaint, paint);
Does phase 2 remove the need for this downcast?

[jvanverth1, 2014-01-21 21:10:56]

https://codereview.chromium.org/144283002/diff/1/include/gpu/GrTextContext.h
File include/gpu/GrTextContext.h (right):

https://codereview.chromium.org/144283002/diff/1/include/gpu/GrTextContext.h#...
include/gpu/GrTextContext.h:50: * This class wraps the creation of a single text
context
On 2014/01/21 20:46:17, bsalomon wrote:
> Maybe a little more clear in the comment that it isn't a factory for a
singleton
> text context but that it assumes there is at most one undestroyed text context
> at any given time.

Done.

https://codereview.chromium.org/144283002/diff/1/include/gpu/GrTextContext.h#...
include/gpu/GrTextContext.h:56: fAllocation = malloc(sizeof(TextContextClass));
On 2014/01/21 20:46:17, bsalomon wrote:
> SkMalloc/SkFree

Done.

https://codereview.chromium.org/144283002/diff/1/src/gpu/SkGpuDevice.cpp
File src/gpu/SkGpuDevice.cpp (right):

https://codereview.chromium.org/144283002/diff/1/src/gpu/SkGpuDevice.cpp#newc...
src/gpu/SkGpuDevice.cpp:1884: (GrDistanceFieldTextContext*)
fTextContextFactory->Create(fContext, grPaint, paint);
On 2014/01/21 20:46:17, bsalomon wrote:
> Does phase 2 remove the need for this downcast?

Yes. The current shared interface is for working with SkDraw. The new one will
be closer to GrDistanceFieldContext, so this cast won't be necessary.

[tfarina, 2014-01-21 23:49:45]

https://codereview.chromium.org/144283002/diff/1/include/gpu/GrTextContext.h
File include/gpu/GrTextContext.h (right):

https://codereview.chromium.org/144283002/diff/1/include/gpu/GrTextContext.h#...
include/gpu/GrTextContext.h:38: private:
while here, remove this empty section?

[jvanverth1, 2014-01-22 18:24:33]

I've revised how GrTextContexts are destroyed; now operator delete() has been
overridden for GrTextContext so that they get recycled properly, and they can be
encapsulated in a SkAutoTDelete. PTAL.

[bsalomon, 2014-01-22 19:13:36]

https://codereview.chromium.org/144283002/diff/100001/include/gpu/GrTextConte...
File include/gpu/GrTextContext.h (right):

https://codereview.chromium.org/144283002/diff/100001/include/gpu/GrTextConte...
include/gpu/GrTextContext.h:45: GrTextContext* Create(GrContext* context, const
GrPaint& grPaint,
nit: since Create and Destroy are not static they should have initial lower
case.

https://codereview.chromium.org/144283002/diff/100001/include/gpu/GrTextConte...
include/gpu/GrTextContext.h:76: static void operator delete(void* ptr) throw() {
Do we need the throw here? We build without exceptions.

What do you think of encapsulating the delete overload within the factory?
Something like:

template <class TextContextClass>
class GrTTextContextFactory : public GrTextContextFactory {

    class ManagedTextContext : public TextContextClass {
    public:
        static void operator delete(void* ptr) throw() {
           // ...        
        }
    private:
        GrTTextContextFactory* fFactory; 
    }
    // ...
};

Aside from encapsulating the memory management in the factory, I think this
would mean that the destroy function need not be virtual since the sub-subclass
has a ptr to GrTTextContextFactory<Foo>.

https://codereview.chromium.org/144283002/diff/100001/src/gpu/SkGpuDevice.cpp
File src/gpu/SkGpuDevice.cpp (right):

https://codereview.chromium.org/144283002/diff/100001/src/gpu/SkGpuDevice.cpp...
src/gpu/SkGpuDevice.cpp:204: fTextContextFactory = new
GrTTextContextFactory<GrDistanceFieldTextContext>();
SkNEW()

https://codereview.chromium.org/144283002/diff/100001/src/gpu/SkGpuDevice.cpp...
src/gpu/SkGpuDevice.cpp:245: fTextContextFactory = new
GrTTextContextFactory<GrDistanceFieldTextContext>();
SkNEW()

https://codereview.chromium.org/144283002/diff/100001/src/gpu/SkGpuDevice.cpp...
src/gpu/SkGpuDevice.cpp:296: delete fTextContextFactory;
delete doesn't need a null check.

[jvanverth1, 2014-01-22 20:28:40]

https://codereview.chromium.org/144283002/diff/100001/include/gpu/GrTextConte...
File include/gpu/GrTextContext.h (right):

https://codereview.chromium.org/144283002/diff/100001/include/gpu/GrTextConte...
include/gpu/GrTextContext.h:45: GrTextContext* Create(GrContext* context, const
GrPaint& grPaint,
On 2014/01/22 19:13:36, bsalomon wrote:
> nit: since Create and Destroy are not static they should have initial lower
> case.

Done.

https://codereview.chromium.org/144283002/diff/100001/include/gpu/GrTextConte...
include/gpu/GrTextContext.h:76: static void operator delete(void* ptr) throw() {
On 2014/01/22 19:13:36, bsalomon wrote:
> Do we need the throw here? We build without exceptions.
> 
> What do you think of encapsulating the delete overload within the factory?
> Something like:
> 
> template <class TextContextClass>
> class GrTTextContextFactory : public GrTextContextFactory {
> 
>     class ManagedTextContext : public TextContextClass {
>     public:
>         static void operator delete(void* ptr) throw() {
>            // ...        
>         }
>     private:
>         GrTTextContextFactory* fFactory; 
>     }
>     // ...
> };
> 
> Aside from encapsulating the memory management in the factory, I think this
> would mean that the destroy function need not be virtual since the
sub-subclass
> has a ptr to GrTTextContextFactory<Foo>.

Done.

https://codereview.chromium.org/144283002/diff/100001/src/gpu/SkGpuDevice.cpp
File src/gpu/SkGpuDevice.cpp (right):

https://codereview.chromium.org/144283002/diff/100001/src/gpu/SkGpuDevice.cpp...
src/gpu/SkGpuDevice.cpp:204: fTextContextFactory = new
GrTTextContextFactory<GrDistanceFieldTextContext>();
On 2014/01/22 19:13:36, bsalomon wrote:
> SkNEW()

Done.

https://codereview.chromium.org/144283002/diff/100001/src/gpu/SkGpuDevice.cpp...
src/gpu/SkGpuDevice.cpp:245: fTextContextFactory = new
GrTTextContextFactory<GrDistanceFieldTextContext>();
On 2014/01/22 19:13:36, bsalomon wrote:
> SkNEW()

Done.

https://codereview.chromium.org/144283002/diff/100001/src/gpu/SkGpuDevice.cpp...
src/gpu/SkGpuDevice.cpp:296: delete fTextContextFactory;
On 2014/01/22 19:13:36, bsalomon wrote:
> delete doesn't need a null check.

Done.

[jvanverth1, 2014-01-22 20:55:05]

Note: I renamed GrTextContextFactory to GrTextContextManager, since that seemed
more accurate.

[bsalomon, 2014-01-22 21:01:17]

On 2014/01/22 20:55:05, JimVV wrote:
> Note: I renamed GrTextContextFactory to GrTextContextManager, since that
seemed
> more accurate.

lgtm

[commit-bot: I haz the power, 2014-01-22 21:25:13]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/jvanverth@google.com/144283002/310001

[commit-bot: I haz the power, 2014-01-22 21:37:07]

Change committed as 13142

[jvanverth1, 2014-01-22 21:53:29]

A revert of this CL has been created in
https://codereview.chromium.org/145023006/ by jvanverth@google.com.

The reason for reverting is: Breaks Chrome. Need to fix template issue..

[commit-bot: I haz the power, 2014-01-24 13:40:25]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/jvanverth@google.com/144283002/430001

[commit-bot: I haz the power, 2014-01-24 14:38:36]

Change committed as 13165