Skip to content

Commit

Permalink
LibGfx+LibWeb: Draw glyph runs with subpixel accuracy
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
gmta committed Dec 10, 2024
1 parent 617ad9c commit 87085da
Show file tree
Hide file tree
Showing 80 changed files with 510 additions and 509 deletions.
4 changes: 3 additions & 1 deletion Libraries/LibGfx/Font/ScaledFontSkia.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

}
2 changes: 0 additions & 2 deletions Libraries/LibWeb/Layout/LineBoxFragment.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion Libraries/LibWeb/Layout/LineBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
3 changes: 0 additions & 3 deletions Libraries/LibWeb/Layout/Node.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
7 changes: 5 additions & 2 deletions Libraries/LibWeb/Painting/BackgroundPainting.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
6 changes: 3 additions & 3 deletions Libraries/LibWeb/Painting/DisplayListRecorder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
Expand Down
4 changes: 1 addition & 3 deletions Libraries/LibWeb/Painting/DisplayListRecorder.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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);

Expand Down
9 changes: 6 additions & 3 deletions Libraries/LibWeb/Painting/PaintableBox.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -678,16 +678,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() + static_cast<float>(context.rounded_device_pixels(fragment.baseline()).value()),
} * 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);
Expand Down
20 changes: 10 additions & 10 deletions Tests/LibWeb/Layout/expected/acid1.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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>)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,19 @@ Viewport <#document> at (0,0) content-size 800x600 children: not-inline
frag 0 from BlockContainer start: 0, length: 0, rect: [8,8 61.1875x48] baseline: 36
BlockContainer <div.ib> at (8,8) content-size 61.1875x48 inline-block [BFC] children: inline
frag 0 from BlockContainer start: 0, length: 0, rect: [9,27 17.828125x22] baseline: 18
frag 1 from TextNode start: 0, length: 1, rect: [28,30 8x17] baseline: 13.296875
frag 1 from TextNode start: 0, length: 1, rect: [27.828125,30 8x17] baseline: 13.296875
" "
frag 2 from BlockContainer start: 0, length: 0, rect: [41,10 23.359375x44] baseline: 36
frag 2 from BlockContainer start: 0, length: 0, rect: [40.828125,10 23.359375x44] baseline: 36
TextNode <#text>
BlockContainer <div.label> at (9,27) content-size 17.828125x22 inline-block [BFC] children: inline
frag 0 from TextNode start: 0, length: 1, rect: [9,27 17.828125x22] baseline: 17
"A"
TextNode <#text>
TextNode <#text>
BlockContainer <button> at (41,10) content-size 23.359375x44 inline-block [BFC] children: not-inline
BlockContainer <(anonymous)> at (41,10) content-size 23.359375x44 flex-container(column) [FFC] children: not-inline
BlockContainer <(anonymous)> at (41,10) content-size 23.359375x44 flex-item [BFC] children: inline
frag 0 from TextNode start: 0, length: 1, rect: [41,10 23.359375x44] baseline: 34
BlockContainer <button> at (40.828125,10) content-size 23.359375x44 inline-block [BFC] children: not-inline
BlockContainer <(anonymous)> at (40.828125,10) content-size 23.359375x44 flex-container(column) [FFC] children: not-inline
BlockContainer <(anonymous)> at (40.828125,10) content-size 23.359375x44 flex-item [BFC] children: inline
frag 0 from TextNode start: 0, length: 1, rect: [40.828125,10 23.359375x44] baseline: 34
"B"
TextNode <#text>
TextNode <#text>
Expand All @@ -28,7 +28,7 @@ ViewportPaintable (Viewport<#document>) [0,0 800x600]
PaintableWithLines (BlockContainer<DIV>.label) [8,26 19.828125x24]
TextPaintable (TextNode<#text>)
TextPaintable (TextNode<#text>)
PaintableWithLines (BlockContainer<BUTTON>) [36,8 33.359375x48]
PaintableWithLines (BlockContainer(anonymous)) [41,10 23.359375x44]
PaintableWithLines (BlockContainer(anonymous)) [41,10 23.359375x44]
PaintableWithLines (BlockContainer<BUTTON>) [35.828125,8 33.359375x48]
PaintableWithLines (BlockContainer(anonymous)) [40.828125,10 23.359375x44]
PaintableWithLines (BlockContainer(anonymous)) [40.828125,10 23.359375x44]
TextPaintable (TextNode<#text>)
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ Viewport <#document> at (0,0) content-size 800x600 children: not-inline
BlockContainer <(anonymous)> at (22,66.5) content-size 48.6875x0 children: inline
TextNode <#text>
BlockContainer <div.border-black> at (32,76.5) content-size 28.6875x100 children: inline
frag 0 from TextNode start: 1, length: 3, rect: [32,76.5 28.4375x17] baseline: 13.296875
frag 0 from TextNode start: 1, length: 3, rect: [32.125,76.5 28.4375x17] baseline: 13.296875
"two"
TextNode <#text>
BlockContainer <(anonymous)> at (22,186.5) content-size 48.6875x0 children: inline
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Viewport <#document> at (0,0) content-size 800x600 children: not-inline
BlockContainer <button.btn.fixed-width> at (13,10) content-size 190x17 children: not-inline
BlockContainer <(anonymous)> at (13,10) content-size 190x17 flex-container(column) [FFC] children: not-inline
BlockContainer <(anonymous)> at (13,10) content-size 190x17 flex-item [BFC] children: inline
frag 0 from TextNode start: 0, length: 11, rect: [61,10 94.921875x17] baseline: 13.296875
frag 0 from TextNode start: 0, length: 11, rect: [60.53125,10 94.921875x17] baseline: 13.296875
"200px width"
TextNode <#text>
BlockContainer <button.btn> at (13,31) content-size 324.671875x17 children: not-inline
Expand Down
Loading

0 comments on commit 87085da

Please sign in to comment.