Fix SkTypeface::serialize() on Mac by properly indicating local fonts

We used to always set isLocalStream to false in SkTypeface_Mac::onGetFontDescriptor(),
which caused SkTypeface::serialize() to never actually serialize fonts.

BUG=skia:2698
R=reed@google.com, bungeman@google.com

Author: caseq@chromium.org

Review URL: https://codereview.chromium.org/353993003

diff --git a/src/ports/SkFontHost_mac.cpp b/src/ports/SkFontHost_mac.cpp
index e00b87b..77bf4b8 100755 (executable)
--- a/src/ports/SkFontHost_mac.cpp
+++ b/src/ports/SkFontHost_mac.cpp
@@ -426,21 +426,23 @@ static SkTypeface::Style fontstyle2stylebits(const SkFontStyle& fs) {
 class SkTypeface_Mac : public SkTypeface {
 public:
     SkTypeface_Mac(SkTypeface::Style style, SkFontID fontID, bool isFixedPitch,
-                   CTFontRef fontRef, const char name[])
+                   CTFontRef fontRef, const char name[], bool isLocalStream)
         : SkTypeface(style, fontID, isFixedPitch)
         , fName(name)
         , fFontRef(fontRef) // caller has already called CFRetain for us
         , fFontStyle(stylebits2fontstyle(style))
+        , fIsLocalStream(isLocalStream)
     {
         SkASSERT(fontRef);
     }
 
     SkTypeface_Mac(const SkFontStyle& fs, SkFontID fontID, bool isFixedPitch,
-                   CTFontRef fontRef, const char name[])
+                   CTFontRef fontRef, const char name[], bool isLocalStream)
         : SkTypeface(fontstyle2stylebits(fs), fontID, isFixedPitch)
         , fName(name)
         , fFontRef(fontRef) // caller has already called CFRetain for us
         , fFontStyle(fs)
+        , fIsLocalStream(isLocalStream)
     {
         SkASSERT(fontRef);
     }
@@ -469,17 +471,18 @@ protected:
     virtual int onCountGlyphs() const SK_OVERRIDE;
 
 private:
+    bool fIsLocalStream;
 
     typedef SkTypeface INHERITED;
 };
 
-static SkTypeface* NewFromFontRef(CTFontRef fontRef, const char name[]) {
+static SkTypeface* NewFromFontRef(CTFontRef fontRef, const char name[], bool isLocalStream) {
     SkASSERT(fontRef);
     bool isFixedPitch;
     SkTypeface::Style style = computeStyleBits(fontRef, &isFixedPitch);
     SkFontID fontID = CTFontRef_to_SkFontID(fontRef);
 
-    return new SkTypeface_Mac(style, fontID, isFixedPitch, fontRef, name);
+    return new SkTypeface_Mac(style, fontID, isFixedPitch, fontRef, name, isLocalStream);
 }
 
 static SkTypeface* NewFromName(const char familyName[], SkTypeface::Style theStyle) {
@@ -524,7 +527,7 @@ static SkTypeface* NewFromName(const char familyName[], SkTypeface::Style theSty
         }
     }
 
-    return ctFont ? NewFromFontRef(ctFont, familyName) : NULL;
+    return ctFont ? NewFromFontRef(ctFont, familyName, false) : NULL;
 }
 
 static SkTypeface* GetDefaultFace() {
@@ -557,7 +560,7 @@ SkTypeface* SkCreateTypefaceFromCTFont(CTFontRef fontRef) {
     if (face) {
         face->ref();
     } else {
-        face = NewFromFontRef(fontRef, NULL);
+        face = NewFromFontRef(fontRef, NULL, false);
         SkTypefaceCache::Add(face, face->style());
         // NewFromFontRef doesn't retain the parameter, but the typeface it
         // creates does release it in its destructor, so we balance that with
@@ -1438,7 +1441,7 @@ static SkTypeface* create_from_dataProvider(CGDataProviderRef provider) {
         return NULL;
     }
     CTFontRef ct = CTFontCreateWithGraphicsFont(cg, 0, NULL, NULL);
-    return cg ? SkCreateTypefaceFromCTFont(ct) : NULL;
+    return ct ? NewFromFontRef(ct, NULL, true) : NULL;
 }
 
 // Web fonts added to the the CTFont registry do not return their character set.
@@ -1908,8 +1911,7 @@ void SkTypeface_Mac::onGetFontDescriptor(SkFontDescriptor* desc,
     desc->setFamilyName(get_str(CTFontCopyFamilyName(fFontRef), &tmpStr));
     desc->setFullName(get_str(CTFontCopyFullName(fFontRef), &tmpStr));
     desc->setPostscriptName(get_str(CTFontCopyPostScriptName(fFontRef), &tmpStr));
-    // TODO: need to add support for local-streams (here and openStream)
-    *isLocalStream = false;
+    *isLocalStream = fIsLocalStream;
 }
 
 int SkTypeface_Mac::onCharsToGlyphs(const void* chars, Encoding encoding,
@@ -2128,7 +2130,7 @@ static SkTypeface* createFromDesc(CFStringRef cfFamilyName,
     SkFontID fontID = CTFontRef_to_SkFontID(ctFont);
 
     face = SkNEW_ARGS(SkTypeface_Mac, (rec.fFontStyle, fontID, isFixedPitch,
-                                       ctFont, str.c_str()));
+                                       ctFont, str.c_str(), false));
     SkTypefaceCache::Add(face, face->style());
     return face;
 }

diff --git a/tests/FontHostStreamTest.cpp b/tests/FontHostStreamTest.cpp
index 5f959f3..a2254fd 100644 (file)
--- a/tests/FontHostStreamTest.cpp
+++ b/tests/FontHostStreamTest.cpp
@@ -8,6 +8,7 @@
 #include "SkBitmap.h"
 #include "SkCanvas.h"
 #include "SkColor.h"
+#include "SkFontDescriptor.h"
 #include "SkFontHost.h"
 #include "SkGraphics.h"
 #include "SkPaint.h"
@@ -98,6 +99,12 @@ DEF_TEST(FontHostStream, reporter) {
         int ttcIndex;
         SkAutoTUnref<SkStream> fontData(origTypeface->openStream(&ttcIndex));
         SkTypeface* streamTypeface = SkTypeface::CreateFromStream(fontData);
+
+        SkFontDescriptor desc;
+        bool isLocalStream = false;
+        streamTypeface->getFontDescriptor(&desc, &isLocalStream);
+        REPORTER_ASSERT(reporter, isLocalStream);
+
         SkSafeUnref(paint.setTypeface(streamTypeface));
         drawBG(&streamCanvas);
         streamCanvas.drawPosText("A", 1, &point, paint);

diff --git a/tests/SerializationTest.cpp b/tests/SerializationTest.cpp
index 9917119..3a443f3 100644 (file)
--- a/tests/SerializationTest.cpp
+++ b/tests/SerializationTest.cpp
@@ -5,11 +5,14 @@
  * found in the LICENSE file.
  */
 
+#include "Resources.h"
 #include "SkBitmapSource.h"
 #include "SkCanvas.h"
 #include "SkMallocPixelRef.h"
+#include "SkOSFile.h"
 #include "SkPictureRecorder.h"
 #include "SkTemplates.h"
+#include "SkTypeface.h"
 #include "SkWriteBuffer.h"
 #include "SkValidatingReadBuffer.h"
 #include "SkXfermodeImageFilter.h"
@@ -258,6 +261,77 @@ static void TestBitmapSerialization(const SkBitmap& validBitmap,
     }
 }
 
+static SkBitmap draw_picture(SkPicture& picture) {
+     SkBitmap bitmap;
+     bitmap.allocN32Pixels(picture.width(), picture.height());
+     SkCanvas canvas(bitmap);
+     picture.draw(&canvas);
+     return bitmap;
+}
+
+static void compare_bitmaps(skiatest::Reporter* reporter,
+                            const SkBitmap& b1, const SkBitmap& b2) {
+    REPORTER_ASSERT(reporter, b1.width() == b2.width());
+    REPORTER_ASSERT(reporter, b1.height() == b2.height());
+    SkAutoLockPixels autoLockPixels1(b1);
+    SkAutoLockPixels autoLockPixels2(b2);
+
+    if ((b1.width() != b2.width()) ||
+        (b1.height() != b2.height())) {
+        return;
+    }
+
+    int pixelErrors = 0;
+    for (int y = 0; y < b2.height(); ++y) {
+        for (int x = 0; x < b2.width(); ++x) {
+            if (b1.getColor(x, y) != b2.getColor(x, y))
+                ++pixelErrors;
+        }
+    }
+    REPORTER_ASSERT(reporter, 0 == pixelErrors);
+}
+
+static void TestPictureTypefaceSerialization(skiatest::Reporter* reporter) {
+    // Load typeface form file.
+    // This test cannot run if there is no resource path.
+    SkString resourcePath = GetResourcePath();
+    if (resourcePath.isEmpty()) {
+        SkDebugf("Could not run fontstream test because resourcePath not specified.");
+        return;
+    }
+    SkString filename = SkOSPath::SkPathJoin(resourcePath.c_str(), "test.ttc");
+    SkTypeface* typeface = SkTypeface::CreateFromFile(filename.c_str());
+    if (!typeface) {
+        SkDebugf("Could not run fontstream test because test.ttc not found.");
+        return;
+    }
+
+    // Create a paint with the typeface we loaded.
+    SkPaint paint;
+    paint.setColor(SK_ColorGRAY);
+    paint.setTextSize(SkIntToScalar(30));
+    SkSafeUnref(paint.setTypeface(typeface));
+
+    // Paint some text.
+    SkPictureRecorder recorder;
+    SkIRect canvasRect = SkIRect::MakeWH(kBitmapSize, kBitmapSize);
+    SkCanvas* canvas = recorder.beginRecording(canvasRect.width(), canvasRect.height(), NULL, 0);
+    canvas->drawColor(SK_ColorWHITE);
+    canvas->drawText("A", 1, 24, 32, paint);
+    SkAutoTUnref<SkPicture> picture(recorder.endRecording());
+
+    // Serlialize picture and create its clone from stream.
+    SkDynamicMemoryWStream stream;
+    picture->serialize(&stream);
+    SkAutoTUnref<SkStream> inputStream(stream.detachAsStream());
+    SkAutoTUnref<SkPicture> loadedPicture(SkPicture::CreateFromStream(inputStream.get()));
+
+    // Draw both original and clone picture and compare bitmaps -- they should be identical.
+    SkBitmap origBitmap = draw_picture(*picture);
+    SkBitmap destBitmap = draw_picture(*loadedPicture);
+    compare_bitmaps(reporter, origBitmap, destBitmap);
+}
+
 static bool setup_bitmap_for_canvas(SkBitmap* bitmap) {
     SkImageInfo info = SkImageInfo::Make(
         kBitmapSize, kBitmapSize, kN32_SkColorType, kPremul_SkAlphaType);
@@ -321,7 +395,7 @@ DEF_TEST(Serialization, reporter) {
     {
         SkMatrix matrix = SkMatrix::I();
         TestObjectSerialization(&matrix, reporter);
-     }
+    }
 
     // Test path serialization
     {
@@ -421,4 +495,6 @@ DEF_TEST(Serialization, reporter) {
             SkPicture::CreateFromBuffer(reader));
         REPORTER_ASSERT(reporter, NULL != readPict.get());
     }
+
+    TestPictureTypefaceSerialization(reporter);
 }