SerenityOS / serenity

The Serenity Operating System 🐞
https://serenityos.org
BSD 2-Clause "Simplified" License
29.63k stars 3.15k forks source link

LibGfx: Can't load the font "Hæck" #23654

Open PaddiM8 opened 3 months ago

PaddiM8 commented 3 months ago

I noticed that Ladybird crashes when I have Hæck installed. It's a variant of the Hack font with ligatures.

Here's a gdb backtrace:

ak_verification_failed () at /home/paddi/clones/serenity/AK/Assertions.cpp:108
108     __builtin_trap();
(gdb) bt
#0  ak_verification_failed () at /home/paddi/clones/serenity/AK/Assertions.cpp:108
#1  0x00007f6f9aeeb28c in AK::ErrorOr<AK::String, AK::Error>::release_value_but_fixme_should_propagate_errors ()
    at /home/paddi/clones/serenity/Meta/Lagom/../../AK/Error.h:202
#2  0x00007f6f9aee3d4d in OpenType::Name::string_for_id ()
    at /home/paddi/clones/serenity/Userland/Libraries/LibGfx/Font/OpenType/Tables.cpp:238
#3  0x00007f6f9aecc029 in OpenType::Name::family_name ()
    at /home/paddi/clones/serenity/Meta/Lagom/../../Userland/Libraries/LibGfx/Font/OpenType/Tables.h:341
#4  0x00007f6f9aec9425 in operator() ()
    at /home/paddi/clones/serenity/Userland/Libraries/LibGfx/Font/OpenType/Font.cpp:586
#5  0x00007f6f9aec9509 in OpenType::Font::family ()
    at /home/paddi/clones/serenity/Userland/Libraries/LibGfx/Font/OpenType/Font.cpp:58---Type <RET> for more, q to quit, c to continue without paging--

#6  0x00007f6f9aeb1f0f in operator() ()
    at /home/paddi/clones/serenity/Userland/Libraries/LibGfx/Font/FontDatabase.cpp:133
#7  0x00007f6f9aeb47c8 in operator() ()
    at /home/paddi/clones/serenity/Meta/Lagom/../../Userland/Libraries/LibCore/Resource.h:89
#8  0x00007f6f9aeb7989 in for_each_descendant<Core::Resource::for_each_descendant_file<Gfx::FontDatabase::load_all_fonts_from_uri(AK::StringView)::<lambda(const Core::Resource&)> >(Gfx::FontDatabase::load_all_fonts_from_uri(AK::StringView)::<lambda(const Core::Resource&)>&&) const::<lambda(const Core::Resource&)>&>(void) ()
    at /home/paddi/clones/serenity/Meta/Lagom/../../Userland/Libraries/LibCore/Resource.h:74
#9  0x00007f6f9aeb4ba8 in for_each_descendant<Core::Resource::for_each_descendant_file<Gfx::FontDatabase::load_all_fonts_from_uri(AK::StringView)::<lambda(const Core::Resource&)> >(Gfx::FontDatabase::load_all_fonts_from_uri(AK::StringView)::<lambda(const Core::Resource&)>&&) const::<lambda(const Core::Resource&)> >(void) ()
    at /home/paddi/clones/serenity/Meta/Lagom/../../Userland/Libraries/LibCore/Resource.h:76
#10 0x00007f6f9aeb4811 in for_each_descendant_file<Gfx::FontDatabase::load_all_fonts_from_uri(AK::StringView)::<lambda(const Core::Resource&)> >(void) ()
    at /home/paddi/clones/serenity/Meta/Lagom/../../Userland/Libraries/LibCore/Resource.h:86
#11 0x00007f6f9aeb26a0 in Gfx::FontDatabase::load_all_fonts_from_uri ()
    at /home/paddi/clones/serenity/Userland/Libraries/LibGfx/Font/FontDatabase.cpp:120
#12 0x0000564c399410af in Ladybird::FontPlugin::FontPlugin ()
    at /home/paddi/clones/serenity/Ladybird/FontPlugin.cpp:24
#13 0x0000564c3996d4d1 in serenity_main ()
    at /home/paddi/clones/serenity/Ladybird/WebContent/main.cpp:121

I used the font from here: https://github.com/ignatov/Haack/releases/download/0.1/Haeck-Regular.ttf

PaddiM8 commented 3 months ago

Wait, it seems to be related to the name? It tries to decode 48be636b. Afaik, be is not how æ is represented in UTF-8, and my hex editor just shows it as H.ck, but other programs do display the font name as Hæck and loads it without issues. I can confirm that 48be636b is present in the font file itself.

nico commented 3 months ago

This might help; haven't tested it though:

% git diff
diff --git a/Userland/Libraries/LibGfx/Font/OpenType/Tables.cpp b/Userland/Libraries/LibGfx/Font/OpenType/Tables.cpp
index 3dc111295d..030cd72011 100644
--- a/Userland/Libraries/LibGfx/Font/OpenType/Tables.cpp
+++ b/Userland/Libraries/LibGfx/Font/OpenType/Tables.cpp
@@ -235,7 +235,8 @@ String Name::string_for_id(NameId id) const
         return decoder.to_utf8(m_string_data.slice(offset, length)).release_value_but_fixme_should_propagate_errors();
     }

-    return String::from_utf8(m_string_data.slice(offset, length)).release_value_but_fixme_should_propagate_errors();
+    static auto& decoder = *TextCodec::decoder_for("x-mac-roman"sv);
+    return decoder.to_utf8(m_string_data.slice(offset, length)).release_value_but_fixme_should_propagate_errors();
 }

 ErrorOr<Kern> Kern::from_slice(ReadonlyBytes slice)

For bonus points, we'd also check the encoding_id to be 0, in addition to checking language_id. (ref: https://learn.microsoft.com/en-us/typography/opentype/spec/name#macintosh-encoding-ids-script-manager-codes)

PaddiM8 commented 3 months ago

That seems to fix it in indeed!