Skip to content

Commit 71a8568

Browse files
raphamorimraphamorim
authored
Fix underline metrics (#1220)
* Fix underline and strikethrough positioning using font metrics Improve underline and strikethrough positioning by using actual font metrics instead of hardcoded values, ensuring proper alignment with terminal cell grid. * Improve underline positioning using proper font metrics - Remove hardcoded size and offset fields from UnderlineInfo struct - Update underline offset calculation to trust font metrics more directly - Add font-based thickness support using stroke_size from font metrics - Enhance both underline and strikethrough with proper font metrics - Use accurate underline positioning that matches font designer intentions * Trust font metrics for underline positioning - Remove forced negation of underline offset - Use font metrics directly as they should already be properly positioned - Only apply fallback for fonts with missing metrics - Font metrics show correct negative offset values (-1.37 for 28px font) * Fix underline positioning by negating font metrics offset - Rendering system uses: uy = baseline - offset - Font metrics provide negative offset (below baseline) - Need to negate font metrics to get correct rendering position - Now underlines appear below characters instead of above them * Position underlines relative to line height instead of font metrics - Calculate underline position as 20% of line height below baseline - This places underlines in the lower portion of the line - Should clear descenders while staying above the next line - More consistent positioning across different fonts * Add comprehensive unit tests for underline positioning - Add 12 unit tests covering underline offset calculation logic - Test font metrics usage, fallback behavior, and edge cases - Ensure underlines clear descenders (25% of font size minimum) - Test positioning relative to baseline and reasonable bounds - Verify UnderlineInfo struct behavior without size/offset fields - Tests validate proper underline positioning below text characters * Fix underline positioning with proper scaling and reasonable defaults - Use 10% of font size as default underline position (more reasonable than tiny font metrics) - Handle cases where font metrics are too small (< 5% of font size) - Enforce minimum 8% of font size or 2px for proper visibility - Update comprehensive unit tests to match new logic - Should now position underlines properly below text characters * Fix critical underline positioning bug in rendering - Change underline calculation from 'baseline - offset' to 'baseline + offset' - This was the root cause of underlines appearing above text instead of below - Add comprehensive debug unit tests that revealed the coordinate system issue - Underlines now properly appear below the baseline as intended - Unit tests validate the fix works correctly * Increase underline spacing for better visual separation - Increase default underline position from 10% to 15% of font size - Increase minimum offset from 8% to 12% of font size - Increase absolute minimum from 2px to 3px - This provides better spacing below text characters - Update unit tests to match new expected values * Improve underline and strikethrough positioning with better mathematical approach - Implement golden ratio (0.618) positioning for underlines within descent area - Use ascent-based calculation for strikethrough positioning (ascent * 0.25) - Replace jagged curly underlines with smooth circle-based wave rendering - Add comprehensive tests for actual rendering position validation - Ensure proper clearance below text to avoid clipping descenders - Use font metrics when available, with intelligent fallbacks * Improve curly underline rendering with triangle-based smooth curves - Replace circle-based approach with triangle segments for true curved edges - Use two triangles per segment to create thick, continuous wave lines - Reduce wave amplitude for more subtle, professional appearance - Eliminate dotted appearance in favor of smooth, connected curves - Maintain proper thickness scaling based on line height * Fix double underline positioning to render both lines below text Previously, the second underline in double underlines was positioned above the first one (uy - underline.size * 2), which could place it over the character text. Now both underlines render below the baseline with proper spacing between them. Update BatchManager comment to reflect current implementation The FIX comment was outdated - the code already correctly adds the offset to baseline. Updated comment to describe the current behavior. update compositor * 20% clearance * Improve underline thickness calculation and double underline spacing - Use font-size proportional thickness (8% of font size) for all underline types - Remove thickness multiplier for dotted/dashed underlines for consistency - Adjust double underline spacing to have gap equal to one thickness - Simplify fallback thickness calculation logic * Fix strikethrough positioning to match Zed and Neovide using proper font metrics - Use font's built-in strikeout_offset when available (primary method) - Fall back to x_height/2 above baseline (typographically correct) - Final fallback to approximate position at 30% of line height - Add x_height to font metrics pipeline for accurate positioning - Reduce default strikethrough thickness from 2.0 to 1.5 for better appearance - Add comprehensive tests for all positioning scenarios - Extract underline creation logic into testable helper method This matches how modern terminals like Zed and Neovide position strikethrough text decorations using proper typographic principles. Fix cursor block size to be based on font metrics instead of line height - Add ascent to font metrics pipeline (TextRunStyle, RunData, render_data) - Update cursor rendering to use font_height (ascent + descent) instead of line_height - Calculate cursor_top as baseline - ascent for proper positioning - Cursor size now remains consistent regardless of line height increases - Add comprehensive tests for cursor font-based sizing - Test different font metrics combinations and line heights - Verify cursor positioning with various ascent/descent ratios This ensures that increasing line height for better readability doesn't make the cursor unnecessarily tall, matching the behavior of modern text editors. Clean up tests and comments - Remove println! statements from tests for cleaner output - Remove references to other editors from comments - Update cursor height calculations in tests to use font_height instead of line_height - Maintain focus on Rio's own implementation and standards * fix lint
1 parent 290f771 commit 71a8568

File tree

10 files changed

+814
-205
lines changed

10 files changed

+814
-205
lines changed

‎frontends/rioterm/src/renderer/mod.rs

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -196,38 +196,28 @@ impl Renderer {
196196

197197
if square.flags.contains(Flags::UNDERLINE) {
198198
decoration = Some(FragmentStyleDecoration::Underline(UnderlineInfo {
199-
offset: -1.0,
200-
size: 1.0,
201199
is_doubled: false,
202200
shape: UnderlineShape::Regular,
203201
}));
204202
} else if square.flags.contains(Flags::STRIKEOUT) {
205203
decoration = Some(FragmentStyleDecoration::Strikethrough);
206204
} else if square.flags.contains(Flags::DOUBLE_UNDERLINE) {
207205
decoration = Some(FragmentStyleDecoration::Underline(UnderlineInfo {
208-
offset: -1.0,
209-
size: 1.0,
210206
is_doubled: true,
211207
shape: UnderlineShape::Regular,
212208
}));
213209
} else if square.flags.contains(Flags::DOTTED_UNDERLINE) {
214210
decoration = Some(FragmentStyleDecoration::Underline(UnderlineInfo {
215-
offset: -1.0,
216-
size: 2.0,
217211
is_doubled: false,
218212
shape: UnderlineShape::Dotted,
219213
}));
220214
} else if square.flags.contains(Flags::DASHED_UNDERLINE) {
221215
decoration = Some(FragmentStyleDecoration::Underline(UnderlineInfo {
222-
offset: -1.0,
223-
size: 2.0,
224216
is_doubled: false,
225217
shape: UnderlineShape::Dashed,
226218
}));
227219
} else if square.flags.contains(Flags::UNDERCURL) {
228220
decoration = Some(FragmentStyleDecoration::Underline(UnderlineInfo {
229-
offset: -1.0,
230-
size: 2.0,
231221
is_doubled: false,
232222
shape: UnderlineShape::Curly,
233223
}));
@@ -294,8 +284,6 @@ impl Renderer {
294284
{
295285
style.decoration =
296286
Some(FragmentStyleDecoration::Underline(UnderlineInfo {
297-
offset: -1.0,
298-
size: -1.0,
299287
is_doubled: false,
300288
shape: UnderlineShape::Regular,
301289
}));
@@ -641,8 +629,6 @@ impl Renderer {
641629
CursorShape::Underline => {
642630
style.decoration =
643631
Some(FragmentStyleDecoration::Underline(UnderlineInfo {
644-
offset: 0.0,
645-
size: 3.0,
646632
is_doubled: false,
647633
shape: UnderlineShape::Regular,
648634
}));

‎sugarloaf/examples/text.rs

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,6 @@ impl ApplicationHandler for Application {
154154
FragmentStyle {
155155
decoration: Some(FragmentStyleDecoration::Underline(
156156
UnderlineInfo {
157-
offset: -2.0,
158-
size: 1.0,
159157
is_doubled: false,
160158
shape: UnderlineShape::Regular,
161159
},
@@ -179,8 +177,6 @@ impl ApplicationHandler for Application {
179177
FragmentStyle {
180178
decoration: Some(FragmentStyleDecoration::Underline(
181179
UnderlineInfo {
182-
offset: -2.0,
183-
size: 1.0,
184180
is_doubled: false,
185181
shape: UnderlineShape::Regular,
186182
},
@@ -204,8 +200,6 @@ impl ApplicationHandler for Application {
204200
FragmentStyle {
205201
decoration: Some(FragmentStyleDecoration::Underline(
206202
UnderlineInfo {
207-
offset: -2.0,
208-
size: 1.0,
209203
is_doubled: false,
210204
shape: UnderlineShape::Curly,
211205
},
@@ -221,8 +215,6 @@ impl ApplicationHandler for Application {
221215
FragmentStyle {
222216
decoration: Some(FragmentStyleDecoration::Underline(
223217
UnderlineInfo {
224-
offset: -2.0,
225-
size: 1.0,
226218
is_doubled: false,
227219
shape: UnderlineShape::Dashed,
228220
},
@@ -246,8 +238,6 @@ impl ApplicationHandler for Application {
246238
FragmentStyle {
247239
decoration: Some(FragmentStyleDecoration::Underline(
248240
UnderlineInfo {
249-
offset: -2.0,
250-
size: 1.0,
251241
is_doubled: false,
252242
shape: UnderlineShape::Dotted,
253243
},

‎sugarloaf/src/components/rich_text/batch.rs

Lines changed: 51 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use bytemuck::{Pod, Zeroable};
1717
#[derive(Default, Clone, Copy)]
1818
pub struct RunUnderline {
1919
pub enabled: bool,
20-
pub offset: i32,
20+
pub offset: f32,
2121
pub size: f32,
2222
pub color: [f32; 4],
2323
pub is_doubled: bool,
@@ -3803,7 +3803,10 @@ impl BatchManager {
38033803
) {
38043804
if underline.enabled {
38053805
let ux = x;
3806-
let uy = baseline - underline.offset as f32;
3806+
// Position underline below baseline by adding the calculated offset
3807+
// This ensures proper underline placement in the descent area
3808+
let uy = baseline + underline.offset;
3809+
38073810
let end = x + advance;
38083811
if ux < end {
38093812
match underline.shape {
@@ -3814,10 +3817,12 @@ impl BatchManager {
38143817
&underline.color,
38153818
);
38163819
if underline.is_doubled {
3820+
// Position the second underline with a gap equal to thickness
3821+
// First line is at uy, gap of underline.size, then second line
38173822
self.add_rect(
38183823
&Rect::new(
38193824
ux,
3820-
uy - (underline.size * 2.),
3825+
uy + (underline.size * 2.0),
38213826
end - ux,
38223827
underline.size,
38233828
),
@@ -3851,35 +3856,53 @@ impl BatchManager {
38513856
}
38523857
}
38533858
UnderlineShape::Curly => {
3854-
let style_line_height = (line_height / 10.).clamp(2.0, 16.0);
3855-
let size = (style_line_height / 1.5).clamp(1.0, 4.0);
3856-
let offset = style_line_height * 1.6;
3857-
3858-
let mut curly_width = ux;
3859-
let mut rect_width = 1.0f32.min(end - curly_width);
3860-
3861-
while curly_width < end {
3862-
rect_width = rect_width.min(end - curly_width);
3863-
3864-
let dot_bottom_offset = match curly_width as u32 % 8 {
3865-
3..=5 => offset + style_line_height,
3866-
2 | 6 => offset + 2.0 * style_line_height / 3.0,
3867-
1 | 7 => offset + 1.0 * style_line_height / 3.0,
3868-
_ => offset,
3869-
};
3859+
// Create smooth curly underlines using triangles to form thick curved segments
3860+
let wave_amplitude = (line_height / 12.).clamp(0.9, 1.8); // Slightly reduced amplitude
3861+
let wave_frequency = 8.0; // pixels per complete wave cycle
3862+
let thickness = (line_height / 16.).clamp(1.0, 2.0);
3863+
3864+
let mut x = ux;
3865+
let step_size: f32 = 0.8; // Larger steps for triangle segments
3866+
3867+
while x < end - step_size {
3868+
let progress1 = (x - ux) / wave_frequency;
3869+
let progress2 = ((x + step_size) - ux) / wave_frequency;
3870+
3871+
let wave_phase1 = progress1 * std::f32::consts::PI * 2.0;
3872+
let wave_phase2 = progress2 * std::f32::consts::PI * 2.0;
3873+
3874+
// Calculate Y positions for current and next points
3875+
let y1 = uy + wave_phase1.sin() * wave_amplitude;
3876+
let y2 = uy + wave_phase2.sin() * wave_amplitude;
3877+
3878+
// Create thick line segment using two triangles (quad)
3879+
let half_thickness = thickness * 0.5;
3880+
3881+
// Top triangle of the quad
3882+
self.add_triangle(
3883+
x,
3884+
y1 - half_thickness, // top-left
3885+
x + step_size,
3886+
y2 - half_thickness, // top-right
3887+
x,
3888+
y1 + half_thickness, // bottom-left
3889+
depth,
3890+
underline.color,
3891+
);
38703892

3871-
self.add_rect(
3872-
&Rect::new(
3873-
curly_width,
3874-
uy - (dot_bottom_offset - offset),
3875-
rect_width,
3876-
size,
3877-
),
3893+
// Bottom triangle of the quad
3894+
self.add_triangle(
3895+
x + step_size,
3896+
y2 - half_thickness, // top-right
3897+
x + step_size,
3898+
y2 + half_thickness, // bottom-right
3899+
x,
3900+
y1 + half_thickness, // bottom-left
38783901
depth,
3879-
&underline.color,
3902+
underline.color,
38803903
);
38813904

3882-
curly_width += rect_width;
3905+
x += step_size;
38833906
}
38843907
}
38853908
}

0 commit comments

Comments
 (0)