widget: simplify and improve cursor position generation

This commit updates the strategy of our cursor positioning index to eliminate
cursor positions *after* trailing whitespace characters on a line. Eliminating
such cursor positions enables us to collapse trailing whitespace visually without
impacting the editability of text (this will be done in a future commit).

Signed-off-by: Chris Waldon <christopher.waldon.dev@gmail.com>

widget/editor_test.go

@@ -508,13 +508,14 @@ func TestEditorLigature(t *testing.T) {
 	// Ensure that all runes in the final cluster of a line are properly
 	// decoded when moving to the end of the line. This is a regression test.
 	e.text.MoveEnd(selectionClear)
-	// The first line was broken by line wrapping, not a newline character. As such,
+	// The first line was broken by line wrapping, not a newline character, and has a trailing
-	// the cursor can reach the position after the final glyph (a space).
+	// whitespace. However, we should never be able to reach the "other side" of such a trailing
-	assertCaret(t, e, 0, 14, len("fflffl fflffl "))
+	// whitespace glyph.
+	assertCaret(t, e, 0, 13, len("fflffl fflffl"))
 	e.text.MoveLines(1, selectionClear)
 	assertCaret(t, e, 1, 13, len("fflffl fflffl fflffl fflffl"))
 	e.text.MoveLines(-1, selectionClear)
-	assertCaret(t, e, 0, 14, len("fflffl fflffl "))
+	assertCaret(t, e, 0, 13, len("fflffl fflffl"))
 
 	// Absurdly narrow constraints to force each ligature onto its own line.
 	gtx.Constraints = layout.Exact(image.Pt(10, 10))

widget/index.go

@@ -44,12 +44,11 @@ type glyphIndex struct {
 	prog text.Flags
 	// clusterAdvance accumulates the advances of glyphs in a glyph cluster.
 	clusterAdvance fixed.Int26_6
-	// skipPrior controls whether a text position is inserted "before" the
-	// next glyph. Usually this should not happen, but the boundaries of
-	// lines and bidi runs require it.
-	skipPrior bool
 	// truncated indicates that the text was truncated by the shaper.
 	truncated bool
+	// midCluster tracks whether the next glyph processed is not the first glyph in a
+	// cluster.
+	midCluster bool
 }
 
 // reset prepares the index for reuse.
@@ -63,8 +62,8 @@ func (g *glyphIndex) reset() {
 	g.pos = combinedPos{}
 	g.prog = 0
 	g.clusterAdvance = 0
-	g.skipPrior = false
 	g.truncated = false
+	g.midCluster = false
 }
 
 // screenPos represents a character position in text line and column numbers,
@@ -113,6 +112,20 @@ func (g *glyphIndex) incrementPosition(pos combinedPos) (next combinedPos, eof b
 	return candidate, true
 }
 
+func (g *glyphIndex) insertPosition(pos combinedPos) {
+	lastIdx := len(g.positions) - 1
+	if lastIdx >= 0 {
+		lastPos := g.positions[lastIdx]
+		if lastPos.runes == pos.runes && (lastPos.y != pos.y || (lastPos.x == pos.x)) {
+			// If we insert a consecutive position with the same logical position,
+			// overwrite the previous position with the new one.
+			g.positions[lastIdx] = pos
+			return
+		}
+	}
+	g.positions = append(g.positions, pos)
+}
+
 // Glyph indexes the provided glyph, generating text cursor positions for it.
 func (g *glyphIndex) Glyph(gl text.Glyph) {
 	g.glyphs = append(g.glyphs, gl)
@@ -128,30 +141,32 @@ func (g *glyphIndex) Glyph(gl text.Glyph) {
 	if end := gl.X + gl.Advance; end > g.currentLineMax {
 		g.currentLineMax = end
 	}
-	if !g.skipPrior || gl.Flags&text.FlagTowardOrigin != g.prog || gl.Flags&text.FlagParagraphStart != 0 {
+
-		// Set the new text progression based on that of the first glyph.
-		g.prog = gl.Flags & text.FlagTowardOrigin
-		g.pos.towardOrigin = g.prog == text.FlagTowardOrigin
-		// Create the text position prior to the first glyph.
-		pos := g.pos
-		pos.x = gl.X
-		pos.y = int(gl.Y)
-		pos.ascent = gl.Ascent
-		pos.descent = gl.Descent
-		if pos.towardOrigin {
-			pos.x += gl.Advance
-		}
-		g.pos = pos
-		g.positions = append(g.positions, pos)
-		g.skipPrior = true
-	}
 	needsNewLine := gl.Flags&text.FlagLineBreak != 0
 	needsNewRun := gl.Flags&text.FlagRunBreak != 0
 	breaksParagraph := gl.Flags&text.FlagParagraphBreak != 0
-
+	breaksCluster := gl.Flags&text.FlagClusterBreak != 0
 	// We should insert new positions if the glyph we're processing terminates
-	// a glyph cluster.
+	// a glyph cluster, has nonzero runes, and is not a hard newline.
-	insertPositionAfter := gl.Flags&text.FlagClusterBreak != 0 && !breaksParagraph && gl.Runes > 0
+	insertPositionsWithin := breaksCluster && !breaksParagraph && gl.Runes > 0
+
+	// Get the text progression/direction right.
+	g.prog = gl.Flags & text.FlagTowardOrigin
+	g.pos.towardOrigin = g.prog == text.FlagTowardOrigin
+	if !g.midCluster {
+		// Create the text position prior to the glyph.
+		g.pos.x = gl.X
+		g.pos.y = int(gl.Y)
+		g.pos.ascent = gl.Ascent
+		g.pos.descent = gl.Descent
+		if g.pos.towardOrigin {
+			g.pos.x += gl.Advance
+		}
+		g.insertPosition(g.pos)
+	}
+
+	g.midCluster = !breaksCluster
+
 	if breaksParagraph {
 		// Paragraph breaking clusters shouldn't have positions generated for both
 		// sides of them. They're always zero-width, so doing so would
@@ -164,12 +179,11 @@ func (g *glyphIndex) Glyph(gl text.Glyph) {
 	// Always track the cumulative advance added by the glyph, even if it
 	// doesn't terminate a cluster itself.
 	g.clusterAdvance += gl.Advance
-	if insertPositionAfter {
+	if insertPositionsWithin {
-		// Construct the text position _after_ gl.
+		// Construct the text positions _within_ gl.
-		pos := g.pos
+		g.pos.y = int(gl.Y)
-		pos.y = int(gl.Y)
+		g.pos.ascent = gl.Ascent
-		pos.ascent = gl.Ascent
+		g.pos.descent = gl.Descent
-		pos.descent = gl.Descent
 		width := g.clusterAdvance
 		positionCount := int(gl.Runes)
 		runesPerPosition := 1
@@ -181,19 +195,18 @@ func (g *glyphIndex) Glyph(gl text.Glyph) {
 		}
 		perRune := width / fixed.Int26_6(positionCount)
 		adjust := fixed.Int26_6(0)
-		if pos.towardOrigin {
+		if g.pos.towardOrigin {
 			// If RTL, subtract increments from the width of the cluster
 			// instead of adding.
 			adjust = width
 			perRune = -perRune
 		}
 		for i := 1; i <= positionCount; i++ {
-			pos.x = gl.X + adjust + perRune*fixed.Int26_6(i)
+			g.pos.x = gl.X + adjust + perRune*fixed.Int26_6(i)
-			pos.runes += runesPerPosition
+			g.pos.runes += runesPerPosition
-			pos.lineCol.col += runesPerPosition
+			g.pos.lineCol.col += runesPerPosition
-			g.positions = append(g.positions, pos)
+			g.insertPosition(g.pos)
 		}
-		g.pos = pos
 		g.clusterAdvance = 0
 	}
 	if needsNewRun {
@@ -214,7 +227,6 @@ func (g *glyphIndex) Glyph(gl text.Glyph) {
 		g.currentLineMin = math.MaxInt32
 		g.currentLineMax = 0
 		g.currentLineGlyphs = 0
-		g.skipPrior = false
 	}
 }
 

widget/index_test.go

@@ -108,6 +108,7 @@ func getGlyphs(fontSize, minWidth, lineWidth int, align text.Alignment, str stri
 		MinWidth:   minWidth,
 		MaxWidth:   lineWidth,
 		Locale:     english,
+		WrapPolicy: text.WrapWords,
 	}
 	shaper.LayoutString(params, str)
 	for g, ok := shaper.NextGlyph(); ok; g, ok = shaper.NextGlyph() {
@@ -122,6 +123,7 @@ func TestIndexPositionWhitespace(t *testing.T) {
 	type testcase struct {
 		name      string
 		str       string
+		lineWidth int
 		align     text.Alignment
 		expected  []combinedPos
 	}
@@ -129,6 +131,7 @@ func TestIndexPositionWhitespace(t *testing.T) {
 		{
 			name:      "empty string",
 			str:       "",
+			lineWidth: 200,
 			expected: []combinedPos{
 				{x: fixed.Int26_6(0), y: 16, ascent: fixed.Int26_6(968), descent: fixed.Int26_6(216)},
 			},
@@ -136,6 +139,7 @@ func TestIndexPositionWhitespace(t *testing.T) {
 		{
 			name:      "just hard newline",
 			str:       "\n",
+			lineWidth: 200,
 			expected: []combinedPos{
 				{x: fixed.Int26_6(0), y: 16, ascent: fixed.Int26_6(968), descent: fixed.Int26_6(216)},
 				{x: fixed.Int26_6(0), y: 35, ascent: fixed.Int26_6(968), descent: fixed.Int26_6(216), runes: 1, lineCol: screenPos{line: 1}},
@@ -144,6 +148,7 @@ func TestIndexPositionWhitespace(t *testing.T) {
 		{
 			name:      "trailing newline",
 			str:       "a\n",
+			lineWidth: 200,
 			expected: []combinedPos{
 				{x: fixed.Int26_6(0), y: 16, ascent: fixed.Int26_6(968), descent: fixed.Int26_6(216)},
 				{x: fixed.Int26_6(570), y: 16, ascent: fixed.Int26_6(968), descent: fixed.Int26_6(216), runes: 1, lineCol: screenPos{col: 1}},
@@ -153,6 +158,7 @@ func TestIndexPositionWhitespace(t *testing.T) {
 		{
 			name:      "just blank line",
 			str:       "\n\n",
+			lineWidth: 200,
 			expected: []combinedPos{
 				{x: fixed.Int26_6(0), y: 16, ascent: fixed.Int26_6(968), descent: fixed.Int26_6(216)},
 				{x: fixed.Int26_6(0), y: 35, ascent: fixed.Int26_6(968), descent: fixed.Int26_6(216), runes: 1, lineCol: screenPos{line: 1}},
@@ -163,6 +169,7 @@ func TestIndexPositionWhitespace(t *testing.T) {
 			name:      "middle aligned blank lines",
 			str:       "\n\n\nabc",
 			align:     text.Middle,
+			lineWidth: 200,
 			expected: []combinedPos{
 				{x: fixed.Int26_6(832), y: 16, ascent: fixed.Int26_6(968), descent: fixed.Int26_6(216)},
 				{x: fixed.Int26_6(832), y: 35, ascent: fixed.Int26_6(968), descent: fixed.Int26_6(216), runes: 1, lineCol: screenPos{line: 1}},
@@ -176,6 +183,7 @@ func TestIndexPositionWhitespace(t *testing.T) {
 		{
 			name:      "blank line",
 			str:       "a\n\nb",
+			lineWidth: 200,
 			expected: []combinedPos{
 				{x: fixed.Int26_6(0), y: 16, ascent: fixed.Int26_6(968), descent: fixed.Int26_6(216)},
 				{x: fixed.Int26_6(570), y: 16, ascent: fixed.Int26_6(968), descent: fixed.Int26_6(216), runes: 1, lineCol: screenPos{col: 1}},
@@ -184,9 +192,45 @@ func TestIndexPositionWhitespace(t *testing.T) {
 				{x: fixed.Int26_6(570), y: 54, ascent: fixed.Int26_6(968), descent: fixed.Int26_6(216), runes: 4, lineCol: screenPos{line: 2, col: 1}},
 			},
 		},
+		{
+			name:      "soft wrap",
+			str:       "abc def",
+			lineWidth: 30,
+			expected: []combinedPos{
+				{runes: 0, lineCol: screenPos{line: 0, col: 0}, ascent: fixed.Int26_6(968), descent: fixed.Int26_6(216), x: 0, y: 16},
+				{runes: 1, lineCol: screenPos{line: 0, col: 1}, ascent: fixed.Int26_6(968), descent: fixed.Int26_6(216), x: 570, y: 16},
+				{runes: 2, lineCol: screenPos{line: 0, col: 2}, ascent: fixed.Int26_6(968), descent: fixed.Int26_6(216), x: 1140, y: 16},
+				{runes: 3, lineCol: screenPos{line: 0, col: 3}, ascent: fixed.Int26_6(968), descent: fixed.Int26_6(216), x: 1652, y: 16},
+				{runes: 4, lineCol: screenPos{line: 1, col: 0}, ascent: fixed.Int26_6(968), descent: fixed.Int26_6(216), x: 0, y: 35},
+				{runes: 5, lineCol: screenPos{line: 1, col: 1}, ascent: fixed.Int26_6(968), descent: fixed.Int26_6(216), x: 570, y: 35},
+				{runes: 6, lineCol: screenPos{line: 1, col: 2}, ascent: fixed.Int26_6(968), descent: fixed.Int26_6(216), x: 1140, y: 35},
+				{runes: 7, lineCol: screenPos{line: 1, col: 3}, ascent: fixed.Int26_6(968), descent: fixed.Int26_6(216), x: 1425, y: 35},
+			},
+		},
+		{
+			name:      "soft wrap arabic",
+			str:       "ثنائي الاتجاه",
+			lineWidth: 30,
+			expected: []combinedPos{
+				{runes: 0, lineCol: screenPos{line: 0, col: 0}, ascent: 1407, descent: 756, x: 2250, y: 22, towardOrigin: true},
+				{runes: 1, lineCol: screenPos{line: 0, col: 1}, ascent: 1407, descent: 756, x: 1944, y: 22, towardOrigin: true},
+				{runes: 2, lineCol: screenPos{line: 0, col: 2}, ascent: 1407, descent: 756, x: 1593, y: 22, towardOrigin: true},
+				{runes: 3, lineCol: screenPos{line: 0, col: 3}, ascent: 1407, descent: 756, x: 1295, y: 22, towardOrigin: true},
+				{runes: 4, lineCol: screenPos{line: 0, col: 4}, ascent: 1407, descent: 756, x: 1020, y: 22, towardOrigin: true},
+				{runes: 5, lineCol: screenPos{line: 0, col: 5}, ascent: 1407, descent: 756, x: 266, y: 22, towardOrigin: true},
+				{runes: 6, lineCol: screenPos{line: 1, col: 0}, ascent: 1407, descent: 756, x: 2511, y: 41, towardOrigin: true},
+				{runes: 7, lineCol: screenPos{line: 1, col: 1}, ascent: 1407, descent: 756, x: 2267, y: 41, towardOrigin: true},
+				{runes: 8, lineCol: screenPos{line: 1, col: 2}, ascent: 1407, descent: 756, x: 1969, y: 41, towardOrigin: true},
+				{runes: 9, lineCol: screenPos{line: 1, col: 3}, ascent: 1407, descent: 756, x: 1671, y: 41, towardOrigin: true},
+				{runes: 10, lineCol: screenPos{line: 1, col: 4}, ascent: 1407, descent: 756, x: 1365, y: 41, towardOrigin: true},
+				{runes: 11, lineCol: screenPos{line: 1, col: 5}, ascent: 1407, descent: 756, x: 713, y: 41, towardOrigin: true},
+				{runes: 12, lineCol: screenPos{line: 1, col: 6}, ascent: 1407, descent: 756, x: 415, y: 41, towardOrigin: true},
+				{runes: 13, lineCol: screenPos{line: 1, col: 7}, ascent: 1407, descent: 756, x: 0, y: 41, towardOrigin: true},
+			},
+		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
-			glyphs := getGlyphs(16, 0, 200, tc.align, tc.str)
+			glyphs := getGlyphs(16, 0, tc.lineWidth, tc.align, tc.str)
 			var gi glyphIndex
 			gi.reset()
 			for _, g := range glyphs {
@@ -227,11 +271,11 @@ func TestIndexPositionBidi(t *testing.T) {
 			name:   "bidi ltr",
 			glyphs: bidiLTRText,
 			expectedXs: []fixed.Int26_6{
-				0, 626, 1196, 1766, 2051, 2621, 3191, 3444, 3956, 4468, 4753, 7133, 6330, 5738, 5440, 5019, 4753, // Positions on line 0.
+				0, 626, 1196, 1766, 2051, 2621, 3191, 3444, 3956, 4468, 4753, 7133, 6330, 5738, 5440, 5019, // Positions on line 0.
 
-				3953, 3185, 2417, 1649, 881, 596, 298, 0, 3953, 4238, 4523, 5093, 5605, 5890, 7905, 7599, 7007, 6156, 5890, // Positions on line 1.
+				3953, 3185, 2417, 1649, 881, 596, 298, 0, 3953, 4238, 4523, 5093, 5605, 5890, 7905, 7599, 7007, 6156, // Positions on line 1.
 
-				4660, 3892, 3124, 2356, 1588, 1303, 788, 406, 0, 4660, 4945, 5235, 5805, 6375, 6660, 6934, 7504, 8016, 8528, 8813, // Positions on line 2.
+				4660, 3892, 3124, 2356, 1588, 1303, 788, 406, 0, 4660, 4945, 5235, 5805, 6375, 6660, 6934, 7504, 8016, 8528, // Positions on line 2.
 
 				0, 570, 1140, 1710, 2034, // Positions on line 3.
 			},
@@ -240,11 +284,11 @@ func TestIndexPositionBidi(t *testing.T) {
 			name:   "bidi rtl",
 			glyphs: bidiRTLText,
 			expectedXs: []fixed.Int26_6{
-				2665, 3291, 3861, 4431, 4716, 5286, 5856, 6109, 6621, 7133, 2665, 2380, 1577, 985, 687, 266, 0, // Positions on line 0.
+				2665, 3291, 3861, 4431, 4716, 5286, 5856, 6109, 6621, 7133, 2665, 2380, 1577, 985, 687, 266, // Positions on line 0.
 
-				7886, 7118, 6350, 5582, 4814, 4529, 4231, 3933, 3667, 2300, 2585, 3155, 3667, 2300, 2015, 1709, 1117, 266, 0, // Positions on line 1.
+				7886, 7118, 6350, 5582, 4814, 4529, 4231, 3933, 3667, 2300, 2585, 3155, 3667, 2300, 2015, 1709, 1117, 266, // Positions on line 1.
 
-				8794, 8026, 7258, 6490, 5722, 5437, 4922, 4540, 4134, 3868, 0, 290, 860, 1430, 1715, 1989, 2559, 3071, 3583, 3868, // Positions on line 2.
+				8794, 8026, 7258, 6490, 5722, 5437, 4922, 4540, 4134, 3868, 0, 290, 860, 1430, 1715, 1989, 2559, 3071, 3583, // Positions on line 2.
 
 				324, 894, 1464, 2034, 324, 0, // Positions on line 3.
 			},
@@ -524,13 +568,13 @@ func TestIndexPositionRunes(t *testing.T) {
 				{runes: 12, lineCol: screenPos{line: 1, col: 8}, runIndex: 1, towardOrigin: true},
 				{runes: 13, lineCol: screenPos{line: 1, col: 9}, runIndex: 1, towardOrigin: true},
 				{runes: 14, lineCol: screenPos{line: 1, col: 10}, runIndex: 1, towardOrigin: true},
-				{runes: 15, lineCol: screenPos{line: 1, col: 11}, runIndex: 1, towardOrigin: true},
+				{runes: 15, lineCol: screenPos{line: 1, col: 11}, runIndex: 2, towardOrigin: true},
 				{runes: 16, lineCol: screenPos{line: 1, col: 12}, runIndex: 2, towardOrigin: true},
 				{runes: 17, lineCol: screenPos{line: 1, col: 13}, runIndex: 2, towardOrigin: true},
 				{runes: 18, lineCol: screenPos{line: 2, col: 0}, runIndex: 0, towardOrigin: true},
 				{runes: 19, lineCol: screenPos{line: 2, col: 1}, runIndex: 0, towardOrigin: true},
 				{runes: 20, lineCol: screenPos{line: 2, col: 2}, runIndex: 0, towardOrigin: true},
-				{runes: 21, lineCol: screenPos{line: 2, col: 3}, runIndex: 0, towardOrigin: true},
+				{runes: 21, lineCol: screenPos{line: 2, col: 3}, runIndex: 1, towardOrigin: true},
 				{runes: 22, lineCol: screenPos{line: 2, col: 4}, runIndex: 1, towardOrigin: true},
 				{runes: 23, lineCol: screenPos{line: 2, col: 5}, runIndex: 1, towardOrigin: true},
 				{runes: 24, lineCol: screenPos{line: 2, col: 6}, runIndex: 1, towardOrigin: true},
@@ -542,7 +586,7 @@ func TestIndexPositionRunes(t *testing.T) {
 				{runes: 29, lineCol: screenPos{line: 3, col: 1}, runIndex: 0, towardOrigin: true},
 				{runes: 30, lineCol: screenPos{line: 3, col: 2}, runIndex: 0, towardOrigin: true},
 				{runes: 31, lineCol: screenPos{line: 3, col: 3}, runIndex: 0, towardOrigin: true},
-				{runes: 32, lineCol: screenPos{line: 3, col: 4}, runIndex: 0, towardOrigin: true},
+				{runes: 32, lineCol: screenPos{line: 3, col: 4}, runIndex: 1, towardOrigin: true},
 				{runes: 33, lineCol: screenPos{line: 3, col: 5}, runIndex: 1, towardOrigin: true},
 				{runes: 34, lineCol: screenPos{line: 3, col: 6}, runIndex: 1, towardOrigin: true},
 				{runes: 35, lineCol: screenPos{line: 4, col: 0}, runIndex: 0, towardOrigin: true},