From dd1b8de671747f0a5efc84b77933e13208136507 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Mon, 19 Aug 2024 20:35:49 +0200 Subject: [PATCH] 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. --- Ladybird/CMakeLists.txt | 2 +- Ladybird/HelperProcess.cpp | 2 ++ Ladybird/WebContent/main.cpp | 7 +++++++ Tests/LibWeb/rebaseline-libweb-test | 2 +- Userland/Libraries/LibGfx/Font/FontDatabase.cpp | 11 +++++++++++ Userland/Libraries/LibGfx/Font/FontDatabase.h | 3 +++ Userland/Libraries/LibGfx/Font/TypefaceSkia.cpp | 11 ++++++----- Userland/Libraries/LibWebView/Application.cpp | 3 +++ Userland/Libraries/LibWebView/Options.h | 6 ++++++ vcpkg.json | 5 +++-- 10 files changed, 43 insertions(+), 9 deletions(-) diff --git a/Ladybird/CMakeLists.txt b/Ladybird/CMakeLists.txt index b1df0217bb1..4181ba53203 100644 --- a/Ladybird/CMakeLists.txt +++ b/Ladybird/CMakeLists.txt @@ -162,7 +162,7 @@ endif() if (BUILD_TESTING) add_test( NAME LibWeb - COMMAND $ --run-tests ${LADYBIRD_SOURCE_DIR}/Tests/LibWeb --dump-failed-ref-tests + COMMAND $ --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 index ce7dded09c6..a17e06bf5de 100644 --- a/Ladybird/HelperProcess.cpp +++ b/Ladybird/HelperProcess.cpp @@ -104,6 +104,8 @@ ErrorOr> 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 index 47a9b5e4f05..3876ac73a97 100644 --- a/Ladybird/WebContent/main.cpp +++ b/Ladybird/WebContent/main.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -105,6 +106,7 @@ ErrorOr 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,6 +124,7 @@ ErrorOr 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); @@ -129,6 +132,10 @@ ErrorOr serenity_main(Main::Arguments arguments) 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 index ba417724317..2b3f1d31302 100755 --- 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 index d3512a4ff97..782ec4e4aa5 100644 --- 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>, 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 index 21a1f089d03..b8157a382de 100644 --- a/Userland/Libraries/LibGfx/Font/FontDatabase.h +++ b/Userland/Libraries/LibGfx/Font/FontDatabase.h @@ -28,6 +28,9 @@ public: 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 index f78d25bef2b..c8d2f20630e 100644 --- a/Userland/Libraries/LibGfx/Font/TypefaceSkia.cpp +++ b/Userland/Libraries/LibGfx/Font/TypefaceSkia.cpp @@ -6,16 +6,16 @@ #define AK_DONT_REPLACE_STD +#include #include #include #include #include +#include #ifdef AK_OS_MACOS # include -#else -# include #endif namespace Gfx { @@ -26,10 +26,11 @@ RefPtr 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 index 4a1efdb00bf..ca20d0ac8ab 100644 --- 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 index 925a0457ee6..ed3456e0471 100644 --- 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 index b9451dd5807..c6d2d8e732d 100644 --- 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" ] }, {