widget: define incrementing combinedPos and test

This commit restructures seekPosition from a complex state-manipulating
loop into a simple loop of iteratively applying an increment operation
to the combinedPos. The increment operation itself is now tested, and
much easier to understand.

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

widget/editor.go

@@ -1084,46 +1084,59 @@ func (e *Editor) closestPosition(pos combinedPos) combinedPos {
 // seekPosition seeks to the position closest to needle, starting at start and returns true.
 // If limit is non-zero, seekPosition stops seeks after limit runes and returns false.
 func seekPosition(lines []text.Line, alignment text.Alignment, width int, start, needle combinedPos, limit int) (combinedPos, bool) {
-	l := lines[start.lineCol.Y]
 	count := 0
-	// Advance next and prev until next is greater than or equal to pos.
+	// Advance until start is greater than or equal to needle.
 	for {
-		start.clusterIndex = clusterIndexFor(l, start.lineCol.X, start.clusterIndex)
+		if positionGreaterOrEqual(lines, start, needle) {
-		for ; start.lineCol.X < l.Layout.Runes.Count; start.lineCol.X++ {
-			cluster := l.Layout.Clusters[start.clusterIndex]
-			if start.runes >= cluster.Runes.Offset+cluster.Runes.Count {
-				start.clusterIndex++
-				cluster = l.Layout.Clusters[start.clusterIndex]
-			}
-			if limit != 0 && count == limit {
-				return start, false
-			}
-			count++
-			if positionGreaterOrEqual(lines, start, needle) {
-				return start, true
-			}
-
-			start.x += cluster.RuneWidth()
-			start.runes++
-		}
-		if start.lineCol.Y == len(lines)-1 {
-			// End of file.
 			return start, true
 		}
-
+		var eof bool
-		prevDesc := l.Descent
+		start, eof = incrementPosition(lines, alignment, width, start)
-		start.lineCol.Y++
+		if eof {
-		start.lineCol.X = 0
+			return start, true
-		start.clusterIndex = 0
+		}
-		l = lines[start.lineCol.Y]
+		count++
-		start.x = align(alignment, l.Layout.Direction, l.Width, width)
+		if limit != 0 && count == limit {
-		if l.Layout.Direction.Progression() == system.TowardOrigin {
+			return start, false
-			start.x += l.Width
 		}
-		start.y += (prevDesc + l.Ascent).Ceil()
 	}
 }
 
+// incrementPosition updates pos to be one rune further into the text.
+// All fields of pos must be valid before calling incrementPosition. eof will be true when
+// pos represents the final text position in the lines.
+func incrementPosition(lines []text.Line, alignment text.Alignment, width int, pos combinedPos) (_ combinedPos, eof bool) {
+	l := lines[pos.lineCol.Y]
+	handleLineTransition := func() bool {
+		if pos.lineCol.X >= l.Layout.Runes.Count {
+			if pos.lineCol.Y == len(lines)-1 {
+				// End of file.
+				return true
+			}
+			// Move to next line.
+			prevDesc := l.Descent
+			pos.lineCol.Y++
+			pos.lineCol.X = 0
+			pos.clusterIndex = 0
+			l = lines[pos.lineCol.Y]
+			// Use firstPos to get the correct x coordinate of the beginning of the line.
+			alignedPos := firstPos(l, alignment, width)
+			pos.x = alignedPos.x
+			pos.y += (prevDesc + l.Ascent).Ceil()
+		}
+		return false
+	}
+	if handleLineTransition() {
+		return pos, true
+	}
+	pos.x += l.Layout.Clusters[pos.clusterIndex].RuneWidth()
+	pos.runes++
+	pos.lineCol.X++
+	pos.clusterIndex = clusterIndexFor(l, pos.lineCol.X, pos.clusterIndex)
+
+	return pos, handleLineTransition()
+}
+
 // indexRune returns the latest rune index and byte offset no later than r.
 func (e *Editor) indexRune(r int) offEntry {
 	// Initialize index.

widget/label.go

@@ -32,11 +32,17 @@ type screenPos image.Point
 
 const inf = 1e6
 
+// posIsAbove returns whether the position described in pos by the lineCol and
+// y fields is above the given y coordinate. It is invalid to call this function
+// unless both the lineCol and (x,y) fields of pos are populated.
 func posIsAbove(lines []text.Line, pos combinedPos, y int) bool {
 	line := lines[pos.lineCol.Y]
 	return pos.y+line.Bounds.Max.Y.Ceil() < y
 }
 
+// posIsAbove returns whether the position described in pos by the lineCol and
+// y fields is below the given y coordinate. It is invalid to call this function
+// unless both the lineCol and (x,y) fields of pos are populated.
 func posIsBelow(lines []text.Line, pos combinedPos, y int) bool {
 	line := lines[pos.lineCol.Y]
 	return pos.y+line.Bounds.Min.Y.Floor() > y
@@ -84,6 +90,9 @@ func subLayout(line text.Line, start, end combinedPos) text.Layout {
 //
 // The results can be counterinuitive due to the fact that meaning
 // of alignment changes depending on the text direction.
+//
+// The returned pos can be considered valid only for the first line
+// of a body of text.
 func firstPos(line text.Line, alignment text.Alignment, width int) combinedPos {
 	p := combinedPos{
 		x: align(alignment, line.Layout.Direction, line.Width, width),

widget/text_test.go

@@ -6,12 +6,13 @@ import (
 
 	nsareg "eliasnaur.com/font/noto/sans/arabic/regular"
 	"gioui.org/font/opentype"
+	"gioui.org/io/system"
 	"gioui.org/text"
 	"golang.org/x/image/font/gofont/goregular"
 	"golang.org/x/image/math/fixed"
 )
 
-func TestFirstPos(t *testing.T) {
+func makeTestText(fontSize, lineWidth int) ([]text.Line, []text.Line) {
 	ltrFace, _ := opentype.Parse(goregular.TTF)
 	rtlFace, _ := opentype.Parse(nsareg.TTF)
 
@@ -25,11 +26,15 @@ func TestFirstPos(t *testing.T) {
 			Face: rtlFace,
 		},
 	})
-	fontSize := 16
-	lineWidth := int(fontSize) * 10
 	ltrText := shaper.LayoutString(text.Font{Typeface: "LTR"}, fixed.I(fontSize), lineWidth, english, "The quick brown fox\njumps over the lazy dog.")
 	rtlText := shaper.LayoutString(text.Font{Typeface: "RTL"}, fixed.I(fontSize), lineWidth, arabic, "الحب سماء لا\nتمط غير الأحلام")
+	return ltrText, rtlText
+}
 
+func TestFirstPos(t *testing.T) {
+	fontSize := 16
+	lineWidth := fontSize * 10
+	ltrText, rtlText := makeTestText(fontSize, lineWidth)
 	type testcase struct {
 		name      string
 		line      text.Line
@@ -142,3 +147,100 @@ func TestFirstPos(t *testing.T) {
 		}
 	}
 }
+
+func TestIncrementPosition(t *testing.T) {
+	fontSize := 16
+	lineWidth := fontSize * 3
+	ltrText, rtlText := makeTestText(fontSize, lineWidth)
+	type trial struct {
+		input, output combinedPos
+	}
+	type testcase struct {
+		name       string
+		align      text.Alignment
+		width      int
+		lines      []text.Line
+		firstInput combinedPos
+		check      func(t *testing.T, iteration int, input, output combinedPos, end bool)
+	}
+	for _, tc := range []testcase{
+		{
+			name:       "ltr",
+			align:      text.Start,
+			width:      lineWidth,
+			lines:      ltrText,
+			firstInput: firstPos(ltrText[0], text.Start, lineWidth),
+		},
+		{
+			name:       "rtl",
+			align:      text.Start,
+			width:      lineWidth,
+			lines:      rtlText,
+			firstInput: firstPos(rtlText[0], text.Start, lineWidth),
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			input := tc.firstInput
+			for i := 0; true; i++ {
+				output, end := incrementPosition(tc.lines, tc.align, tc.width, input)
+				finalRunes := tc.lines[len(tc.lines)-1].Layout.Runes
+				finalRune := finalRunes.Count + finalRunes.Offset
+				if end && output.runes != finalRune {
+					t.Errorf("iteration %d ended prematurely. Has runes %d, expected %d", i, output.runes, finalRune)
+				}
+				if end {
+					break
+				}
+				if input == output {
+					t.Errorf("iteration %d: identical output:\ninput:  %#+v\noutput: %#+v", i, input, output)
+				}
+				// We should always advance on either the X or Y axis.
+				if input.y == output.y {
+					expectedAdvance := tc.lines[input.lineCol.Y].Layout.Clusters[input.clusterIndex].Advance != 0
+					rtl := tc.lines[input.lineCol.Y].Layout.Direction.Progression() == system.TowardOrigin
+					if expectedAdvance {
+						if (rtl && input.x <= output.x) || (!rtl && input.x >= output.x) {
+							t.Errorf("iteration %d advanced the wrong way on x axis: input %v(%d) output %v(%d)", i, input.x, input.x, output.x, output.x)
+						}
+					} else if input.x != output.x {
+						t.Errorf("iteration %d advanced x axis when it should not have: input %v(%d) output %v(%d)", i, input.x, input.x, output.x, output.x)
+					}
+					// If we stayed on the same line, the line-local rune count should
+					// be incremented.
+					if input.lineCol.X >= output.lineCol.X {
+						t.Errorf("iteration %d advanced lineCol.X incorrectly: input %d output %d", i, input.lineCol.X, output.lineCol.X)
+					}
+					// We don't necessarily increment clusters every time, but it should never
+					// go down.
+					if input.clusterIndex > output.clusterIndex {
+						t.Errorf("iteration %d advanced clusterIndex incorrectly: input %d output %d", i, input.clusterIndex, output.clusterIndex)
+					}
+				} else {
+					if input.y >= output.y {
+						t.Errorf("iteration %d advanced the wrong way on y axis: input %v(%d) output %v(%d)", i, input.y, input.y, output.y, output.y)
+					} else {
+						// We correctly advanced on Y axis, so X should be reset to "start of line"
+						// for the text direction.
+						rtl := tc.lines[input.lineCol.Y].Layout.Direction.Progression() == system.TowardOrigin
+						if (rtl && input.x >= output.x) || (!rtl && input.x <= output.x) {
+							t.Errorf("iteration %d reset x axis incorrectly: input %v(%d) output %v(%d)", i, input.x, input.x, output.x, output.x)
+						}
+					}
+					if input.lineCol.Y >= output.lineCol.Y {
+						t.Errorf("iteration %d advanced lineCol.Y incorrectly: input %d output %d", i, input.lineCol.Y, output.lineCol.Y)
+					}
+					if output.clusterIndex != 0 {
+						t.Errorf("iteration %d should have zeroed clusterIndex, got: %d", i, output.clusterIndex)
+					}
+					if output.lineCol.X != 0 {
+						t.Errorf("iteration %d should have zeroed lineCol.X, got: %d", i, output.lineCol.X)
+					}
+				}
+				if output.runes != input.runes+1 {
+					t.Errorf("iteration %d advanced runes incorrectly: input %d output %d", i, input.runes, output.runes)
+				}
+				input = output
+			}
+		})
+	}
+}