handle double-registering fonts. expand font paths

this should make it much less likely registering a font will fail:

1. paths to fonts with .., sym links, etc are resolved before passed to
the OS and freetype

2. when you register a font with identical metadata to one you have
already registered, its description will be adjusted accordingly (if you
call registerFont with name = 'a' then name = 'b', you will then need
to set ctx.font = 'b' to select it)

this commit also fixes some minor memory leaks

Author: chearon

lib/canvas.js

@@ -75,6 +75,14 @@ exports.JPEGStream = JPEGStream;
 exports.Image = Image;
 exports.ImageData = canvas.ImageData;
 
+/**
+ * Resolve paths for registerFont
+ */
+
+Canvas.registerFont = function(src, fontFace){
+  return Canvas._registerFont(fs.realpathSync(src), fontFace);
+};
+
 /**
  * Context2d implementation.
  */

src/Canvas.cc

@@ -66,7 +66,7 @@ Canvas::Initialize(Nan::ADDON_REGISTER_FUNCTION_ARGS_TYPE target) {
   Nan::SetTemplate(proto, "PNG_ALL_FILTERS", Nan::New<Uint32>(PNG_ALL_FILTERS));
 
   // Class methods
-  Nan::SetMethod(ctor, "registerFont", RegisterFont);
+  Nan::SetMethod(ctor, "_registerFont", RegisterFont);
 
   Nan::Set(target, Nan::New("Canvas").ToLocalChecked(), ctor->GetFunction());
 }
@@ -575,72 +575,75 @@ NAN_METHOD(Canvas::StreamJPEGSync) {
 
 #endif
 
-NAN_METHOD(Canvas::RegisterFont) {
-  FontFace face;
+char *
+str_value(Local<Value> val, const char *fallback, bool can_be_number) {
+  if (val->IsString() || (can_be_number && val->IsNumber())) {
+    return g_strdup(*String::Utf8Value(val));
+  } else if (fallback) {
+    return g_strdup(fallback);
+  } else {
+    return NULL;
+  }
+}
 
+NAN_METHOD(Canvas::RegisterFont) {
   if (!info[0]->IsString()) {
     return Nan::ThrowError("Wrong argument type");
+  } else if (!info[1]->IsObject()) {
+    return Nan::ThrowError(GENERIC_FACE_ERROR);
   }
 
   String::Utf8Value filePath(info[0]);
+  PangoFontDescription *sys_desc = get_pango_font_description((unsigned char *) *filePath);
 
-  if (!register_font((unsigned char *) *filePath, &face.sys_desc)) {
-    Nan::ThrowError("Could not load font to the system's font host");
-  } else {
-    PangoFontDescription *d = pango_font_description_new();
-
-    if (!info[1]->IsObject()) {
-      Nan::ThrowError(GENERIC_FACE_ERROR);
-    } else { // now check the attrs, there are many ways to be wrong
-      Local<Object> desc = info[1]->ToObject();
-      Local<String> family_prop = Nan::New<String>("family").ToLocalChecked();
-      Local<String> weight_prop = Nan::New<String>("weight").ToLocalChecked();
-      Local<String> style_prop = Nan::New<String>("style").ToLocalChecked();
-
-      const char *family;
-      const char *weight = "normal";
-      const char *style = "normal";
-
-      Local<Value> family_val = desc->Get(family_prop);
-      if (family_val->IsString()) {
-        family = strdup(*String::Utf8Value(family_val));
-      } else {
-        Nan::ThrowError(GENERIC_FACE_ERROR);
-        return;
-      }
+  if (!sys_desc) return Nan::ThrowError("Could not parse font file");
 
-      if (desc->HasOwnProperty(weight_prop)) {
-        Local<Value> weight_val = desc->Get(weight_prop);
-        if (weight_val->IsString() || weight_val->IsNumber()) {
-          weight = strdup(*String::Utf8Value(weight_val));
-        } else {
-          Nan::ThrowError(GENERIC_FACE_ERROR);
-          return;
-        }
-      }
+  PangoFontDescription *user_desc = pango_font_description_new();
 
-      if (desc->HasOwnProperty(style_prop)) {
-        Local<Value> style_val = desc->Get(style_prop);
-        if (style_val->IsString()) {
-          style = strdup(*String::Utf8Value(style_val));
-        } else {
-          Nan::ThrowError(GENERIC_FACE_ERROR);
-          return;
-        }
-      }
+  // now check the attrs, there are many ways to be wrong
+  Local<Object> js_user_desc = info[1]->ToObject();
+  Local<String> family_prop = Nan::New<String>("family").ToLocalChecked();
+  Local<String> weight_prop = Nan::New<String>("weight").ToLocalChecked();
+  Local<String> style_prop = Nan::New<String>("style").ToLocalChecked();
+
+  char *family = str_value(js_user_desc->Get(family_prop), NULL, false);
+  char *weight = str_value(js_user_desc->Get(weight_prop), "normal", true);
+  char *style = str_value(js_user_desc->Get(style_prop), "normal", false);
 
-      pango_font_description_set_weight(d, Canvas::GetWeightFromCSSString(weight));
-      pango_font_description_set_style(d, Canvas::GetStyleFromCSSString(style));
-      pango_font_description_set_family(d, family);
+  if (family && weight && style) {
+    pango_font_description_set_weight(user_desc, Canvas::GetWeightFromCSSString(weight));
+    pango_font_description_set_style(user_desc, Canvas::GetStyleFromCSSString(style));
+    pango_font_description_set_family(user_desc, family);
+
+    std::vector<FontFace>::iterator it = _font_face_list.begin();
+    FontFace *already_registered = NULL;
 
-      free((char *)family);
-      if (desc->HasOwnProperty(weight_prop)) free((char *)weight);
-      if (desc->HasOwnProperty(style_prop)) free((char *)style);
+    for (; it != _font_face_list.end() && !already_registered; ++it) {
+      if (pango_font_description_equal(it->sys_desc, sys_desc)) {
+        already_registered = &(*it);
+      }
+    }
 
-      face.user_desc = d;
+    if (already_registered) {
+      pango_font_description_free(already_registered->user_desc);
+      already_registered->user_desc = user_desc;
+    } else if (register_font((unsigned char *) *filePath)) {
+      FontFace face;
+      face.user_desc = user_desc;
+      face.sys_desc = sys_desc;
       _font_face_list.push_back(face);
+    } else {
+      pango_font_description_free(user_desc);
+      Nan::ThrowError("Could not load font to the system's font host");
     }
+  } else {
+    pango_font_description_free(user_desc);
+    Nan::ThrowError(GENERIC_FACE_ERROR);
   }
+
+  g_free(family);
+  g_free(weight);
+  g_free(style);
 }
 
 /*

src/register_font.cc

@@ -191,6 +191,10 @@ get_pango_style(FT_Long flags) {
   }
 }
 
+/*
+ * Return a PangoFontDescription that will resolve to the font file
+ */
+
 PangoFontDescription *
 get_pango_font_description(unsigned char* filepath) {
   FT_Library library;
@@ -199,16 +203,18 @@ get_pango_font_description(unsigned char* filepath) {
 
   if (!FT_Init_FreeType(&library) && !FT_New_Face(library, (const char*)filepath, 0, &face)) {
     TT_OS2 *table = (TT_OS2*)FT_Get_Sfnt_Table(face, FT_SFNT_OS2);
-    char *family = get_family_name(face);
+    if (table) {
+      char *family = get_family_name(face);
 
-    if (family) pango_font_description_set_family_static(desc, family);
-    pango_font_description_set_weight(desc, get_pango_weight(table->usWeightClass));
-    pango_font_description_set_stretch(desc, get_pango_stretch(table->usWidthClass));
-    pango_font_description_set_style(desc, get_pango_style(face->style_flags));
+      if (family) pango_font_description_set_family_static(desc, family);
+      pango_font_description_set_weight(desc, get_pango_weight(table->usWeightClass));
+      pango_font_description_set_stretch(desc, get_pango_stretch(table->usWidthClass));
+      pango_font_description_set_style(desc, get_pango_style(face->style_flags));
 
-    FT_Done_Face(face);
+      FT_Done_Face(face);
 
-    return desc;
+      return desc;
+    }
   }
 
   pango_font_description_free(desc);
@@ -221,7 +227,7 @@ get_pango_font_description(unsigned char* filepath) {
  */
 
 bool
-register_font(unsigned char *filepath, PangoFontDescription **desc) {
+register_font(unsigned char *filepath) {
   bool success;
 
   #ifdef __APPLE__
@@ -235,8 +241,6 @@ register_font(unsigned char *filepath, PangoFontDescription **desc) {
 
   if (!success) return false;
 
-  *desc = get_pango_font_description(filepath);
-
   // Tell Pango to throw away the current FontMap and create a new one. This
   // has the effect of registering the new font in Pango by re-looking up all
   // font families.

src/register_font.h

@@ -1,4 +1,5 @@
 #include <pango/pango.h>
 
-bool register_font(unsigned char *filepath, PangoFontDescription** desc);
+PangoFontDescription *get_pango_font_description(unsigned char *filepath);
+bool register_font(unsigned char *filepath);