text: fix over-reading on truncated EOF

When consuming text from an io.Reader, the shaper could hit an EOF when reading the
text, then still try to check whether it was done by calling ReadByte() followed by
UnreadByte(). The ReadByte() would still return EOF, but the UnreadByte() would then
walk the iterator cursor backwards to the final byte of the text. If and only if the
text was being truncated, this unexpected cursor position could cause the shaper to
conclude that there were additional runes that were truncated, and thus the returned
glyph stream would account for too many runes. This commit provides a test and a fix.

Many thanks to Jack Mordaunt for the excellent bug report leading to this fix.

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

text/shaper.go

@@ -247,9 +247,11 @@ func (l *Shaper) layoutText(params Parameters, txt io.Reader, str string) {
 					break
 				}
 			}
-			_, re := l.reader.ReadByte()
-			done = re != nil
-			_ = l.reader.UnreadByte()
+			if !done {
+				_, re := l.reader.ReadByte()
+				done = re != nil
+				_ = l.reader.UnreadByte()
+			}
 		} else {
 			idx := strings.IndexByte(str, '\n')
 			if idx == -1 {

text/shaper_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	nsareg "eliasnaur.com/font/noto/sans/arabic/regular"
+	"gioui.org/font/gofont"
 	"gioui.org/font/opentype"
 	"gioui.org/io/system"
 	"golang.org/x/exp/slices"
@@ -431,3 +432,71 @@ func printLinePositioning(t *testing.T, lines []line, glyphs []Glyph) {
 		}
 	}
 }
+
+// TestShapeStringRuneAccounting tries shaping the same string/parameter combinations with both
+// shaping methods and ensures that the resulting glyph stream always has the right number of
+// runes accounted for.
+func TestShapeStringRuneAccounting(t *testing.T) {
+	type testcase struct {
+		name   string
+		input  string
+		params Parameters
+	}
+	type setup struct {
+		kind string
+		do   func(*Shaper, Parameters, string)
+	}
+	for _, tc := range []testcase{
+		{
+			name:  "simple truncated",
+			input: "abc",
+			params: Parameters{
+				PxPerEm:  fixed.Int26_6(16),
+				MaxWidth: 100,
+				MaxLines: 1,
+			},
+		},
+		{
+			name:  "simple",
+			input: "abc",
+			params: Parameters{
+				PxPerEm:  fixed.Int26_6(16),
+				MaxWidth: 100,
+			},
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			for _, setup := range []setup{
+				{
+					kind: "LayoutString",
+					do: func(shaper *Shaper, params Parameters, input string) {
+						shaper.LayoutString(params, input)
+					},
+				},
+				{
+					kind: "Layout",
+					do: func(shaper *Shaper, params Parameters, input string) {
+						shaper.Layout(params, strings.NewReader(input))
+					},
+				},
+			} {
+				t.Run(setup.kind, func(t *testing.T) {
+					shaper := NewShaper(gofont.Collection())
+					setup.do(shaper, tc.params, tc.input)
+
+					glyphs := []Glyph{}
+					for g, ok := shaper.NextGlyph(); ok; g, ok = shaper.NextGlyph() {
+						glyphs = append(glyphs, g)
+					}
+					totalRunes := 0
+					for _, g := range glyphs {
+						totalRunes += g.Runes
+					}
+					if inputRunes := len([]rune(tc.input)); totalRunes != inputRunes {
+						t.Errorf("input contained %d runes, but glyphs contained %d", inputRunes, totalRunes)
+					}
+				})
+			}
+		})
+	}
+}