LibGfx+LibWeb: Draw glyph runs with subpixel accuracy

This improves the quality of our font rendering, especially when animations are involved. Relevant changes: * Skia fonts have their subpixel flag set, which means that individual glyphs are rendered at subpixel offsets causing glyph runs as a whole to look better. * Fragment offsets are no longer rounded to whole device pixels, and instead the floating point offset is kept. This allows us to pass through the floating point baseline position all the way to the Skia calls, which already expected that to be a float position. The `scrollable-contains-table.html` ref test needed different table headings since they would slightly inflate the column size in the test file, but not the reference.
sakgoyal · Dec 21, 2024 · 4d9f17e · 4d9f17e
1 parent e32a9b2
commit 4d9f17e
Show file tree

Hide file tree

Showing 83 changed files with 524 additions and 525 deletions.
diff --git a/Libraries/LibGfx/Font/ScaledFontSkia.cpp b/Libraries/LibGfx/Font/ScaledFontSkia.cpp
@@ -15,7 +15,9 @@ namespace Gfx {
 SkFont ScaledFont::skia_font(float scale) const
 {
     auto const& sk_typeface = verify_cast<TypefaceSkia>(*m_typeface).sk_typeface();
-    return SkFont { sk_ref_sp(sk_typeface), pixel_size() * scale };
+    auto sk_font = SkFont { sk_ref_sp(sk_typeface), pixel_size() * scale };
+    sk_font.setSubpixel(true);
+    return sk_font;
 }
 
 }
diff --git a/Libraries/LibWeb/Layout/LineBoxFragment.cpp b/Libraries/LibWeb/Layout/LineBoxFragment.cpp
@@ -5,10 +5,8 @@
  */
 
 #include <AK/Utf8View.h>
-#include <LibWeb/DOM/Range.h>
 #include <LibWeb/Layout/LayoutState.h>
 #include <LibWeb/Layout/TextNode.h>
-#include <LibWeb/Layout/Viewport.h>
 #include <ctype.h>
 
 namespace Web::Layout {

diff --git a/Libraries/LibWeb/Layout/LineBuilder.cpp b/Libraries/LibWeb/Layout/LineBuilder.cpp
@@ -269,7 +269,7 @@ void LineBuilder::update_last_line()
     for (size_t i = 0; i < line_box.fragments().size(); ++i) {
         auto& fragment = line_box.fragments()[i];
 
-        CSSPixels new_fragment_inline_offset = round(inline_offset + fragment.inline_offset());
+        CSSPixels new_fragment_inline_offset = inline_offset + fragment.inline_offset();
         CSSPixels new_fragment_block_offset = 0;
 
         auto block_offset_value_for_alignment = [&](CSS::VerticalAlign vertical_align) {

diff --git a/Libraries/LibWeb/Layout/Node.h b/Libraries/LibWeb/Layout/Node.h
@@ -7,10 +7,7 @@
 #pragma once
 
 #include <AK/NonnullRefPtr.h>
-#include <AK/TypeCasts.h>
 #include <AK/Vector.h>
-#include <LibGC/Root.h>
-#include <LibGfx/Rect.h>
 #include <LibJS/Heap/Cell.h>
 #include <LibWeb/CSS/ComputedValues.h>
 #include <LibWeb/CSS/StyleComputer.h>

diff --git a/Libraries/LibWeb/Painting/BackgroundPainting.cpp b/Libraries/LibWeb/Painting/BackgroundPainting.cpp
@@ -29,9 +29,12 @@ static RefPtr<DisplayList> compute_text_clip_paths(PaintContext& context, Painta
         auto fragment_absolute_rect = fragment.absolute_rect();
         auto fragment_absolute_device_rect = context.enclosing_device_rect(fragment_absolute_rect);
 
-        DevicePixelPoint baseline_start { fragment_absolute_device_rect.x(), fragment_absolute_device_rect.y() + context.rounded_device_pixels(fragment.baseline()) };
         auto scale = context.device_pixels_per_css_pixel();
-        display_list_recorder.draw_text_run(baseline_start.to_type<int>(), *glyph_run, Gfx::Color::Black, fragment_absolute_device_rect.to_type<int>(), scale, fragment.orientation());
+        auto baseline_start = Gfx::FloatPoint {
+            fragment_absolute_rect.x().to_float(),
+            fragment_absolute_rect.y().to_float() + fragment.baseline().to_float(),
+        } * scale;
+        display_list_recorder.draw_text_run(baseline_start, *glyph_run, Gfx::Color::Black, fragment_absolute_device_rect.to_type<int>(), scale, fragment.orientation());
     };
 
     paintable.for_each_in_inclusive_subtree([&](auto& paintable) {

diff --git a/Libraries/LibWeb/Painting/Command.h b/Libraries/LibWeb/Painting/Command.h
@@ -164,12 +164,12 @@ struct PaintTextShadow {
     double glyph_run_scale { 1 };
     Gfx::IntRect shadow_bounding_rect;
     Gfx::IntRect text_rect;
-    Gfx::IntPoint draw_location;
+    Gfx::FloatPoint draw_location;
     int blur_radius;
     Color color;
 
-    [[nodiscard]] Gfx::IntRect bounding_rect() const { return { draw_location, shadow_bounding_rect.size() }; }
-    void translate_by(Gfx::IntPoint const& offset) { draw_location.translate_by(offset); }
+    [[nodiscard]] Gfx::IntRect bounding_rect() const { return { draw_location.to_type<int>(), shadow_bounding_rect.size() }; }
+    void translate_by(Gfx::IntPoint const& offset) { draw_location.translate_by(offset.to_type<float>()); }
 };
 
 struct FillRectWithRoundedCorners {

diff --git a/Libraries/LibWeb/Painting/DisplayListPlayerSkia.cpp b/Libraries/LibWeb/Painting/DisplayListPlayerSkia.cpp
@@ -504,7 +504,7 @@ void DisplayListPlayerSkia::paint_text_shadow(PaintTextShadow const& command)
         .glyph_run = command.glyph_run,
         .scale = command.glyph_run_scale,
         .rect = command.text_rect,
-        .translation = command.draw_location.to_type<float>() + command.text_rect.location().to_type<float>(),
+        .translation = command.draw_location + command.text_rect.location().to_type<float>(),
         .color = command.color,
     });
     canvas.restore();

diff --git a/Libraries/LibWeb/Painting/DisplayListRecorder.cpp b/Libraries/LibWeb/Painting/DisplayListRecorder.cpp
@@ -244,18 +244,18 @@ void DisplayListRecorder::draw_text(Gfx::IntRect const& rect, String raw_text, G
     }
     auto metrics = font.pixel_metrics();
     float baseline_y = static_cast<float>(rect.y()) + metrics.ascent + (static_cast<float>(rect.height()) - (metrics.ascent + metrics.descent)) / 2.0f;
-    draw_text_run(Gfx::IntPoint(roundf(baseline_x), roundf(baseline_y)), *glyph_run, color, rect, 1.0, Orientation::Horizontal);
+    draw_text_run({ baseline_x, baseline_y }, *glyph_run, color, rect, 1.0, Orientation::Horizontal);
 }
 
-void DisplayListRecorder::draw_text_run(Gfx::IntPoint baseline_start, Gfx::GlyphRun const& glyph_run, Color color, Gfx::IntRect const& rect, double scale, Orientation orientation)
+void DisplayListRecorder::draw_text_run(Gfx::FloatPoint baseline_start, Gfx::GlyphRun const& glyph_run, Color color, Gfx::IntRect const& rect, double scale, Orientation orientation)
 {
     if (rect.is_empty())
         return;
     append(DrawGlyphRun {
         .glyph_run = glyph_run,
         .scale = scale,
         .rect = rect,
-        .translation = baseline_start.to_type<float>(),
+        .translation = baseline_start,
         .color = color,
         .orientation = orientation,
     });
@@ -331,7 +331,7 @@ void DisplayListRecorder::paint_inner_box_shadow_params(PaintBoxShadowParams par
     append(PaintInnerBoxShadow { .box_shadow_params = params });
 }
 
-void DisplayListRecorder::paint_text_shadow(int blur_radius, Gfx::IntRect bounding_rect, Gfx::IntRect text_rect, Gfx::GlyphRun const& glyph_run, double glyph_run_scale, Color color, Gfx::IntPoint draw_location)
+void DisplayListRecorder::paint_text_shadow(int blur_radius, Gfx::IntRect bounding_rect, Gfx::IntRect text_rect, Gfx::GlyphRun const& glyph_run, double glyph_run_scale, Color color, Gfx::FloatPoint draw_location)
 {
     append(PaintTextShadow {
         .glyph_run = glyph_run,

diff --git a/Libraries/LibWeb/Painting/DisplayListRecorder.h b/Libraries/LibWeb/Painting/DisplayListRecorder.h
@@ -8,8 +8,6 @@
 
 #include <AK/Forward.h>
 #include <AK/NonnullRefPtr.h>
-#include <AK/SegmentedVector.h>
-#include <AK/Utf8View.h>
 #include <AK/Vector.h>
 #include <LibGfx/Color.h>
 #include <LibGfx/Forward.h>
@@ -106,7 +104,7 @@ class DisplayListRecorder {
     void draw_text(Gfx::IntRect const&, String, Gfx::Font const&, Gfx::TextAlignment, Color);
 
     // Streamlined text drawing routine that does no wrapping/elision/alignment.
-    void draw_text_run(Gfx::IntPoint baseline_start, Gfx::GlyphRun const& glyph_run, Color color, Gfx::IntRect const& rect, double scale, Gfx::Orientation);
+    void draw_text_run(Gfx::FloatPoint baseline_start, Gfx::GlyphRun const& glyph_run, Color color, Gfx::IntRect const& rect, double scale, Gfx::Orientation);
 
     void add_clip_rect(Gfx::IntRect const& rect);
 
@@ -137,7 +135,7 @@ class DisplayListRecorder {
 
     void paint_outer_box_shadow_params(PaintBoxShadowParams params);
     void paint_inner_box_shadow_params(PaintBoxShadowParams params);
-    void paint_text_shadow(int blur_radius, Gfx::IntRect bounding_rect, Gfx::IntRect text_rect, Gfx::GlyphRun const&, double glyph_run_scale, Color color, Gfx::IntPoint draw_location);
+    void paint_text_shadow(int blur_radius, Gfx::IntRect bounding_rect, Gfx::IntRect text_rect, Gfx::GlyphRun const&, double glyph_run_scale, Color color, Gfx::FloatPoint draw_location);
 
     void fill_rect_with_rounded_corners(Gfx::IntRect const& rect, Color color, CornerRadius top_left_radius, CornerRadius top_right_radius, CornerRadius bottom_right_radius, CornerRadius bottom_left_radius);
     void fill_rect_with_rounded_corners(Gfx::IntRect const& a_rect, Color color, int radius);

diff --git a/Libraries/LibWeb/Painting/PaintableBox.cpp b/Libraries/LibWeb/Painting/PaintableBox.cpp
@@ -683,16 +683,19 @@ void paint_text_fragment(PaintContext& context, TextPaintable const& paintable,
         if (!glyph_run)
             return;
 
-        DevicePixelPoint baseline_start { fragment_absolute_device_rect.x(), fragment_absolute_device_rect.y() + context.rounded_device_pixels(fragment.baseline()) };
         auto scale = context.device_pixels_per_css_pixel();
-        painter.draw_text_run(baseline_start.to_type<int>(), *glyph_run, paintable.computed_values().webkit_text_fill_color(), fragment_absolute_device_rect.to_type<int>(), scale, fragment.orientation());
+        auto baseline_start = Gfx::FloatPoint {
+            fragment_absolute_rect.x().to_float(),
+            fragment_absolute_rect.y().to_float() + fragment.baseline().to_float(),
+        } * scale;
+        painter.draw_text_run(baseline_start, *glyph_run, paintable.computed_values().webkit_text_fill_color(), fragment_absolute_device_rect.to_type<int>(), scale, fragment.orientation());
 
         auto selection_rect = context.enclosing_device_rect(fragment.selection_rect()).to_type<int>();
         if (!selection_rect.is_empty()) {
             painter.fill_rect(selection_rect, CSS::SystemColor::highlight());
             DisplayListRecorderStateSaver saver(painter);
             painter.add_clip_rect(selection_rect);
-            painter.draw_text_run(baseline_start.to_type<int>(), *glyph_run, CSS::SystemColor::highlight_text(), fragment_absolute_device_rect.to_type<int>(), scale, fragment.orientation());
+            painter.draw_text_run(baseline_start, *glyph_run, CSS::SystemColor::highlight_text(), fragment_absolute_device_rect.to_type<int>(), scale, fragment.orientation());
         }
 
         paint_text_decoration(context, paintable, fragment);
@@ -744,9 +747,8 @@ void PaintableWithLines::paint(PaintContext& context, PaintPhase phase) const
     // So, we paint the shadows before painting any text.
     // FIXME: Find a smarter way to do this?
     if (phase == PaintPhase::Foreground) {
-        for (auto& fragment : fragments()) {
+        for (auto& fragment : fragments())
             paint_text_shadow(context, fragment, fragment.shadows());
-        }
     }
 
     for (auto const& fragment : m_fragments) {

diff --git a/Libraries/LibWeb/Painting/ShadowPainting.cpp b/Libraries/LibWeb/Painting/ShadowPainting.cpp
@@ -74,13 +74,10 @@ void paint_text_shadow(PaintContext& context, PaintableFragment const& fragment,
 
     auto fragment_width = context.enclosing_device_pixels(fragment.width()).value();
     auto fragment_height = context.enclosing_device_pixels(fragment.height()).value();
-    auto draw_rect = context.enclosing_device_rect(fragment.absolute_rect()).to_type<int>();
     auto fragment_baseline = context.rounded_device_pixels(fragment.baseline()).value();
 
     // Note: Box-shadow layers are ordered front-to-back, so we paint them in reverse
     for (auto& layer : shadow_layers.in_reverse()) {
-        int offset_x = context.rounded_device_pixels(layer.offset_x).value();
-        int offset_y = context.rounded_device_pixels(layer.offset_y).value();
         int blur_radius = context.rounded_device_pixels(layer.blur_radius).value();
 
         // Space around the painted text to allow it to blur.
@@ -95,12 +92,14 @@ void paint_text_shadow(PaintContext& context, PaintableFragment const& fragment,
             text_rect.width() + margin + margin,
             text_rect.height() + margin + margin
         };
-        Gfx::IntPoint draw_location {
-            draw_rect.x() + offset_x - margin,
-            draw_rect.y() + offset_y - margin
-        };
 
-        context.display_list_recorder().paint_text_shadow(blur_radius, bounding_rect, text_rect.translated(0, fragment_baseline), *glyph_run, context.device_pixels_per_css_pixel(), layer.color, draw_location);
+        auto scale = context.device_pixels_per_css_pixel();
+        auto draw_location = Gfx::FloatPoint {
+            fragment.absolute_rect().x() + layer.offset_x - margin,
+            fragment.absolute_rect().y() + layer.offset_y - margin,
+        } * scale;
+
+        context.display_list_recorder().paint_text_shadow(blur_radius, bounding_rect, text_rect.translated(0, fragment_baseline), *glyph_run, scale, layer.color, draw_location);
     }
 }
 

diff --git a/Tests/LibWeb/Layout/expected/acid1.txt b/Tests/LibWeb/Layout/expected/acid1.txt
@@ -38,16 +38,16 @@ Viewport <#document> at (0,0) content-size 800x600 children: not-inline
               BlockContainer <p> at (235,65) content-size 139.96875x19 children: inline
                 frag 0 from TextNode start: 1, length: 5, rect: [235,65 27.5x19] baseline: 12.5
                     "bang "
-                frag 1 from RadioButton start: 0, length: 0, rect: [263,65 12x12] baseline: 12
+                frag 1 from RadioButton start: 0, length: 0, rect: [262.5,65 12x12] baseline: 12
                 TextNode <#text>
-                RadioButton <input> at (263,65) content-size 12x12 inline-block children: not-inline
+                RadioButton <input> at (262.5,65) content-size 12x12 inline-block children: not-inline
                 TextNode <#text>
               BlockContainer <p> at (235,84) content-size 139.96875x19 children: inline
                 frag 0 from TextNode start: 1, length: 8, rect: [235,84 45.171875x19] baseline: 12.5
                     "whimper "
-                frag 1 from RadioButton start: 0, length: 0, rect: [280,84 12x12] baseline: 12
+                frag 1 from RadioButton start: 0, length: 0, rect: [280.171875,84 12x12] baseline: 12
                 TextNode <#text>
-                RadioButton <input> at (280,84) content-size 12x12 inline-block children: not-inline
+                RadioButton <input> at (280.171875,84) content-size 12x12 inline-block children: not-inline
                 TextNode <#text>
               BlockContainer <(anonymous)> at (235,103) content-size 139.96875x0 children: inline
                 TextNode <#text>
@@ -95,22 +95,22 @@ Viewport <#document> at (0,0) content-size 800x600 children: not-inline
             "agents should be able to render the document elements above this paragraph"
         frag 2 from TextNode start: 167, length: 43, rect: [20,361 207.9375x13] baseline: 9.5
             "indistinguishably (to the pixel) from this "
-        frag 3 from TextNode start: 0, length: 31, rect: [331,361 159.671875x13] baseline: 9.5
+        frag 3 from TextNode start: 0, length: 31, rect: [330.96875,361 159.671875x13] baseline: 9.5
             " (except font rasterization and"
         frag 4 from TextNode start: 32, length: 89, rect: [20,374 465.09375x13] baseline: 9.5
             "form widgets). All discrepancies should be traceable to CSS1 implementation shortcomings."
         frag 5 from TextNode start: 122, length: 67, rect: [20,387 345.59375x13] baseline: 9.5
             "Once you have finished evaluating this test, you can return to the "
-        frag 6 from TextNode start: 0, length: 1, rect: [426,387 2.71875x13] baseline: 9.5
+        frag 6 from TextNode start: 0, length: 1, rect: [425.5,387 2.71875x13] baseline: 9.5
             "."
         TextNode <#text>
         InlineNode <a>
-          frag 0 from TextNode start: 0, length: 20, rect: [228,361 103.03125x13] baseline: 9.5
+          frag 0 from TextNode start: 0, length: 20, rect: [227.9375,361 103.03125x13] baseline: 9.5
               "reference rendering,"
           TextNode <#text>
         TextNode <#text>
         InlineNode <a>
-          frag 0 from TextNode start: 0, length: 11, rect: [366,387 59.90625x13] baseline: 9.5
+          frag 0 from TextNode start: 0, length: 11, rect: [365.59375,387 59.90625x13] baseline: 9.5
               "parent page"
           TextNode <#text>
         TextNode <#text>
@@ -138,10 +138,10 @@ ViewportPaintable (Viewport<#document>) [0,0 800x600]
                 PaintableWithLines (InlineNode<FORM>)
               PaintableWithLines (BlockContainer<P>) [235,65 139.96875x19]
                 TextPaintable (TextNode<#text>)
-                RadioButtonPaintable (RadioButton<INPUT>) [263,65 12x12]
+                RadioButtonPaintable (RadioButton<INPUT>) [262.5,65 12x12]
               PaintableWithLines (BlockContainer<P>) [235,84 139.96875x19]
                 TextPaintable (TextNode<#text>)
-                RadioButtonPaintable (RadioButton<INPUT>) [280,84 12x12]
+                RadioButtonPaintable (RadioButton<INPUT>) [280.171875,84 12x12]
               PaintableWithLines (BlockContainer(anonymous)) [235,103 139.96875x0]
             PaintableWithLines (BlockContainer<LI>) [394.96875,45 80x120]
               TextPaintable (TextNode<#text>)

diff --git a/Tests/LibWeb/Layout/expected/block-and-inline/atomic-inline-with-white-space-nowrap.txt b/Tests/LibWeb/Layout/expected/block-and-inline/atomic-inline-with-white-space-nowrap.txt
@@ -4,14 +4,14 @@ Viewport <#document> at (0,0) content-size 800x600 children: not-inline
       BlockContainer <div> at (8,8) content-size 37.15625x17 children: inline
         frag 0 from TextNode start: 0, length: 3, rect: [8,8 27.15625x17] baseline: 13.296875
             "foo"
-        frag 1 from ImageBox start: 0, length: 0, rect: [35,11 10x10] baseline: 10
+        frag 1 from ImageBox start: 0, length: 0, rect: [35.15625,11 10x10] baseline: 10
         TextNode <#text>
-        ImageBox <img> at (35,11) content-size 10x10 children: not-inline
+        ImageBox <img> at (35.15625,11) content-size 10x10 children: not-inline
         TextNode <#text>
 
 ViewportPaintable (Viewport<#document>) [0,0 800x600]
   PaintableWithLines (BlockContainer<HTML>) [0,0 800x33]
     PaintableWithLines (BlockContainer<BODY>) [8,8 37.15625x17]
       PaintableWithLines (BlockContainer<DIV>) [8,8 37.15625x17]
         TextPaintable (TextNode<#text>)
-        ImagePaintable (ImageBox<IMG>) [35,11 10x10]
+        ImagePaintable (ImageBox<IMG>) [35.15625,11 10x10]
diff --git a/Tests/LibWeb/Layout/expected/block-and-inline/bfc-consider-all-currently-stacked-floats.txt b/Tests/LibWeb/Layout/expected/block-and-inline/bfc-consider-all-currently-stacked-floats.txt
@@ -9,12 +9,12 @@ Viewport <#document> at (0,0) content-size 800x600 children: not-inline
             "xxx"
         TextNode <#text>
       BlockContainer <(anonymous)> at (8,8) content-size 784x17 children: inline
-        frag 0 from TextNode start: 1, length: 3, rect: [137,8 27.640625x17] baseline: 13.296875
+        frag 0 from TextNode start: 1, length: 3, rect: [137.109375,8 27.640625x17] baseline: 13.296875
             "bar"
         TextNode <#text>
         TextNode <#text>
       BlockContainer <div> at (8,25) content-size 784x17 children: inline
-        frag 0 from TextNode start: 1, length: 3, rect: [130,25 27.203125x17] baseline: 13.296875
+        frag 0 from TextNode start: 1, length: 3, rect: [129.515625,25 27.203125x17] baseline: 13.296875
             "baz"
         TextNode <#text>
         BlockContainer <div.yyy> at (108,25) content-size 21.515625x17 floating [BFC] children: inline