Skip to content

Conversation

@chearon
Copy link
Collaborator

@chearon chearon commented Sep 1, 2017

This makes the font example work on macOS.

People can still run into the Pango bug on Mac if they do something like ctx.font = "10px Arial, Helvetica".

Behdad said that the patch looked good but hasn't merged it yet: https://bugzilla.gnome.org/show_bug.cgi?id=762873

Works around https://bugzilla.gnome.org/show_bug.cgi?id=762873 for when
`ctx.font` is set to something that resolves to more than one registered
font with identical SFNT names
@LinusU
Copy link
Collaborator

LinusU commented Sep 2, 2017

🍻

@LinusU LinusU merged commit aa52bcd into Automattic:master Sep 2, 2017

// Avoid sending duplicate SFNT font names due to a bug in Pango for macOS:
// https://bugzilla.gnome.org/show_bug.cgi?id=762873
if (g_hash_table_add(seen_families, name)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chearon g_hash_table_add was added in 2.32, and the latest Windows GTK bundle (the one suggested in the install instructions) is 2.22.

Apparently it's equivalent to g_hash_table_replace(seen_families, name, name), but in 2.22 g_hash_table_replace returns void. Also 2.22 has no _contains function, so maybe this:

        bool existed = g_hash_table_lookup(seen_families, name) == NULL;
        g_hash_table_replace(seen_families, name, name);
        if (existed) {

That fixes the Windows build, but I don't have a Mac to test if it fixes the font resolution bug.

(2.22 source: https://github.com/GNOME/glib/blob/glib-2-22/glib/ghash.c)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, 2.22 was released in 2009. 2.32 in 2012

I've also thought of maybe switching to c++ libraries, but the GLib stuff plays nicely with Pango.

I'll switch it to g_hash_table_replace in a sec

@LinusU
Copy link
Collaborator

LinusU commented Sep 5, 2017

Published in 2.0.0-alpha.5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants