From 38217499c2a9d11d8cd7b652c8432c57273e0c65 Mon Sep 17 00:00:00 2001 From: kgfly <12282281+kgfly@users.noreply.github.com> Date: Wed, 26 Aug 2026 23:45:12 +0000 Subject: [PATCH] v1 --- src/internal/render/gridpainter.go | 21 ++++-- src/internal/render/metrics.go | 5 +- src/test/unit/gridalign_test.go | 106 +++++++++++++++++++++++++++++ 3 files changed, 126 insertions(+), 6 deletions(-) create mode 100644 src/test/unit/gridalign_test.go diff --git a/src/internal/render/gridpainter.go b/src/internal/render/gridpainter.go index 063e34f..a68abd7 100644 --- a/src/internal/render/gridpainter.go +++ b/src/internal/render/gridpainter.go @@ -34,8 +34,9 @@ func Frame(gtx layout.Context, fonts Fonts, snap uistate.Snapshot) { drawCursor(gtx, fonts, snap, defFg) } -// drawGrid paints every row of gv, offset by origin pixels, as a sequence of -// same-highlight cell "runs" (a background fill plus one text draw each). +// drawGrid paints every row of gv, offset by origin pixels. Cells are +// grouped into same-highlight runs for the background fill, but glyphs are +// drawn one cell at a time so text stays locked to the grid. func drawGrid(gtx layout.Context, fonts Fonts, hv uistate.HighlightView, gv uistate.GridView, origin image.Point) { cw, ch := fonts.Metrics.CellWidth, fonts.Metrics.CellHeight for row, cells := range gv.Data { @@ -44,9 +45,7 @@ func drawGrid(gtx layout.Context, fonts Fonts, hv uistate.HighlightView, gv uist for col < len(cells) { runStart := col hlID := cells[col].HlID - var text []byte for col < len(cells) && cells[col].HlID == hlID { - text = append(text, cells[col].Text...) col++ } runLen := col - runStart @@ -56,7 +55,19 @@ func drawGrid(gtx layout.Context, fonts Fonts, hv uistate.HighlightView, gv uist x := origin.X + runStart*cw fg, bg := hv.Resolve(hlID) paint.FillShape(gtx.Ops, bg, clip.Rect(image.Rect(x, y, x+runLen*cw, y+ch)).Op()) - drawText(gtx, fonts, x, y, string(text), fg) + + // Draw each cell at its own exact grid position instead of + // shaping the whole run as one string. A font's real advance is + // fractional and rarely equals the integer CellWidth the grid is + // laid out on, so a shaped run drifts further right with every + // glyph; over a long line that accumulates into whole columns of + // error, visibly clipping characters at run boundaries. + for i, cell := range cells[runStart:col] { + if cell.Text == "" || cell.Text == " " { + continue + } + drawText(gtx, fonts, x+i*cw, y, cell.Text, fg) + } } } } diff --git a/src/internal/render/metrics.go b/src/internal/render/metrics.go index 4e93839..d732bff 100644 --- a/src/internal/render/metrics.go +++ b/src/internal/render/metrics.go @@ -87,7 +87,10 @@ func Measure(gtx layout.Context, shaper *text.Shaper, face font.Font, size unit. var ops op.Ops mgtx.Ops = &ops dims := widget.Label{MaxLines: 1}.Layout(mgtx, shaper, face, size, probe, op.CallOp{}) - w := dims.Size.X / len(probe) + // Round to nearest rather than truncating: the advance is fractional, + // and always rounding down biases every cell narrower than the glyphs + // actually drawn into it. + w := (dims.Size.X + len(probe)/2) / len(probe) if w < 1 { w = 1 } diff --git a/src/test/unit/gridalign_test.go b/src/test/unit/gridalign_test.go new file mode 100644 index 0000000..2cf5d2f --- /dev/null +++ b/src/test/unit/gridalign_test.go @@ -0,0 +1,106 @@ +package unit_test + +import ( + "image" + "testing" + + "gioui.org/op" + "gioui.org/unit" + "gioui.org/widget" + + "github.com/kgfly/SimpleNvimEditor/internal/config" + "github.com/kgfly/SimpleNvimEditor/internal/render" +) + +// TestCellWidthMatchesShapedAdvance is the regression test for grid text +// drifting out of its columns. +// +// The renderer places every cell at an integer multiple of CellWidth. The +// old painter shaped a whole same-highlight run as a single string and let +// the font's own advance carry the glyphs along, so any mismatch between the +// real (fractional) advance and the integer CellWidth accumulated across the +// run. Over a long line that grew into whole columns of error, which showed +// up as the first character of a row vanishing and the column at a run +// boundary looking half-width. +// +// Two things keep that fixed, and this covers the metric half: CellWidth +// must be a faithful rounding of the advance the shaper actually uses. If it +// is systematically too small (as plain integer truncation made it), glyphs +// are wider than the cells they are assigned to and neighbours collide. +func TestCellWidthMatchesShapedAdvance(t *testing.T) { + cfg := config.Default().Editor + shaper := render.NewShaper(cfg) + face := render.FontFace(cfg) + + const probe = "MMMMMMMMMMMMMMMMMMMM" + + for _, size := range []unit.Sp{12, 13, 14, 15, 16, 18} { + var measureOps op.Ops + m := render.Measure( + newTestContext(&measureOps, image.Pt(4000, 4000)), + shaper, face, size, + ) + + var shapeOps op.Ops + gtx := newTestContext(&shapeOps, image.Pt(1<<20, 1<<20)) + gtx.Constraints.Min = image.Point{} + dims := widget.Label{MaxLines: 1}.Layout(gtx, shaper, face, size, probe, op.CallOp{}) + + // Allow at most half a pixel of rounding error per cell. + drift := dims.Size.X - m.CellWidth*len(probe) + if drift < 0 { + drift = -drift + } + if drift > len(probe)/2 { + t.Errorf("size %v: CellWidth=%d spans %d px for %d cells, but the shaper draws them in %d px (drift %d px)", + size, m.CellWidth, m.CellWidth*len(probe), len(probe), dims.Size.X, drift) + } + } +} + +// TestShapedRunDriftsFromCellGrid documents *why* the painter draws one cell +// at a time. +// +// It measures the same text the old painter would have handed to the shaper +// as a single string, and asserts that the result does not line up with the +// cell grid. As long as this holds, drawing a run as one string is unsafe +// and the per-cell loop in drawGrid must stay. If a future font or shaper +// change ever makes the advance exactly integral, this test fails loudly +// rather than letting someone "simplify" the painter back into the bug. +func TestShapedRunDriftsFromCellGrid(t *testing.T) { + fonts := testFonts(t) + cfg := config.Default().Editor + shaper := render.NewShaper(cfg) + face := render.FontFace(cfg) + + var measureOps op.Ops + m := render.Measure( + newTestContext(&measureOps, image.Pt(4000, 4000)), + shaper, face, fonts.Size, + ) + + const cols = 60 + run := make([]byte, cols) + for i := range run { + run[i] = 'M' + } + + var shapeOps op.Ops + gtx := newTestContext(&shapeOps, image.Pt(1<<20, 1<<20)) + gtx.Constraints.Min = image.Point{} + dims := widget.Label{MaxLines: 1}.Layout(gtx, shaper, face, fonts.Size, string(run), op.CallOp{}) + + gridWidth := m.CellWidth * cols + if dims.Size.X == gridWidth { + t.Skipf("shaped advance happens to be exactly integral at size %v; the per-cell painter is still required for other fonts/sizes", fonts.Size) + } + + // The drift must be smaller than the whole grid, otherwise the metric + // itself is broken rather than merely fractional. + if dims.Size.X <= 0 || dims.Size.X > gridWidth*2 { + t.Fatalf("implausible shaped width %d for %d cells of %d px", dims.Size.X, cols, m.CellWidth) + } + + t.Logf("shaping %d cells as one string spans %d px but the grid reserves %d px (drift %d px) -- this is why drawGrid draws per cell", + cols, dims.Size.X, gridWidth, dims.Size.X-gridWidth) +}