Add SkTypeface* parameter to SkScalerContext (and its callers)

trunk/src/core/SkScalerContext.cpp

Index: trunk/src/core/SkScalerContext.cpp
===================================================================
--- trunk/src/core/SkScalerContext.cpp	(revision 8166)
+++ trunk/src/core/SkScalerContext.cpp	(working copy)
@@ -75,11 +75,11 @@
     return obj;
 }
 
-SkScalerContext::SkScalerContext(const SkDescriptor* desc)
+SkScalerContext::SkScalerContext(SkTypeface* typeface, const SkDescriptor* desc)
     : fRec(*static_cast<const Rec*>(desc->findEntry(kRec_SkDescriptorTag, NULL)))
 
     , fBaseGlyphCount(0)
-
+    , fTypeface(SkRef(typeface))

[bungeman-skia, 2013/03/15 16:12:44]

fTypeface is a pointer, not an SkAutoTUnref, so the ref should be done in the
body. I'd actually prefer fTypeface to be an SkAutoTUnref<SkTypeface> and leave
this line alone.

[reed1, 2013/03/15 17:46:44]

Not sure I agree that pointers should not be initialized this way, but I have
changed fTypeface to be an auto-guy

     , fPathEffect(static_cast<SkPathEffect*>(load_flattenable(desc, kPathEffect_SkDescriptorTag)))
     , fMaskFilter(static_cast<SkMaskFilter*>(load_flattenable(desc, kMaskFilter_SkDescriptorTag)))
     , fRasterizer(static_cast<SkRasterizer*>(load_flattenable(desc, kRasterizer_SkDescriptorTag)))
@@ -115,16 +115,20 @@
     SkSafeUnref(fPathEffect);
     SkSafeUnref(fMaskFilter);
     SkSafeUnref(fRasterizer);
+    fTypeface->unref();

[bungeman-skia, 2013/03/15 16:12:44]

Remove if fTypeface is changed to an SkAutoTUnref.

[reed1, 2013/03/15 17:46:44]

Done.

 }
 
 static SkScalerContext* allocNextContext(const SkScalerContext::Rec& rec) {
     // fonthost will determine the next possible font to search, based
     // on the current font in fRec. It will return NULL if ctx is our
     // last font that can be searched (i.e. ultimate fallback font)
-    uint32_t newFontID = SkFontHost::NextLogicalFont(rec.fFontID, rec.fOrigFontID);
-    if (0 == newFontID) {
+    SkTypeface* newFace = SkFontHost::NextLogicalTypeface(rec.fFontID, rec.fOrigFontID);

[bungeman-skia, 2013/03/15 16:12:44]

NextLogicalTypeface returns a ref?

[reed1, 2013/03/15 17:46:44]

rewrote dox

+    if (0 == newFace) {
         return NULL;
     }
+    
+    SkAutoTUnref<SkTypeface> aur(newFace);
+    uint32_t newFontID = newFace->uniqueID();
 
     SkAutoDescriptor    ad(sizeof(rec) + SkDescriptor::ComputeOverhead(1));
     SkDescriptor*       desc = ad.getDesc();
@@ -136,7 +140,7 @@
     newRec->fFontID = newFontID;
     desc->computeChecksum();
 
-    return SkFontHost::CreateScalerContext(desc);
+    return newFace->createScalerContext(desc);
 }
 
 /*  Return the next context, creating it if its not already created, but return
@@ -740,7 +744,8 @@
 
 class SkScalerContext_Empty : public SkScalerContext {
 public:
-    SkScalerContext_Empty(const SkDescriptor* desc) : SkScalerContext(desc) {}
+    SkScalerContext_Empty(SkTypeface* face, const SkDescriptor* desc)
+        : SkScalerContext(face, desc) {}
 
 protected:
     virtual unsigned generateGlyphCount() SK_OVERRIDE {
@@ -770,13 +775,13 @@
 
 extern SkScalerContext* SkCreateColorScalerContext(const SkDescriptor* desc);
 
-SkScalerContext* SkScalerContext::Create(const SkDescriptor* desc) {
+SkScalerContext* SkTypeface::createScalerContext(const SkDescriptor* desc) {
     SkScalerContext* c = NULL;  //SkCreateColorScalerContext(desc);
     if (NULL == c) {
-        c = SkFontHost::CreateScalerContext(desc);
+        c = this->onCreateScalerContext(desc);
     }
     if (NULL == c) {
-        c = SkNEW_ARGS(SkScalerContext_Empty, (desc));
+        c = SkNEW_ARGS(SkScalerContext_Empty, (this, desc));

[bungeman-skia, 2013/03/15 16:12:44]

Since all Empty do (almost) the same thing, should we have just one? It seems it
would be nice to avoid having to new something up when that's probably the only
reason the above call would have returned NULL. (I suppose in the case of
FreeType the font data could just be bad and creating the face fail.)

[reed1, 2013/03/15 17:46:44]

1. This case is incredibly rare
2. SkScalerContext is not reference counted

     }
     return c;
 }