Ladybird+LibWeb+LibGfx: Add option to force use of fontconfig

This allows us to get identical metrics on macOS and Linux. Without this, Skia will use CoreText on macOS and give us slightly different text metrics. That causes layout tests to be slightly different on different platforms, which is a huge headache. So let's not do that. You can now launch Ladybird and headless-browser with --force-fontconfig to load fonts through fontconfig. Tests run in this mode by default.
gmta · Aug 20, 2024 · dd1b8de · dd1b8de
1 parent 3ed0381
commit dd1b8de
Show file tree

Hide file tree

Showing 10 changed files with 43 additions and 9 deletions.
diff --git a/Ladybird/CMakeLists.txt b/Ladybird/CMakeLists.txt
@@ -162,7 +162,7 @@ endif()
 if (BUILD_TESTING)
     add_test(
         NAME LibWeb
-        COMMAND $<TARGET_FILE:headless-browser> --run-tests ${LADYBIRD_SOURCE_DIR}/Tests/LibWeb --dump-failed-ref-tests
+        COMMAND $<TARGET_FILE:headless-browser> --run-tests ${LADYBIRD_SOURCE_DIR}/Tests/LibWeb --dump-failed-ref-tests --force-fontconfig
     )
     add_test(
         NAME WPT

diff --git a/Ladybird/HelperProcess.cpp b/Ladybird/HelperProcess.cpp
@@ -104,6 +104,8 @@ ErrorOr<NonnullRefPtr<WebView::WebContentClient>> launch_web_content_process(
         arguments.append("--expose-internals-object"sv);
     if (web_content_options.force_cpu_painting == WebView::ForceCPUPainting::Yes)
         arguments.append("--force-cpu-painting"sv);
+    if (web_content_options.force_fontconfig == WebView::ForceFontconfig::Yes)
+        arguments.append("--force-fontconfig"sv);
     if (auto server = mach_server_name(); server.has_value()) {
         arguments.append("--mach-server-name"sv);
         arguments.append(server.value());

diff --git a/Ladybird/WebContent/main.cpp b/Ladybird/WebContent/main.cpp
@@ -15,6 +15,7 @@
 #include <LibCore/Process.h>
 #include <LibCore/Resource.h>
 #include <LibCore/SystemServerTakeover.h>
+#include <LibGfx/Font/FontDatabase.h>
 #include <LibIPC/ConnectionFromClient.h>
 #include <LibJS/Bytecode/Interpreter.h>
 #include <LibMain/Main.h>
@@ -105,6 +106,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
     bool enable_idl_tracing = false;
     bool enable_http_cache = false;
     bool force_cpu_painting = false;
+    bool force_fontconfig = false;
 
     Core::ArgsParser args_parser;
     args_parser.add_option(command_line, "Chrome process command line", "command-line", 0, "command_line");
@@ -122,13 +124,18 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
     args_parser.add_option(enable_idl_tracing, "Enable IDL tracing", "enable-idl-tracing");
     args_parser.add_option(enable_http_cache, "Enable HTTP cache", "enable-http-cache");
     args_parser.add_option(force_cpu_painting, "Force CPU painting", "force-cpu-painting");
+    args_parser.add_option(force_fontconfig, "Force using fontconfig for font loading", "force-fontconfig");
 
     args_parser.parse(arguments);
 
     if (wait_for_debugger) {
         Core::Process::wait_for_debugger_and_break();
     }
 
+    if (force_fontconfig) {
+        Gfx::FontDatabase::the().set_force_fontconfig(true);
+    }
+
     // Layout test mode implies internals object is exposed and the Skia CPU backend is used
     if (is_layout_test_mode) {
         expose_internals_object = true;

diff --git a/Tests/LibWeb/rebaseline-libweb-test b/Tests/LibWeb/rebaseline-libweb-test
@@ -28,4 +28,4 @@ else
 fi
 
 mkdir -p $expected_dir
-$ladybird_headless_binary $mode_flag --layout-test-mode $LADYBIRD_SOURCE_DIR/Tests/LibWeb/$input_dir/$test_name.html > $LADYBIRD_SOURCE_DIR/Tests/LibWeb/$expected_dir/$test_name.txt
+$ladybird_headless_binary $mode_flag --force-fontconfig --layout-test-mode $input_dir/$test_name.html > $expected_dir/$test_name.txt
diff --git a/Userland/Libraries/LibGfx/Font/FontDatabase.cpp b/Userland/Libraries/LibGfx/Font/FontDatabase.cpp
@@ -24,9 +24,20 @@ FontDatabase& FontDatabase::the()
 }
 
 struct FontDatabase::Private {
+    bool force_fontconfig { false };
     HashMap<FlyString, Vector<NonnullRefPtr<Typeface>>, AK::ASCIICaseInsensitiveFlyStringTraits> typeface_by_family;
 };
 
+void FontDatabase::set_force_fontconfig(bool force_fontconfig)
+{
+    m_private->force_fontconfig = force_fontconfig;
+}
+
+bool FontDatabase::should_force_fontconfig() const
+{
+    return m_private->force_fontconfig;
+}
+
 void FontDatabase::load_all_fonts_from_uri(StringView uri)
 {
     auto root_or_error = Core::Resource::load_from_uri(uri);

diff --git a/Userland/Libraries/LibGfx/Font/FontDatabase.h b/Userland/Libraries/LibGfx/Font/FontDatabase.h
@@ -28,6 +28,9 @@ class FontDatabase {
 
     void load_all_fonts_from_uri(StringView);
 
+    void set_force_fontconfig(bool);
+    [[nodiscard]] bool should_force_fontconfig() const;
+
 private:
     FontDatabase();
     ~FontDatabase() = default;

diff --git a/Userland/Libraries/LibGfx/Font/TypefaceSkia.cpp b/Userland/Libraries/LibGfx/Font/TypefaceSkia.cpp
@@ -6,16 +6,16 @@
 
 #define AK_DONT_REPLACE_STD
 
+#include <LibGfx/Font/FontDatabase.h>
 #include <LibGfx/Font/Typeface.h>
 
 #include <core/SkData.h>
 #include <core/SkFontMgr.h>
 #include <core/SkTypeface.h>
+#include <ports/SkFontMgr_fontconfig.h>
 
 #ifdef AK_OS_MACOS
 #    include <ports/SkFontMgr_mac_ct.h>
-#else
-#    include <ports/SkFontMgr_fontconfig.h>
 #endif
 
 namespace Gfx {
@@ -26,10 +26,11 @@ RefPtr<SkTypeface> const& Typeface::skia_typeface() const
 {
     if (!s_font_manager) {
 #ifdef AK_OS_MACOS
-        s_font_manager = SkFontMgr_New_CoreText(nullptr);
-#else
-        s_font_manager = SkFontMgr_New_FontConfig(nullptr);
+        if (!Gfx::FontDatabase::the().should_force_fontconfig())
+            s_font_manager = SkFontMgr_New_CoreText(nullptr);
 #endif
+        if (!s_font_manager)
+            s_font_manager = SkFontMgr_New_FontConfig(nullptr);
     }
 
     if (!m_skia_typeface) {

diff --git a/Userland/Libraries/LibWebView/Application.cpp b/Userland/Libraries/LibWebView/Application.cpp
@@ -46,6 +46,7 @@ void Application::initialize(Main::Arguments const& arguments, URL::URL new_tab_
     bool enable_http_cache = false;
     bool expose_internals_object = false;
     bool force_cpu_painting = false;
+    bool force_fontconfig = false;
 
     Core::ArgsParser args_parser;
     args_parser.set_general_help("The Ladybird web browser :^)");
@@ -63,6 +64,7 @@ void Application::initialize(Main::Arguments const& arguments, URL::URL new_tab_
     args_parser.add_option(enable_http_cache, "Enable HTTP cache", "enable-http-cache");
     args_parser.add_option(expose_internals_object, "Expose internals object", "expose-internals-object");
     args_parser.add_option(force_cpu_painting, "Force CPU painting", "force-cpu-painting");
+    args_parser.add_option(force_fontconfig, "Force using fontconfig for font loading", "force-fontconfig");
 
     create_platform_arguments(args_parser);
     args_parser.parse(arguments);
@@ -99,6 +101,7 @@ void Application::initialize(Main::Arguments const& arguments, URL::URL new_tab_
         .enable_http_cache = enable_http_cache ? EnableHTTPCache::Yes : EnableHTTPCache::No,
         .expose_internals_object = expose_internals_object ? ExposeInternalsObject::Yes : ExposeInternalsObject::No,
         .force_cpu_painting = force_cpu_painting ? ForceCPUPainting::Yes : ForceCPUPainting::No,
+        .force_fontconfig = force_fontconfig ? ForceFontconfig::Yes : ForceFontconfig::No,
     };
 
     create_platform_options(m_chrome_options, m_web_content_options);

diff --git a/Userland/Libraries/LibWebView/Options.h b/Userland/Libraries/LibWebView/Options.h
@@ -84,6 +84,11 @@ enum class ForceCPUPainting {
     Yes,
 };
 
+enum class ForceFontconfig {
+    No,
+    Yes,
+};
+
 struct WebContentOptions {
     String command_line;
     String executable_path;
@@ -95,6 +100,7 @@ struct WebContentOptions {
     EnableHTTPCache enable_http_cache { EnableHTTPCache::No };
     ExposeInternalsObject expose_internals_object { ExposeInternalsObject::No };
     ForceCPUPainting force_cpu_painting { ForceCPUPainting::No };
+    ForceFontconfig force_fontconfig { ForceFontconfig::No };
 };
 
 }
diff --git a/vcpkg.json b/vcpkg.json
@@ -3,7 +3,7 @@
   "dependencies": [
     {
       "name": "fontconfig",
-      "platform": "linux | freebsd | openbsd"
+      "platform": "linux | freebsd | openbsd | osx"
     },
     "harfbuzz",
     "icu",
@@ -34,7 +34,8 @@
       "name": "skia",
       "platform": "osx",
       "features": [
-        "metal"
+        "metal",
+        "fontconfig"
       ]
     },
     {