From f54b0e7c226608278c4d78f2c66c1124ec2d9bf0 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Tue, 16 Jan 2024 18:10:36 -0500 Subject: [PATCH] LibPDF: Don't accidentally put horizontal_scaling in places Fonts should have size font_size times total scaling. We tried to get that by computing text_rendering_matrix.x_scale() * font_size, but text_rendering_matrix.x_scale() also includes horizontal_scaling, which shouldn't be part of font size. Same for character_spacing and word_spacing. This is all a big mess that's caused by LibPDF using ScaledFont, which requires scaling to be aprt of the text type. I have an in-progress local branch that moves LibPDF to directly use VectorFont, which will hopefully make this (and other things) nicer. But first, let's get this right, and then make sure we don't regress it when things change :^) --- Userland/Libraries/LibPDF/Fonts/SimpleFont.cpp | 11 ++++++----- Userland/Libraries/LibPDF/Renderer.cpp | 4 ++-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/Userland/Libraries/LibPDF/Fonts/SimpleFont.cpp b/Userland/Libraries/LibPDF/Fonts/SimpleFont.cpp index 060e334922..dcc546882c 100644 --- a/Userland/Libraries/LibPDF/Fonts/SimpleFont.cpp +++ b/Userland/Libraries/LibPDF/Fonts/SimpleFont.cpp @@ -48,13 +48,14 @@ PDFErrorOr SimpleFont::initialize(Document* document, NonnullRefPtr SimpleFont::draw_string(Gfx::Painter& painter, Gfx::FloatPoint glyph_position, ByteString const& string, Renderer const& renderer) { - auto const& text_rendering_matrix = renderer.calculate_text_rendering_matrix(); - auto font_size = text_rendering_matrix.x_scale() * renderer.text_state().font_size; - - auto character_spacing = text_rendering_matrix.x_scale() * renderer.text_state().character_spacing; - auto word_spacing = text_rendering_matrix.x_scale() * renderer.text_state().word_spacing; auto horizontal_scaling = renderer.text_state().horizontal_scaling; + auto const& text_rendering_matrix = renderer.calculate_text_rendering_matrix(); + auto font_size = text_rendering_matrix.x_scale() * renderer.text_state().font_size / horizontal_scaling; + + auto character_spacing = text_rendering_matrix.x_scale() * renderer.text_state().character_spacing / horizontal_scaling; + auto word_spacing = text_rendering_matrix.x_scale() * renderer.text_state().word_spacing / horizontal_scaling; + for (auto char_code : string.bytes()) { // Use the width specified in the font's dictionary if available, // and use the default width for the given font otherwise. diff --git a/Userland/Libraries/LibPDF/Renderer.cpp b/Userland/Libraries/LibPDF/Renderer.cpp index 955b8cc7ac..4fe98cea55 100644 --- a/Userland/Libraries/LibPDF/Renderer.cpp +++ b/Userland/Libraries/LibPDF/Renderer.cpp @@ -487,7 +487,7 @@ RENDERER_HANDLER(text_set_font) text_state().font_size = args[1].to_float(); auto& text_rendering_matrix = calculate_text_rendering_matrix(); - auto font_size = text_rendering_matrix.x_scale() * text_state().font_size; + auto font_size = text_rendering_matrix.x_scale() * text_state().font_size / text_state().horizontal_scaling; auto resources = extra_resources.value_or(m_page.resources); auto fonts_dictionary = MUST(resources->get_dict(m_document, CommonNames::Font)); @@ -545,7 +545,7 @@ RENDERER_HANDLER(text_set_matrix_and_line_matrix) // Settings the text/line matrix retroactively affects fonts if (text_state().font) { auto new_text_rendering_matrix = calculate_text_rendering_matrix(); - text_state().font->set_font_size(text_state().font_size * new_text_rendering_matrix.x_scale()); + text_state().font->set_font_size(text_state().font_size * new_text_rendering_matrix.x_scale() / text_state().horizontal_scaling); } return {};