From b2d3ceaec51cb0dce4ef01cd972340e0abf78fb5 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Fri, 6 Jan 2023 11:55:23 +0100 Subject: [PATCH] LibGfx: Make text painting better at aligning vector fonts vertically This is achieved by simplifying the logic in TextLayout. We get rid of all the various ways that the layout bounding rect can get cropped. Then we make sure to use the right pixel metrics. Finally we use the font's own line gap metrics instead of hard-coding 4. The end result is that text painted with vector fonts now gets pretty reasonable vertical alignment in most cases. --- Userland/Libraries/LibGUI/Label.cpp | 2 +- Userland/Libraries/LibGfx/Painter.cpp | 13 ++----- Userland/Libraries/LibGfx/TextLayout.cpp | 48 ++++++------------------ Userland/Libraries/LibGfx/TextLayout.h | 17 ++++----- 4 files changed, 22 insertions(+), 58 deletions(-) diff --git a/Userland/Libraries/LibGUI/Label.cpp b/Userland/Libraries/LibGUI/Label.cpp index 2a0bdf07ce..a4b88b2040 100644 --- a/Userland/Libraries/LibGUI/Label.cpp +++ b/Userland/Libraries/LibGUI/Label.cpp @@ -117,7 +117,7 @@ void Label::size_to_fit() int Label::text_calculated_preferred_height() const { - return static_cast(ceilf(Gfx::TextLayout(font(), Utf8View { m_text }, text_rect().to_type()).bounding_rect(Gfx::TextWrapping::Wrap, Gfx::Painter::LINE_SPACING).height())); + return static_cast(ceilf(Gfx::TextLayout(font(), Utf8View { m_text }, text_rect().to_type()).bounding_rect(Gfx::TextWrapping::Wrap).height())); } Optional Label::calculated_preferred_size() const diff --git a/Userland/Libraries/LibGfx/Painter.cpp b/Userland/Libraries/LibGfx/Painter.cpp index 81c5395cb2..06fd77fb02 100644 --- a/Userland/Libraries/LibGfx/Painter.cpp +++ b/Userland/Libraries/LibGfx/Painter.cpp @@ -1518,11 +1518,6 @@ void draw_text_line(FloatRect const& a_rect, Utf8View const& text, Font const& f VERIFY_NOT_REACHED(); } - if (is_vertically_centered_text_alignment(alignment)) { - auto distance_from_baseline_to_bottom = (font.pixel_size() - 1) - font.baseline(); - rect.translate_by(0, distance_from_baseline_to_bottom / 2); - } - auto point = rect.location(); auto space_width = font.glyph_width(' ') + font.glyph_spacing(); @@ -1726,19 +1721,17 @@ void Painter::do_draw_text(FloatRect const& rect, Utf8View const& text, Font con TextLayout layout(font, text, rect); - auto line_height = font.pixel_size() + LINE_SPACING; + auto line_height = font.preferred_line_height(); - auto lines = layout.lines(elision, wrapping, LINE_SPACING); - auto bounding_rect = layout.bounding_rect(wrapping, LINE_SPACING); + auto lines = layout.lines(elision, wrapping); + auto bounding_rect = layout.bounding_rect(wrapping); bounding_rect.align_within(rect, alignment); - bounding_rect.intersect(rect); for (size_t i = 0; i < lines.size(); ++i) { auto line = Utf8View { lines[i] }; FloatRect line_rect { bounding_rect.x(), bounding_rect.y() + i * line_height, bounding_rect.width(), line_height }; - line_rect.intersect(rect); TextDirection line_direction = get_text_direction(line); if (text_contains_bidirectional_text(line, line_direction)) { // Slow Path: The line contains mixed BiDi classes diff --git a/Userland/Libraries/LibGfx/TextLayout.cpp b/Userland/Libraries/LibGfx/TextLayout.cpp index 76c9ba4212..3dbf33ef5c 100644 --- a/Userland/Libraries/LibGfx/TextLayout.cpp +++ b/Userland/Libraries/LibGfx/TextLayout.cpp @@ -20,15 +20,15 @@ struct Block { Utf8View characters; }; -FloatRect TextLayout::bounding_rect(TextWrapping wrapping, int line_spacing) const +FloatRect TextLayout::bounding_rect(TextWrapping wrapping) const { - auto lines = wrap_lines(TextElision::None, wrapping, line_spacing, FitWithinRect::No); - if (!lines.size()) { + auto lines = wrap_lines(TextElision::None, wrapping); + if (lines.is_empty()) { return {}; } FloatRect bounding_rect = { - 0, 0, 0, (lines.size() * (m_font.pixel_size() + line_spacing)) - line_spacing + 0, 0, 0, (static_cast(lines.size()) * (m_font_metrics.ascent + m_font_metrics.descent + m_font_metrics.line_gap)) - m_font_metrics.line_gap }; for (auto& line : lines) { @@ -40,7 +40,7 @@ FloatRect TextLayout::bounding_rect(TextWrapping wrapping, int line_spacing) con return bounding_rect; } -Vector TextLayout::wrap_lines(TextElision elision, TextWrapping wrapping, int line_spacing, FitWithinRect fit_within_rect) const +Vector TextLayout::wrap_lines(TextElision elision, TextWrapping wrapping) const { Vector blocks; @@ -106,34 +106,16 @@ Vector TextLayout::wrap_lines(TextElision elision, TextWra }); } - size_t max_lines_that_can_fit = 0; - if (m_rect.height() >= m_font.glyph_height()) { - // NOTE: If glyph height is 10 and line spacing is 1, we can fit a - // single line into a 10px rect and a 20px rect, but 2 lines into a - // 21px rect. - max_lines_that_can_fit = 1 + (m_rect.height() - m_font.glyph_height()) / (m_font.glyph_height() + line_spacing); - } - - if (max_lines_that_can_fit == 0) - return {}; - Vector lines; StringBuilder builder; float line_width = 0; size_t current_block = 0; - bool did_not_finish = false; for (Block& block : blocks) { switch (block.type) { case BlockType::Newline: { lines.append(builder.to_deprecated_string()); builder.clear(); line_width = 0; - - if (lines.size() == max_lines_that_can_fit && fit_within_rect == FitWithinRect::Yes) { - did_not_finish = true; - goto blocks_processed; - } - current_block++; continue; } @@ -152,11 +134,6 @@ Vector TextLayout::wrap_lines(TextElision elision, TextWra line_width = 0; } - if (lines.size() == max_lines_that_can_fit && fit_within_rect == FitWithinRect::Yes) { - did_not_finish = true; - goto blocks_processed; - } - builder.append(block.characters.as_string()); line_width += block_width; current_block++; @@ -164,18 +141,15 @@ Vector TextLayout::wrap_lines(TextElision elision, TextWra } } -blocks_processed: - if (!did_not_finish) { - auto last_line = builder.to_deprecated_string(); - if (!last_line.is_empty()) - lines.append(last_line); - } + auto last_line = builder.to_deprecated_string(); + if (!last_line.is_empty()) + lines.append(last_line); switch (elision) { case TextElision::None: break; case TextElision::Right: { - lines.at(lines.size() - 1) = elide_text_from_right(Utf8View { lines.at(lines.size() - 1) }, did_not_finish); + lines.at(lines.size() - 1) = elide_text_from_right(Utf8View { lines.at(lines.size() - 1) }); break; } } @@ -183,10 +157,10 @@ blocks_processed: return lines; } -DeprecatedString TextLayout::elide_text_from_right(Utf8View text, bool force_elision) const +DeprecatedString TextLayout::elide_text_from_right(Utf8View text) const { float text_width = m_font.width(text); - if (force_elision || text_width > static_cast(m_rect.width())) { + if (text_width > static_cast(m_rect.width())) { float ellipsis_width = m_font.width("..."sv); float current_width = ellipsis_width; size_t glyph_spacing = m_font.glyph_spacing(); diff --git a/Userland/Libraries/LibGfx/TextLayout.h b/Userland/Libraries/LibGfx/TextLayout.h index 17a44757d2..944876697c 100644 --- a/Userland/Libraries/LibGfx/TextLayout.h +++ b/Userland/Libraries/LibGfx/TextLayout.h @@ -20,11 +20,6 @@ namespace Gfx { -enum class FitWithinRect { - Yes, - No -}; - // FIXME: This currently isn't an ideal way of doing things; ideally, TextLayout // would be doing the rendering by painting individual glyphs. However, this // would regress our Unicode bidirectional text support. Therefore, fixing this @@ -46,23 +41,25 @@ class TextLayout { public: TextLayout(Gfx::Font const& font, Utf8View const& text, FloatRect const& rect) : m_font(font) + , m_font_metrics(font.pixel_metrics()) , m_text(text) , m_rect(rect) { } - Vector lines(TextElision elision, TextWrapping wrapping, int line_spacing) const + Vector lines(TextElision elision, TextWrapping wrapping) const { - return wrap_lines(elision, wrapping, line_spacing, FitWithinRect::Yes); + return wrap_lines(elision, wrapping); } - FloatRect bounding_rect(TextWrapping wrapping, int line_spacing) const; + FloatRect bounding_rect(TextWrapping) const; private: - Vector wrap_lines(TextElision, TextWrapping, int line_spacing, FitWithinRect) const; - DeprecatedString elide_text_from_right(Utf8View, bool force_elision) const; + Vector wrap_lines(TextElision, TextWrapping) const; + DeprecatedString elide_text_from_right(Utf8View) const; Font const& m_font; + FontPixelMetrics m_font_metrics; Utf8View m_text; FloatRect m_rect; };