Skip to content

fix(textfield): place the caret from the shaped glyph run - #236

Open
samyfodil wants to merge 2 commits into
gogpu:mainfrom
samyfodil:fix/textfield-caret-shaped-glyphs
Open

samyfodil wants to merge 2 commits into
gogpu:mainfrom
samyfodil:fix/textfield-caret-shaped-glyphs

Conversation

@samyfodil

Copy link
Copy Markdown
Contributor

The defect

core/textfield places the caret about a pixel — sometimes three — to the right of where the character actually begins, so it overlaps the glyph instead of sitting in the gap before it. It looks intermittent because the size depends on which letters sit either side of the caret.

internal/textmetrics.Metrics measures the text before the caret as a string on its own:

x := baseX + m.Canvas.MeasureText(string(runes[:runePos]), m.FontSize, false)

The renderer never lays text out that way. scene.Scene.DrawText (the RepaintBoundary recorder, and therefore the compositor) shapes the whole string with text.Shape and encodes every glyph at its shaped X; the GPU glyph-mask and MSDF paths that gg.Context.DrawString selects do the same. Measuring a prefix on its own loses two things:

  1. The kerning pair spanning the cut. To, AV, Ya kern negative: in context the second glyph is pulled left and the caret does not follow.
  2. The advance source. Canvas.MeasureText → text.Measure → Face.Advance, which sums grid-fitted per-glyph advances and applies no GPOS at all. Shaping uses the font's own advances plus GPOS. The two disagree even on a string with no kerning pairs, and the error accumulates along the string.

Hit-testing has the same root cause with the opposite sign: RuneIndexFromX compares the click against prefix widths the renderer never used, so a click on the exact start of a character can resolve to the character before it.

Measured

Inter Regular (the embedded default) at 14px, caret position against the pen position of the same cluster in the shaped run:

sample worst caret error at index worst kerning term alone
AV Wave 3.366 px 6 0.957 px
Vy Ya P. 3.310 px 7 1.025 px
common sense 2.898 px 12 0.000 px
Yo, Tavi. 2.670 px 7 1.073 px
To The Top 1.399 px 10 1.094 px

common sense is the control: it has no kerning pairs at all and still drifts 2.9 px, which is term 2 on its own.

Standalone repro

No toolkit needed — this is gg/text against any TTF:

package main

import (
	"fmt"
	"os"

	"github.com/gogpu/gg/text"
)

func main() {
	data, _ := os.ReadFile(os.Args[1])
	src, _ := text.NewFontSource(data)
	face := src.Face(14)

	for _, s := range []string{"To The Top", "AV Wave", "common sense"} {
		runes := []rune(s)
		glyphs := text.Shape(s, face) // what the renderer positions from
		var pen, worst float64
		var at, g int
		for i := 1; i <= len(runes); i++ {
			for g < len(glyphs) && glyphs[g].Cluster < i {
				pen += glyphs[g].XAdvance
				g++
			}
			measured, _ := text.Measure(string(runes[:i]), face) // what the caret used
			if d := measured - pen; d > worst || -d > worst {
				worst, at = max(d, -d), i
			}
		}
		fmt.Printf("%-14q caret is %.3f px off the glyph at index %d\n", s, worst, at)
	}
}
$ go run . /usr/share/fonts/truetype/dejavu/DejaVuSans.ttf
"To The Top"   caret is 6.698 px off the glyph at index 10
"AV Wave"      caret is 2.866 px off the glyph at index 7
"common sense" caret is 2.023 px off the glyph at index 6

The fix

text.ShapedGlyph.Cluster is the rune index the glyph came from, and its own doc comment says what it is for: "Used for hit testing and cursor positioning." Nothing used it. The caret before rune i is the sum of XAdvance over the glyphs of the whole shaped string whose Cluster is below i.

The textfield cannot reach shaping: widget.Canvas exposes only MeasureText(text string, fontSize float32, bold bool) float32. So this adds an optional interface in the same shape as the ones already there (ArcStroker, SVGFiller, TextModeController, StyledTextDrawer):

type TextCaretPositioner interface {
	// CaretOffsets returns the horizontal offset, in logical pixels from the
	// text origin, of the caret placed before each rune of text, followed by
	// one final entry for the caret after the last rune.
	CaretOffsets(text string, fontSize float32, bold bool) []float32
}

One method rather than a caret-offset and a hit-test method, because one shaping answers both and RuneIndexFromX then scans a slice instead of re-measuring.

  • internal/render.Canvas and internal/render.SceneCanvas implement it through one shared helper; they resolve the same default font, so they cannot drift apart (there is a test for exactly that).
  • internal/textmetrics.Metrics type-asserts for it and falls back to the existing prefix measurement when the canvas does not implement it, so uitest.MockCanvas and every downstream widget.Canvas keep working unchanged. I checked every implementor in the tree first: internal/render.Canvas, internal/render.SceneCanvas, uitest.MockCanvas, and a hand-written canvas in eight test packages. Adding a method to Canvas itself would have broken all of them plus every downstream implementor, which docs/VERSIONING.md does not allow.
  • CursorX, CursorRect, SelectionRect, RuneIndexFromX and the TextField's horizontal scroll clamp all read the same offsets, so the caret, the selection highlight and the scroll cannot drift apart.
  • uitest.MockCanvas is deliberately not updated: it has no font, so it would only restate its own 0.5 * fontSize width formula, and the fallback already produces that exactly. A test asserts the mock stays on the fallback path.

It is also cheaper

One shaping per paint, memoized on Metrics, replaces a prefix measurement per cursor, per selection edge and per scroll clamp. RuneIndexFromX went from up to two MeasureText calls per character — each of which allocates a prefix string and walks it — to one shaping and a scan.

Tests

internal/textmetrics/shaping_test.go is the regression test and uses only pre-existing API, so it runs against main unchanged.

Before (on main, 4d78f0d, abridged — 49 failing assertions across the two tests):

--- FAIL: TestCursorXMatchesShapedGlyphPositions/AV_Wave
    CursorX("AV Wave", 1) = 20.0000, glyph begins at 18.7021 (off by 1.2979 px)
    CursorX("AV Wave", 4) = 48.0000, glyph begins at 45.3828 (off by 2.6172 px)
    CursorX("AV Wave", 6) = 64.0000, glyph begins at 60.6338 (off by 3.3662 px)
--- FAIL: TestCursorXMatchesShapedGlyphPositions/common_sense
    CursorX("common sense", 12) = 108.0000, glyph begins at 110.8984 (off by 2.8984 px)
--- FAIL: TestRuneIndexFromXMatchesShapedGlyphPositions/Yo,_Tavi.
    RuneIndexFromX("Yo, Tavi.", 61.7207) = 7, want 8
--- FAIL: TestRuneIndexFromXMatchesShapedGlyphPositions/Vy_Ya_P.
    RuneIndexFromX("Vy Ya P.", 51.7744) = 5, want 6
FAIL	github.com/gogpu/ui/internal/textmetrics	0.010s

After:

ok  	github.com/gogpu/ui/internal/textmetrics	0.012s
ok  	github.com/gogpu/ui/internal/render	0.021s
ok  	github.com/gogpu/ui/core/textfield	0.002s

TestSamplesDivergeFromAPrefixMeasurement guards the fixture rather than the code: if the default font ever stopped kerning these pairs, the two tests above would pass against a prefix measurement and prove nothing. It logs 5 of 5 samples diverge from a prefix measurement, worst 3.366 px.

The new gates were also run against a deliberate violation — an off-by-one in the cluster walk (Cluster <= i) — and all three packages go red, including an existing core/textfield scroll test.

Full suite, go test ./... -count=1: all 62 test packages pass (8 further packages have no test files). go build ./..., go build ./examples/..., go vet ./... and gofmt -l . are clean.

Two notes on the local checks, so they are not overstated:

  • go test ./... -race fails in app on TestWindow_AnimPumper_StartsOnInvalidation (mockWindowProvider.RequestRedraw vs the test goroutine). It reproduces identically on unmodified main, so it is pre-existing and unrelated; every other package is race-clean.
  • golangci-lint reports 0 issues here with staticcheck, gosec, nilerr and errorlint disabled. With those enabled it cannot run on unmodified main either on my machine — its IR builder (honnef.co/go/tools v0.7.0) panics on the Go 1.27 standard library. CI pins Go 1.25, where this does not arise.

The sub-pixel rounding term, and why it is not in here

There is a second, smaller disagreement: DrawText snaps the text origin with x = math.Round(x) after the canvas transform, while CursorX rounds contentRect.Min.X before the widget adds scrollOffsetX. #211 removed the unscrolled half of this; what is left is Round(a+s) - Round(a) - s, which is zero when the field is not scrolled and at most half a pixel when it is. There is also a third layer — the caret is drawn through DrawLine, which is transformed but not rounded, so widget-local rounding cannot match a rounding that happens after a fractional transform.

I left it out on purpose. It is a rounding-order question in how core/textfield applies its scroll offset and in where the canvas rounds, not a measurement error, and fixing it means changing which rect the caret is computed against — the current code comments that choice deliberately ("compute cursor/selection in UNSHIFTED content rect ... they never drift apart"). That deserves its own change with the scroll tests re-derived, rather than riding along with a fix that is exact for the term it addresses. Happy to follow up if you want it.

Observation, not a request

gg's CPU bitmap fallback (text.Draw → Face.Glyphs) positions glyphs from grid-fitted advances and applies no GPOS, so text rendered through it is itself un-kerned and disagrees with every GPU and scene path in gg. That is why the old prefix measurement matched that path. This change follows the shaped layout, which is what a desktop.Run application and the compositor actually draw. If the CPU fallback should shape too, that belongs in gg rather than here.

The caret and the hit test measured the text before them as a string on
its own, through Canvas.MeasureText. The renderer never lays text out
that way: scene.Scene.DrawText and the GPU glyph paths shape the whole
string and position every glyph from that one run. Measuring a prefix
alone loses two things — the kerning pair spanning the cut, and the
advance source itself, since text.Measure sums grid-fitted advances
where shaping uses the font's own plus GPOS.

Inter Regular at 14px: the caret sits up to 3.366px right of the glyph
it should precede ("AV Wave"), of which 1.094px is kerning ("To The
Top" at index 1). "common sense" has no kerning pairs at all and still
drifts 2.898px, from the advance source alone. Click-to-position
carried the same error with the opposite sign — a click on the exact
start of a character resolved to the character before it.

widget.TextCaretPositioner is a new optional Canvas interface, in the
shape of ArcStroker and StyledTextDrawer, returning the caret offset
before every rune of a string from ShapedGlyph.Cluster over the whole
shaped run — the use that field's own doc comment describes. Canvas and
SceneCanvas implement it; a canvas that does not keeps the previous
prefix measurement, so no implementor has to change and the interface
stays additive.

It is also less work than what it replaces: one shaping per paint,
memoized on textmetrics.Metrics, instead of a prefix measurement per
cursor, selection edge and scroll clamp — and one pass for a hit test
where RuneIndexFromX measured up to two prefixes per character.

CursorX, CursorRect, SelectionRect, RuneIndexFromX and the TextField's
horizontal scroll clamp all read the same offsets, so the caret, the
highlight and the scroll cannot drift apart.
@samyfodil
samyfodil requested a review from kolkov as a code owner September 23, 2026 01:54
@codecov

codecov Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.05882% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/render/caret.go 92.85% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant