Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions src/internal/render/gridpainter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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)
}
}
}
}
Expand Down
5 changes: 4 additions & 1 deletion src/internal/render/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
106 changes: 106 additions & 0 deletions src/test/unit/gridalign_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
Loading