From aab0f8079f6994be6b056a9737d46408ae0d9d82 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 5 Jun 2026 13:43:59 -0700 Subject: [PATCH] Revert "font: add exact glyph protocol constraints" This reverts commit b661adad2ec9e32d739684b3051c733dbcd021ea. --- src/font/Glyph.zig | 266 +++++----------------------- src/font/glyf_rasterize.zig | 161 +---------------- src/terminal/apc/glyph.zig | 11 +- src/terminal/apc/glyph/Glossary.zig | 62 +++---- 4 files changed, 69 insertions(+), 431 deletions(-) diff --git a/src/font/Glyph.zig b/src/font/Glyph.zig index 039d69c8a..62c44e227 100644 --- a/src/font/Glyph.zig +++ b/src/font/Glyph.zig @@ -87,9 +87,6 @@ pub const RenderOptions = struct { /// Sizing rule. size: Constraint.Size = .none, - /// Target coordinate space for sizing, alignment, and padding. - target: Target = .face, - /// Vertical alignment rule. align_vertical: Align = .none, /// Horizontal alignment rule. @@ -139,21 +136,6 @@ pub const RenderOptions = struct { /// Stretch the glyph to exactly fit the bounds in both /// directions, disregarding aspect ratio. stretch, - /// Scale the glyph up or down to exactly match the target height, - /// preserving aspect ratio. - height, - /// Scale the glyph up or down to exactly match the target width, - /// preserving aspect ratio. - width, - /// Scale the glyph up or down to fit within the target bounds, - /// preserving aspect ratio. - contain, - /// Scale the glyph up or down to cover the target bounds, - /// preserving aspect ratio. - cover_bounds, - /// Stretch the glyph to exactly fit the target bounds in both - /// directions, disregarding aspect ratio. - stretch_bounds, }; pub const Align = enum { @@ -171,9 +153,6 @@ pub const RenderOptions = struct { /// but always with respect to the first cell even for /// multi-cell constraints. (Nerd Font specific rule.) center1, - /// Move the glyph so that its design-space baseline aligns with - /// the terminal text baseline. - baseline, }; pub const Height = enum { @@ -187,17 +166,6 @@ pub const RenderOptions = struct { icon, }; - pub const Target = enum { - /// Size and align relative to the primary face metrics. This is - /// the default behavior used for normal fonts, emoji, and Nerd - /// Font constraints. - face, - /// Size and align relative to the full terminal cell span. The - /// width is `cell_width * constraint_width`; the height is - /// `cell_height`. - cell_span, - }; - /// Returns true if the constraint does anything. If it doesn't, /// because it neither sizes nor positions the glyph, then this /// returns false. @@ -253,13 +221,10 @@ pub const RenderOptions = struct { ) Glyph.Size { // For extra wide font faces, never stretch glyphs across two cells. // This mirrors font_patcher. - const min_constraint_width: u2 = switch (self.target) { - .cell_span => constraint_width, - .face => if ((self.size == .stretch) and (metrics.face_width > 0.9 * metrics.face_height)) - 1 - else - @min(self.max_constraint_width, constraint_width), - }; + const min_constraint_width: u2 = if ((self.size == .stretch) and (metrics.face_width > 0.9 * metrics.face_height)) + 1 + else + @min(self.max_constraint_width, constraint_width); // The bounding box for the glyph's scale group. // Scaling and alignment rules are calculated for @@ -315,9 +280,24 @@ pub const RenderOptions = struct { return .{ 1.0, 1.0 }; } - const target_width, const target_height = self.targetSize(metrics, min_constraint_width); const multi_cell = (min_constraint_width > 1); + const pad_width_factor = @as(f64, @floatFromInt(min_constraint_width)) - (self.pad_left + self.pad_right); + const pad_height_factor = 1 - (self.pad_bottom + self.pad_top); + + const target_width = pad_width_factor * metrics.face_width; + const target_height = pad_height_factor * switch (self.height) { + .cell => metrics.face_height, + // Like font-patcher, the icon constraint height depends on the + // constraint width. Unlike font-patcher, the multi-cell + // icon_height may be different from face_height due to the + // `adjust-icon-height` config option. + .icon => if (multi_cell) + metrics.icon_height + else + metrics.icon_height_single, + }; + var width_factor = target_width / group.width; var height_factor = target_height / group.height; @@ -356,21 +336,6 @@ pub const RenderOptions = struct { width_factor = height_factor; }, .stretch => {}, - .height => { - width_factor = height_factor; - }, - .width => { - height_factor = width_factor; - }, - .contain => { - height_factor = @min(width_factor, height_factor); - width_factor = height_factor; - }, - .cover_bounds => { - height_factor = @max(width_factor, height_factor); - width_factor = height_factor; - }, - .stretch_bounds => {}, } // Reduce aspect ratio if required @@ -394,23 +359,15 @@ pub const RenderOptions = struct { // we don't touch vertical alignment. return group.y; } - const span_height: f64 = switch (self.target) { - .face => metrics.face_height, - .cell_span => @floatFromInt(metrics.cell_height), - }; - const origin_y: f64 = switch (self.target) { - // We use face_height and offset by face_y, rather than using - // cell_height directly, to account for the asymmetry of the - // pixel cell around the face (a consequence of aligning the - // baseline with a pixel boundary rather than vertically - // centering the face). - .face => metrics.face_y, - .cell_span => 0, - }; - const pad_bottom_dy = self.pad_bottom * span_height; - const pad_top_dy = self.pad_top * span_height; - const start_y = origin_y + pad_bottom_dy; - const end_y = origin_y + (span_height - group.height - pad_top_dy); + // We use face_height and offset by face_y, rather than + // using cell_height directly, to account for the asymmetry + // of the pixel cell around the face (a consequence of + // aligning the baseline with a pixel boundary rather than + // vertically centering the face). + const pad_bottom_dy = self.pad_bottom * metrics.face_height; + const pad_top_dy = self.pad_top * metrics.face_height; + const start_y = metrics.face_y + pad_bottom_dy; + const end_y = metrics.face_y + (metrics.face_height - group.height - pad_top_dy); const center_y = (start_y + end_y) / 2; return switch (self.align_vertical) { // NOTE: Even if there is no prescribed alignment, we ensure @@ -426,7 +383,6 @@ pub const RenderOptions = struct { .start => start_y, .end => end_y, .center, .center1 => center_y, - .baseline => @floatFromInt(metrics.cell_baseline), }; } @@ -442,27 +398,18 @@ pub const RenderOptions = struct { // axis, we don't touch horizontal alignment. return group.x; } - const span_width, const pad_width = switch (self.target) { - // For multi-cell constraints, we align relative to the span - // from the left edge of the first cell to the right edge of - // the last face cell assuming it's left-aligned within the - // rounded and adjusted pixel cell. Any horizontal offset to - // center the face within the grid cell is the responsibility - // of the backend-specific rendering code, and should be done - // after applying constraints. - .face => .{ - metrics.face_width + @as(f64, @floatFromInt((min_constraint_width - 1) * metrics.cell_width)), - metrics.face_width, - }, - .cell_span => .{ - @as(f64, @floatFromInt(min_constraint_width * metrics.cell_width)), - @as(f64, @floatFromInt(min_constraint_width * metrics.cell_width)), - }, - }; - const pad_left_dx = self.pad_left * pad_width; - const pad_right_dx = self.pad_right * pad_width; + // For multi-cell constraints, we align relative to the span + // from the left edge of the first cell to the right edge of + // the last face cell assuming it's left-aligned within the + // rounded and adjusted pixel cell. Any horizontal offset to + // center the face within the grid cell is the responsibility + // of the backend-specific rendering code, and should be done + // after applying constraints. + const full_face_span = metrics.face_width + @as(f64, @floatFromInt((min_constraint_width - 1) * metrics.cell_width)); + const pad_left_dx = self.pad_left * metrics.face_width; + const pad_right_dx = self.pad_right * metrics.face_width; const start_x = pad_left_dx; - const end_x = span_width - group.width - pad_right_dx; + const end_x = full_face_span - group.width - pad_right_dx; return switch (self.align_horizontal) { // NOTE: Even if there is no prescribed alignment, we ensure // that the glyph doesn't protrude outside the padded cell, @@ -482,66 +429,6 @@ pub const RenderOptions = struct { const end1_x = metrics.face_width - group.width - pad_right_dx; break :center1 @max(start_x, (start_x + end1_x) / 2); }, - // Baseline is vertical-only. We share the Align enum between - // axes so registered-glyph options can be stored in the same - // Constraint struct, but the parser never maps a horizontal - // alignment to .baseline. - .baseline => unreachable, - }; - } - - /// Return the available target width and height after padding. - /// - /// Face-targeted constraints preserve the historical behavior used by - /// platform fonts and Nerd Font rules: horizontal padding is measured - /// against one face width, and the available span is one face width - /// plus any additional grid cells. Vertical sizing uses either the - /// face height or configured icon height. - /// - /// Cell-span constraints are for glyphs whose layout contract is the - /// terminal cells themselves. Padding is measured against the full - /// cell span, so a two-cell glyph with 10% left padding starts after - /// 10% of both cells combined, not 10% of a single face width. - fn targetSize( - self: Constraint, - metrics: Metrics, - min_constraint_width: u2, - ) struct { f64, f64 } { - const multi_cell = (min_constraint_width > 1); - - return switch (self.target) { - .face => .{ - // Historical font constraints measure horizontal padding - // in face-width units. Additional cells add raw grid-cell - // width to the available span, matching aligned_x. - (@as(f64, @floatFromInt(min_constraint_width)) - (self.pad_left + self.pad_right)) * metrics.face_width, - // Vertical face constraints operate on the selected face - // or icon height. Icon height may depend on whether the - // constraint spans multiple cells. - (1 - (self.pad_bottom + self.pad_top)) * switch (self.height) { - .cell => metrics.face_height, - // Like font-patcher, the icon constraint height depends on the - // constraint width. Unlike font-patcher, the multi-cell - // icon_height may be different from face_height due to the - // `adjust-icon-height` config option. - .icon => if (multi_cell) - metrics.icon_height - else - metrics.icon_height_single, - }, - }, - .cell_span => span: { - const span_width: f64 = @floatFromInt(min_constraint_width * metrics.cell_width); - const span_height: f64 = @floatFromInt(metrics.cell_height); - break :span .{ - // Cell-span constraints use the full terminal span as - // the target box. This is the exact contract needed by - // runtime registered glyphs, whose width is declared in - // terminal cells rather than face metrics. - (1 - (self.pad_left + self.pad_right)) * span_width, - (1 - (self.pad_bottom + self.pad_top)) * span_height, - }; - }, }; } }; @@ -657,79 +544,6 @@ test "Constraints" { ); } - // Cell-span constraints. These are generic target-bound sizing modes used - // by registered glyphs where the render span is defined by terminal cell - // dimensions rather than primary face metrics. - { - const glyph: GlyphSize = .{ - .width = 100, - .height = 200, - .x = 0, - .y = 0, - }; - - const base: RenderOptions.Constraint = .{ - .target = .cell_span, - .align_horizontal = .start, - .align_vertical = .start, - }; - - // Target span is two cells wide by one cell high: 20px x 22px. - var height_constraint = base; - height_constraint.size = .height; - try comparison.expectApproxEqual( - GlyphSize{ .width = 11, .height = 22, .x = 0, .y = 0 }, - height_constraint.constrain(glyph, metrics, 2), - ); - var width_constraint = base; - width_constraint.size = .width; - try comparison.expectApproxEqual( - GlyphSize{ .width = 20, .height = 40, .x = 0, .y = 0 }, - width_constraint.constrain(glyph, metrics, 2), - ); - var contain_constraint = base; - contain_constraint.size = .contain; - try comparison.expectApproxEqual( - GlyphSize{ .width = 11, .height = 22, .x = 0, .y = 0 }, - contain_constraint.constrain(glyph, metrics, 2), - ); - var cover_constraint = base; - cover_constraint.size = .cover_bounds; - try comparison.expectApproxEqual( - GlyphSize{ .width = 20, .height = 40, .x = 0, .y = 0 }, - cover_constraint.constrain(glyph, metrics, 2), - ); - var stretch_constraint = base; - stretch_constraint.size = .stretch_bounds; - try comparison.expectApproxEqual( - GlyphSize{ .width = 20, .height = 22, .x = 0, .y = 0 }, - stretch_constraint.constrain(glyph, metrics, 2), - ); - - // Baseline alignment places the group's y=0 at the terminal text - // baseline, independent of vertical padding. - try comparison.expectApproxEqual( - GlyphSize{ .width = 100, .height = 200, .x = 0, .y = 5 }, - (RenderOptions.Constraint{ - .target = .cell_span, - .align_vertical = .baseline, - }).constrain(glyph, metrics, 2), - ); - - // Cell-span padding is relative to the full two-cell span. - try comparison.expectApproxEqual( - GlyphSize{ .width = 10, .height = 20, .x = 5, .y = 0 }, - (RenderOptions.Constraint{ - .target = .cell_span, - .size = .width, - .align_horizontal = .start, - .align_vertical = .start, - .pad_left = 0.25, - .pad_right = 0.25, - }).constrain(glyph, metrics, 2), - ); - } - // Nerd Font default. { const constraint = getConstraint(0xea61).?; diff --git a/src/font/glyf_rasterize.zig b/src/font/glyf_rasterize.zig index 04bd08ee7..f321214bc 100644 --- a/src/font/glyf_rasterize.zig +++ b/src/font/glyf_rasterize.zig @@ -231,12 +231,11 @@ const Placement = struct { design: DesignMetrics, opts: Glyph.RenderOptions, ) Placement { - // Start with design units mapped so that the em square occupies one - // cell. This normalizes coordinates into the same cell-relative pixel - // space used by RenderOptions.Constraint. Protocol sizing constraints - // then rescale the declared advance/line-height box so the final - // transform follows the protocol formulas based on advance width and - // line height rather than units-per-em. + // Start with protocol-like design units mapped so that the em square + // occupies one cell. This makes units_per_em the scale reference and + // preserves the linked rasterizer's y=0 baseline/bottom behavior. + // Callers can then use RenderOptions.Constraint to fit/cover/stretch/ + // align the declared advance/line-height box using existing font logic. const scale = @as(f64, @floatFromInt(opts.grid_metrics.cell_height)) / @as(f64, @floatFromInt(design.units_per_em)); @@ -824,153 +823,3 @@ test "glyf_rasterize: line height does not change unconstrained em scale" { try testing.expect(bm.data[2 * bm.width + 10] > 200); try testing.expect(bm.data[17 * bm.width + 10] > 200); } - -test "glyf_rasterize: cell-span height sizing uses line height" { - const testing = std.testing; - - const placement = Placement.init(.{ - .x_min = 0, - .y_min = 0, - .x_max = 1000, - .y_max = 1000, - }, .{ - .units_per_em = 1000, - .advance_width = 1000, - .line_height = 2000, - }, .{ - .grid_metrics = testMetrics(20, 20), - .constraint = .{ - .target = .cell_span, - .size = .height, - .align_horizontal = .start, - .align_vertical = .start, - }, - }); - - // Final scale is cell_height / lh = 20 / 2000 = 0.01, so this - // 1000-unit square is 10px high rather than a full 20px em. - try testing.expectApproxEqAbs(@as(f64, 10), placement.width, 0.000001); - try testing.expectApproxEqAbs(@as(f64, 10), placement.height, 0.000001); -} - -test "glyf_rasterize: cell-span advance sizing uses registered width" { - const testing = std.testing; - - const placement = Placement.init(.{ - .x_min = 0, - .y_min = 0, - .x_max = 1000, - .y_max = 1000, - }, .{ - .units_per_em = 1000, - .advance_width = 1000, - .line_height = 1000, - }, .{ - .grid_metrics = testMetrics(20, 20), - .constraint_width = 2, - .constraint = .{ - .target = .cell_span, - .size = .width, - .align_horizontal = .start, - .align_vertical = .start, - }, - }); - - // Final scale is span_width / aw = 40 / 1000 = 0.04. - try testing.expectApproxEqAbs(@as(f64, 40), placement.width, 0.000001); - try testing.expectApproxEqAbs(@as(f64, 40), placement.height, 0.000001); -} - -test "glyf_rasterize: cell-span contain and cover choose different axes" { - const testing = std.testing; - - const bounds: Bounds = .{ - .x_min = 0, - .y_min = 0, - .x_max = 1000, - .y_max = 1000, - }; - const design: DesignMetrics = .{ - .units_per_em = 1000, - .advance_width = 1000, - .line_height = 2000, - }; - const base_opts: Glyph.RenderOptions = .{ - .grid_metrics = testMetrics(20, 20), - .constraint = .{ - .target = .cell_span, - .align_horizontal = .start, - .align_vertical = .start, - }, - }; - - var contain_opts = base_opts; - contain_opts.constraint.size = .contain; - const contain = Placement.init(bounds, design, contain_opts); - - var cover_opts = base_opts; - cover_opts.constraint.size = .cover_bounds; - const cover = Placement.init(bounds, design, cover_opts); - - try testing.expectApproxEqAbs(@as(f64, 10), contain.width, 0.000001); - try testing.expectApproxEqAbs(@as(f64, 10), contain.height, 0.000001); - try testing.expectApproxEqAbs(@as(f64, 20), cover.width, 0.000001); - try testing.expectApproxEqAbs(@as(f64, 20), cover.height, 0.000001); -} - -test "glyf_rasterize: cell-span stretch uses independent axes" { - const testing = std.testing; - - const placement = Placement.init(.{ - .x_min = 0, - .y_min = 0, - .x_max = 1000, - .y_max = 2000, - }, .{ - .units_per_em = 1000, - .advance_width = 1000, - .line_height = 2000, - }, .{ - .grid_metrics = testMetrics(20, 20), - .constraint_width = 2, - .constraint = .{ - .target = .cell_span, - .size = .stretch_bounds, - .align_horizontal = .start, - .align_vertical = .start, - }, - }); - - try testing.expectApproxEqAbs(@as(f64, 40), placement.width, 0.000001); - try testing.expectApproxEqAbs(@as(f64, 20), placement.height, 0.000001); -} - -test "glyf_rasterize: cell-span baseline aligns design y zero" { - const testing = std.testing; - - var metrics = testMetrics(20, 20); - metrics.cell_baseline = 5; - const bounds: Bounds = .{ - .x_min = 0, - .y_min = -250, - .x_max = 1000, - .y_max = 750, - }; - const placement = Placement.init(bounds, .{ - .units_per_em = 1000, - .advance_width = 1000, - .line_height = 1000, - }, .{ - .grid_metrics = metrics, - .constraint = .{ - .target = .cell_span, - .size = .height, - .align_horizontal = .start, - .align_vertical = .baseline, - }, - }); - - const scale_y = placement.height / bounds.height(); - const design_zero_y = placement.y + ((0 - bounds.y_min) * scale_y); - try testing.expectApproxEqAbs(@as(f64, 5), design_zero_y, 0.000001); -} diff --git a/src/terminal/apc/glyph.zig b/src/terminal/apc/glyph.zig index 02eb26525..727e1ab4d 100644 --- a/src/terminal/apc/glyph.zig +++ b/src/terminal/apc/glyph.zig @@ -96,16 +96,9 @@ //! - `width` — Unicode/wcwidth cell width. Must be `1` or `2`; default `1`. //! This is authoritative for cursor advance, wrapping, and //! selection geometry. -//! - `size` — scale policy. Default `height`. Given the padded render span -//! width `W'`, padded render span height `H'`, authored advance -//! width `aw`, and authored line height `lh`, scale is: -//! `height = H' / lh`, `advance = W' / aw`, -//! `contain = min(W'/aw, H'/lh)`, -//! `cover = max(W'/aw, H'/lh)`, and -//! `stretch = (W'/aw, H'/lh)` independently on each axis. +//! - `size` — scale policy. Default `height`. //! - `align` — horizontal and vertical placement within the render span. -//! Default `center,center`. Vertical `baseline` aligns -//! design-space `y=0` to the terminal text baseline. +//! Default `center,center`. //! - `pad` — fractional insets from the render span edges. Default //! `0,0,0,0`; degenerate padding is treated as no padding. //! - payload — base64-encoded payload for the selected `fmt`. diff --git a/src/terminal/apc/glyph/Glossary.zig b/src/terminal/apc/glyph/Glossary.zig index 5bad341dd..28458dbb2 100644 --- a/src/terminal/apc/glyph/Glossary.zig +++ b/src/terminal/apc/glyph/Glossary.zig @@ -209,13 +209,23 @@ pub const Entry = struct { const pad = req.get(.pad) orelse return error.InvalidOptions; return .{ - .target = .cell_span, .size = switch (size) { - .height => .height, - .advance => .width, - .contain => .contain, - .cover => .cover_bounds, - .stretch => .stretch_bounds, + // The rasterizer's base transform already maps the design em + // to the cell height. That is the closest existing behavior to + // the protocol's default height-driven mode. + .height => .none, + // There is no width-driven, aspect-preserving constraint mode + // today. Leave the base transform intact rather than forcing a + // fit/contain policy that would unexpectedly prevent overflow. + .advance => .none, + // Constraint.cover currently scales preserving aspect ratio to + // the available bounds, which is the best existing match for + // the protocol's contain mode. + .contain => .cover, + // There is no true protocol-cover equivalent that chooses the + // larger axis scale, so use the nearest named renderer policy. + .cover => .cover, + .stretch => .stretch, }, .align_horizontal = switch (alignment.horizontal) { .start => .start, @@ -226,7 +236,11 @@ pub const Entry = struct { .start => .start, .center => .center, .end => .end, - .baseline => .baseline, + // The current constraint API has no baseline alignment mode. + // Start is the closest stable default because the glyf + // rasterizer's coordinate model already treats y=0 as the + // baseline/bottom before constraints are applied. + .baseline => .start, }, .pad_top = pad.top, .pad_right = pad.right, @@ -294,8 +308,7 @@ test "Entry init decodes glyf payload and applies register fields" { try testing.expectEqual(@as(u32, 1024), entry.design.advance_width); try testing.expectEqual(@as(u32, 1536), entry.design.line_height); try testing.expectEqual(request.Width.wide, entry.width); - try testing.expectEqual(Constraint.Target.cell_span, entry.constraint.target); - try testing.expectEqual(Constraint.Size.stretch_bounds, entry.constraint.size); + try testing.expectEqual(Constraint.Size.stretch, entry.constraint.size); try testing.expectEqual(Constraint.Align.end, entry.constraint.align_horizontal); try testing.expectEqual(Constraint.Align.start, entry.constraint.align_vertical); try testing.expectEqual(@as(f64, 0.1), entry.constraint.pad_top); @@ -307,37 +320,6 @@ test "Entry init decodes glyf payload and applies register fields" { try testing.expectEqual(@as(usize, 1), entry.glyph.glyf.contours.len); } -test "Entry constraintFromRegister maps sizing and baseline exactly" { - const testing = std.testing; - const alloc = testing.allocator; - - const cases = .{ - .{ "height", Constraint.Size.height }, - .{ "advance", Constraint.Size.width }, - .{ "contain", Constraint.Size.contain }, - .{ "cover", Constraint.Size.cover_bounds }, - .{ "stretch", Constraint.Size.stretch_bounds }, - }; - - inline for (cases) |case| { - const data = try std.fmt.allocPrint( - alloc, - "r;cp=e000;size={s};align=center,baseline;{s}", - .{ case[0], test_triangle_glyf_payload }, - ); - defer alloc.free(data); - - const req = try testParseRegister(alloc, data); - defer alloc.free(req.raw); - - const constraint = try Entry.constraintFromRegister(req); - try testing.expectEqual(Constraint.Target.cell_span, constraint.target); - try testing.expectEqual(case[1], constraint.size); - try testing.expectEqual(Constraint.Align.center, constraint.align_horizontal); - try testing.expectEqual(Constraint.Align.baseline, constraint.align_vertical); - } -} - test "Entry init rejects invalid register payload" { const testing = std.testing; const alloc = testing.allocator;