Avoid copying of font table data, provide raw font bytes

Minikin is changing its approach to table access to use HarfBuzz to
access the tables, based on raw font data, rather than calling the
MinikinFont::GetTable() virtual method. This patch provides raw access
to the font data to make this work.

There's a bit of plumbing to make sure fonts get a pointer to the raw
data as well.

Bug: 27860101
Change-Id: I638e18cf363644bf22fbc9fb9b3358a9e731087f

5 files changed, 113 insertions, 47 deletions

diff --git a/core/jni/android/graphics/FontFamily.cpp b/core/jni/android/graphics/FontFamily.cpp
index 6dc251c87e5e..1c2d13d98acc 100644
--- a/core/jni/android/graphics/FontFamily.cpp
+++ b/core/jni/android/graphics/FontFamily.cpp
@@ -53,36 +53,14 @@ static void FontFamily_unref(JNIEnv* env, jobject clazz, jlong familyPtr) {
     fontFamily->Unref();
 }
 
-static jboolean addSkTypeface(FontFamily* family, SkTypeface* face) {
-    MinikinFont* minikinFont = new MinikinFontSkia(face);
+static jboolean addSkTypeface(FontFamily* family, SkTypeface* face, const void* fontData,
+        size_t fontSize, int ttcIndex) {
+    MinikinFont* minikinFont = new MinikinFontSkia(face, fontData, fontSize, ttcIndex);
     bool result = family->addFont(minikinFont);
     minikinFont->Unref();
     return result;
 }
 
-static jboolean FontFamily_addFont(JNIEnv* env, jobject clazz, jlong familyPtr, jstring path,
-        jint ttcIndex) {
-    NPE_CHECK_RETURN_ZERO(env, path);
-    ScopedUtfChars str(env, path);
-    SkTypeface* face = SkTypeface::CreateFromFile(str.c_str(), ttcIndex);
-    if (face == NULL) {
-        ALOGE("addFont failed to create font %s", str.c_str());
-        return false;
-    }
-    FontFamily* fontFamily = reinterpret_cast<FontFamily*>(familyPtr);
-    return addSkTypeface(fontFamily, face);
-}
-
-static struct {
-    jmethodID mGet;
-    jmethodID mSize;
-} gListClassInfo;
-
-static struct {
-    jfieldID mTag;
-    jfieldID mStyleValue;
-} gAxisClassInfo;
-
 static void release_global_ref(const void* /*data*/, void* context) {
     JNIEnv* env = AndroidRuntime::getJNIEnv();
     bool needToAttach = (env == NULL);
@@ -106,6 +84,47 @@ static void release_global_ref(const void* /*data*/, void* context) {
     }
 }
 
+static jboolean FontFamily_addFont(JNIEnv* env, jobject clazz, jlong familyPtr, jobject bytebuf,
+        jint ttcIndex) {
+    NPE_CHECK_RETURN_ZERO(env, bytebuf);
+    const void* fontPtr = env->GetDirectBufferAddress(bytebuf);
+    if (fontPtr == NULL) {
+        ALOGE("addFont failed to create font, buffer invalid");
+        return false;
+    }
+    jlong fontSize = env->GetDirectBufferCapacity(bytebuf);
+    if (fontSize < 0) {
+        ALOGE("addFont failed to create font, buffer size invalid");
+        return false;
+    }
+    jobject fontRef = MakeGlobalRefOrDie(env, bytebuf);
+    SkAutoTUnref<SkData> data(SkData::NewWithProc(fontPtr, fontSize,
+            release_global_ref, reinterpret_cast<void*>(fontRef)));
+    std::unique_ptr<SkStreamAsset> fontData(new SkMemoryStream(data));
+
+    SkFontMgr::FontParameters params;
+    params.setCollectionIndex(ttcIndex);
+
+    SkAutoTUnref<SkFontMgr> fm(SkFontMgr::RefDefault());
+    SkTypeface* face = fm->createFromStream(fontData.release(), params);
+    if (face == NULL) {
+        ALOGE("addFont failed to create font");
+        return false;
+    }
+    FontFamily* fontFamily = reinterpret_cast<FontFamily*>(familyPtr);
+    return addSkTypeface(fontFamily, face, fontPtr, (size_t)fontSize, ttcIndex);
+}
+
+static struct {
+    jmethodID mGet;
+    jmethodID mSize;
+} gListClassInfo;
+
+static struct {
+    jfieldID mTag;
+    jfieldID mStyleValue;
+} gAxisClassInfo;
+
 static jboolean FontFamily_addFontWeightStyle(JNIEnv* env, jobject clazz, jlong familyPtr,
         jobject font, jint ttcIndex, jobject listOfAxis, jint weight, jboolean isItalic) {
     NPE_CHECK_RETURN_ZERO(env, font);
@@ -133,7 +152,7 @@ static jboolean FontFamily_addFontWeightStyle(JNIEnv* env, jobject clazz, jlong
         }
     }
 
-    void* fontPtr = env->GetDirectBufferAddress(font);
+    const void* fontPtr = env->GetDirectBufferAddress(font);
     if (fontPtr == NULL) {
         ALOGE("addFont failed to create font, buffer invalid");
         return false;
@@ -159,7 +178,7 @@ static jboolean FontFamily_addFontWeightStyle(JNIEnv* env, jobject clazz, jlong
         return false;
     }
     FontFamily* fontFamily = reinterpret_cast<FontFamily*>(familyPtr);
-    MinikinFont* minikinFont = new MinikinFontSkia(face);
+    MinikinFont* minikinFont = new MinikinFontSkia(face, fontPtr, (size_t)fontSize, ttcIndex);
     fontFamily->addFont(minikinFont, FontStyle(weight / 100, isItalic));
     minikinFont->Unref();
     return true;
@@ -191,6 +210,7 @@ static jboolean FontFamily_addFontFromAsset(JNIEnv* env, jobject, jlong familyPt
         return false;
     }
 
+    size_t bufSize = asset->getLength();
     SkAutoTUnref<SkData> data(SkData::NewWithProc(buf, asset->getLength(), releaseAsset, asset));
     SkMemoryStream* stream = new SkMemoryStream(data);
     // CreateFromStream takes ownership of stream.
@@ -200,7 +220,7 @@ static jboolean FontFamily_addFontFromAsset(JNIEnv* env, jobject, jlong familyPt
         return false;
     }
     FontFamily* fontFamily = reinterpret_cast<FontFamily*>(familyPtr);
-    return addSkTypeface(fontFamily, face);
+    return addSkTypeface(fontFamily, face, buf, bufSize, /* ttcIndex */ 0);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -208,7 +228,7 @@ static jboolean FontFamily_addFontFromAsset(JNIEnv* env, jobject, jlong familyPt
 static const JNINativeMethod gFontFamilyMethods[] = {
     { "nCreateFamily",         "(Ljava/lang/String;I)J", (void*)FontFamily_create },
     { "nUnrefFamily",          "(J)V", (void*)FontFamily_unref },
-    { "nAddFont",              "(JLjava/lang/String;I)Z", (void*)FontFamily_addFont },
+    { "nAddFont",              "(JLjava/nio/ByteBuffer;I)Z", (void*)FontFamily_addFont },
     { "nAddFontWeightStyle",   "(JLjava/nio/ByteBuffer;ILjava/util/List;IZ)Z",
             (void*)FontFamily_addFontWeightStyle },
     { "nAddFontFromAsset",     "(JLandroid/content/res/AssetManager;Ljava/lang/String;)Z",
diff --git a/graphics/java/android/graphics/FontFamily.java b/graphics/java/android/graphics/FontFamily.java
index f741e3cb5f8d..e48bf7956df3 100644
--- a/graphics/java/android/graphics/FontFamily.java
+++ b/graphics/java/android/graphics/FontFamily.java
@@ -17,8 +17,12 @@
 package android.graphics;
 
 import android.content.res.AssetManager;
+import android.util.Log;
 
+import java.io.FileInputStream;
+import java.io.IOException;
 import java.nio.ByteBuffer;
+import java.nio.channels.FileChannel;
 import java.util.List;
 
 /**
@@ -27,6 +31,9 @@ import java.util.List;
  * @hide
  */
 public class FontFamily {
+
+    private static String TAG = "FontFamily";
+
     /**
      * @hide
      */
@@ -62,7 +69,15 @@ public class FontFamily {
     }
 
     public boolean addFont(String path, int ttcIndex) {
-        return nAddFont(mNativePtr, path, ttcIndex);
+        try (FileInputStream file = new FileInputStream(path)) {
+            FileChannel fileChannel = file.getChannel();
+            long fontSize = fileChannel.size();
+            ByteBuffer fontBuffer = fileChannel.map(FileChannel.MapMode.READ_ONLY, 0, fontSize);
+            return nAddFont(mNativePtr, fontBuffer, ttcIndex);
+        } catch (IOException e) {
+            Log.e(TAG, "Error mapping font file " + path);
+            return false;
+        }
     }
 
     public boolean addFontWeightStyle(ByteBuffer font, int ttcIndex, List<FontListParser.Axis> axes,
@@ -76,7 +91,7 @@ public class FontFamily {
 
     private static native long nCreateFamily(String lang, int variant);
     private static native void nUnrefFamily(long nativePtr);
-    private static native boolean nAddFont(long nativeFamily, String path, int ttcIndex);
+    private static native boolean nAddFont(long nativeFamily, ByteBuffer font, int ttcIndex);
     private static native boolean nAddFontWeightStyle(long nativeFamily, ByteBuffer font,
             int ttcIndex, List<FontListParser.Axis> listOfAxis,
             int weight, boolean isItalic);
diff --git a/libs/hwui/hwui/MinikinSkia.cpp b/libs/hwui/hwui/MinikinSkia.cpp
index b9e33589885e..b39de266846c 100644
--- a/libs/hwui/hwui/MinikinSkia.cpp
+++ b/libs/hwui/hwui/MinikinSkia.cpp
@@ -22,8 +22,9 @@
 
 namespace android {
 
-MinikinFontSkia::MinikinFontSkia(SkTypeface *typeface) :
-    mTypeface(typeface) {
+MinikinFontSkia::MinikinFontSkia(SkTypeface* typeface, const void* fontData, size_t fontSize,
+        int ttcIndex) :
+    mTypeface(typeface), mFontData(fontData), mFontSize(fontSize), mTtcIndex(ttcIndex) {
 }
 
 MinikinFontSkia::~MinikinFontSkia() {
@@ -66,22 +67,38 @@ void MinikinFontSkia::GetBounds(MinikinRect* bounds, uint32_t glyph_id,
     bounds->mBottom = skBounds.fBottom;
 }
 
-bool MinikinFontSkia::GetTable(uint32_t tag, uint8_t *buf, size_t *size) {
-    if (buf == NULL) {
-        const size_t tableSize = mTypeface->getTableSize(tag);
-        *size = tableSize;
-        return tableSize != 0;
-    } else {
-        const size_t actualSize = mTypeface->getTableData(tag, 0, *size, buf);
-        *size = actualSize;
-        return actualSize != 0;
+const void* MinikinFontSkia::GetTable(uint32_t tag, size_t* size, MinikinDestroyFunc* destroy) {
+    // we don't have a buffer to the font data, copy to own buffer
+    const size_t tableSize = mTypeface->getTableSize(tag);
+    *size = tableSize;
+    if (tableSize == 0) {
+        return nullptr;
     }
+    void* buf = malloc(tableSize);
+    if (buf == nullptr) {
+        return nullptr;
+    }
+    mTypeface->getTableData(tag, 0, tableSize, buf);
+    *destroy = free;
+    return buf;
 }
 
 SkTypeface *MinikinFontSkia::GetSkTypeface() const {
     return mTypeface;
 }
 
+const void* MinikinFontSkia::GetFontData() const {
+    return mFontData;
+}
+
+size_t MinikinFontSkia::GetFontSize() const {
+    return mFontSize;
+}
+
+int MinikinFontSkia::GetFontIndex() const {
+    return mTtcIndex;
+}
+
 int32_t MinikinFontSkia::GetUniqueId() const {
     return mTypeface->uniqueID();
 }
diff --git a/libs/hwui/hwui/MinikinSkia.h b/libs/hwui/hwui/MinikinSkia.h
index 1d50168adac0..dbc65f98fe12 100644
--- a/libs/hwui/hwui/MinikinSkia.h
+++ b/libs/hwui/hwui/MinikinSkia.h
@@ -28,7 +28,8 @@ namespace android {
 class ANDROID_API MinikinFontSkia : public MinikinFont {
 public:
     // Note: this takes ownership of the reference (will unref on dtor)
-    explicit MinikinFontSkia(SkTypeface *typeface);
+    explicit MinikinFontSkia(SkTypeface *typeface, const void* fontData, size_t fontSize,
+        int ttcIndex);
 
     ~MinikinFontSkia();
 
@@ -38,20 +39,30 @@ public:
     void GetBounds(MinikinRect* bounds, uint32_t glyph_id,
         const MinikinPaint &paint) const;
 
-    // If buf is NULL, just update size
-    bool GetTable(uint32_t tag, uint8_t *buf, size_t *size);
+    const void* GetTable(uint32_t tag, size_t* size, MinikinDestroyFunc* destroy);
 
     int32_t GetUniqueId() const;
 
     SkTypeface* GetSkTypeface() const;
 
+    // Access to underlying raw font bytes
+    const void* GetFontData() const;
+    size_t GetFontSize() const;
+    int GetFontIndex() const;
+
     static uint32_t packPaintFlags(const SkPaint* paint);
     static void unpackPaintFlags(SkPaint* paint, uint32_t paintFlags);
 
     // set typeface and fake bold/italic parameters
     static void populateSkPaint(SkPaint* paint, const MinikinFont* font, FontFakery fakery);
 private:
-    SkTypeface *mTypeface;
+    SkTypeface* mTypeface;
+
+    // A raw pointer to the font data - it should be owned by some other object with
+    // lifetime at least as long as this object.
+    const void* mFontData;
+    size_t mFontSize;
+    int mTtcIndex;
 };
 
 }  // namespace android
diff --git a/libs/hwui/hwui/Typeface.cpp b/libs/hwui/hwui/Typeface.cpp
index fa8ad5d6ce32..c583988554c7 100644
--- a/libs/hwui/hwui/Typeface.cpp
+++ b/libs/hwui/hwui/Typeface.cpp
@@ -66,7 +66,10 @@ static FontCollection *makeFontCollection() {
         ALOGD("makeFontCollection adding %s", fn);
         SkTypeface *skFace = SkTypeface::CreateFromFile(fn);
         if (skFace != NULL) {
-            MinikinFont *font = new MinikinFontSkia(skFace);
+            // TODO: might be a nice optimization to get access to the underlying font
+            // data, but would require us opening the file ourselves and passing that
+            // to the appropriate Create method of SkTypeface.
+            MinikinFont *font = new MinikinFontSkia(skFace, NULL, 0, 0);
             family->addFont(font);
             font->Unref();
         } else {