Skip to content

Commit e42d79b

Browse files
committed
Fix memory leak in SetFont/ResolveFontDescription
Also about 55% faster. Fixes #1202
1 parent af13f8f commit e42d79b

File tree

3 files changed

+45
-53
lines changed

3 files changed

+45
-53
lines changed

CHANGELOG.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ project adheres to [Semantic Versioning](http://semver.org/).
1313

1414
### Fixed
1515
* Don't crash when font string is invalid (bug since 2.2.0) (#1328)
16-
* Fix memory leak in toBuffer
16+
* Fix memory leak in `canvas.toBuffer()` (#1202, #1296)
17+
* Fix memory leak in `ctx.font=` (#1202)
1718

1819
2.2.0
1920
==================

src/Canvas.cc

Lines changed: 41 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@
66

77
#include <assert.h>
88
#include <stdlib.h>
9-
#include <string.h>
9+
#include <cstring>
10+
#include <cctype>
11+
#include <string>
12+
#include <unordered_set>
1013
#include <node_buffer.h>
1114
#include <node_version.h>
1215
#include <glib.h>
@@ -671,18 +674,13 @@ NAN_METHOD(Canvas::RegisterFont) {
671674
pango_font_description_set_style(user_desc, Canvas::GetStyleFromCSSString(style));
672675
pango_font_description_set_family(user_desc, family);
673676

674-
std::vector<FontFace>::iterator it = _font_face_list.begin();
675-
FontFace *already_registered = NULL;
676-
677-
for (; it != _font_face_list.end() && !already_registered; ++it) {
678-
if (pango_font_description_equal(it->sys_desc, sys_desc)) {
679-
already_registered = &(*it);
680-
}
681-
}
682-
683-
if (already_registered) {
684-
pango_font_description_free(already_registered->user_desc);
685-
already_registered->user_desc = user_desc;
677+
auto found = std::find_if(_font_face_list.begin(), _font_face_list.end(), [&](FontFace& f) {
678+
return pango_font_description_equal(f.sys_desc, sys_desc);
679+
});
680+
681+
if (found != _font_face_list.end()) {
682+
pango_font_description_free(found->user_desc);
683+
found->user_desc = user_desc;
686684
} else if (register_font((unsigned char *) *filePath)) {
687685
FontFace face;
688686
face.user_desc = user_desc;
@@ -720,14 +718,7 @@ Canvas::~Canvas() {
720718
}
721719
}
722720

723-
std::vector<FontFace>
724-
_init_font_face_list() {
725-
std::vector<FontFace> x;
726-
return x;
727-
}
728-
729-
std::vector<FontFace> Canvas::_font_face_list = _init_font_face_list();
730-
721+
std::vector<FontFace> Canvas::_font_face_list;
731722
/*
732723
* Get a PangoStyle from a CSS string (like "italic")
733724
*/
@@ -782,57 +773,57 @@ Canvas::GetWeightFromCSSString(const char *weight) {
782773
return w;
783774
}
784775

776+
bool streq_casein(std::string& str1, std::string& str2) {
777+
return str1.size() == str2.size() && std::equal(str1.begin(), str1.end(), str2.begin(), [](char& c1, char& c2) {
778+
return c1 == c2 || std::toupper(c1) == std::toupper(c2);
779+
});
780+
}
781+
785782
/*
786783
* Given a user description, return a description that will select the
787784
* font either from the system or @font-face
788785
*/
789786

790787
PangoFontDescription *
791788
Canvas::ResolveFontDescription(const PangoFontDescription *desc) {
792-
FontFace best;
793-
PangoFontDescription *ret = NULL;
794-
795789
// One of the user-specified families could map to multiple SFNT family names
796790
// if someone registered two different fonts under the same family name.
797791
// https://drafts.csswg.org/css-fonts-3/#font-style-matching
798-
char **families = g_strsplit(pango_font_description_get_family(desc), ",", -1);
799-
GHashTable *seen_families = g_hash_table_new(g_str_hash, g_str_equal);
800-
GString *resolved_families = g_string_new("");
801-
802-
for (int i = 0; families[i]; ++i) {
803-
GString *renamed_families = g_string_new("");
804-
std::vector<FontFace>::iterator it = _font_face_list.begin();
805-
806-
for (; it != _font_face_list.end(); ++it) {
807-
if (g_ascii_strcasecmp(families[i], pango_font_description_get_family(it->user_desc)) == 0) {
808-
char *name = g_strdup(pango_font_description_get_family(it->sys_desc));
809-
bool unseen = g_hash_table_lookup(seen_families, name) == NULL;
792+
FontFace best;
793+
istringstream families(pango_font_description_get_family(desc));
794+
unordered_set<string> seen_families;
795+
string resolved_families;
796+
bool first = true;
797+
798+
for (string family; getline(families, family, ','); ) {
799+
string renamed_families;
800+
for (auto& ff : _font_face_list) {
801+
string pangofamily = string(pango_font_description_get_family(ff.user_desc));
802+
if (streq_casein(family, pangofamily)) {
803+
const char* sys_desc_family_name = pango_font_description_get_family(ff.sys_desc);
804+
bool unseen = seen_families.find(sys_desc_family_name) == seen_families.end();
810805

811806
// Avoid sending duplicate SFNT font names due to a bug in Pango for macOS:
812807
// https://bugzilla.gnome.org/show_bug.cgi?id=762873
813808
if (unseen) {
814-
g_hash_table_replace(seen_families, name, name);
815-
if (renamed_families->len) g_string_append(renamed_families, ",");
816-
g_string_append(renamed_families, name);
809+
seen_families.insert(sys_desc_family_name);
810+
if (renamed_families.size()) renamed_families += ',';
811+
renamed_families += sys_desc_family_name;
817812
}
818813

819-
if (i == 0 && (best.user_desc == NULL || pango_font_description_better_match(desc, best.user_desc, it->user_desc))) {
820-
best = *it;
814+
if (first && (best.user_desc == nullptr || pango_font_description_better_match(desc, best.user_desc, ff.user_desc))) {
815+
best = ff;
821816
}
822817
}
823818
}
824819

825-
if (resolved_families->len) g_string_append(resolved_families, ",");
826-
g_string_append(resolved_families, renamed_families->len ? renamed_families->str : families[i]);
827-
g_string_free(renamed_families, true);
820+
if (resolved_families.size()) resolved_families += ',';
821+
resolved_families += renamed_families.size() ? renamed_families : family;
822+
first = false;
828823
}
829824

830-
ret = pango_font_description_copy(best.sys_desc ? best.sys_desc : desc);
831-
pango_font_description_set_family_static(ret, resolved_families->str);
832-
833-
g_strfreev(families);
834-
g_string_free(resolved_families, false);
835-
g_hash_table_destroy(seen_families);
825+
PangoFontDescription* ret = pango_font_description_copy(best.sys_desc ? best.sys_desc : desc);
826+
pango_font_description_set_family_static(ret, resolved_families.c_str());
836827

837828
return ret;
838829
}

src/Canvas.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ using namespace v8;
3939
*/
4040
class FontFace {
4141
public:
42-
PangoFontDescription *sys_desc = NULL;
43-
PangoFontDescription *user_desc = NULL;
42+
PangoFontDescription *sys_desc = nullptr;
43+
PangoFontDescription *user_desc = nullptr;
4444
};
4545

4646
/*

0 commit comments

Comments
 (0)