From fdd102aaf94c26187ca96830725dc2a07a292459 Mon Sep 17 00:00:00 2001 From: Chris Waldon Date: Mon, 24 Jul 2023 16:25:46 -0400 Subject: [PATCH] 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 --- widget/editor_test.go | 9 ++-- widget/index.go | 88 +++++++++++++++++++--------------- widget/index_test.go | 108 +++++++++++++++++++++++++++++------------- 3 files changed, 131 insertions(+), 74 deletions(-) diff --git a/widget/editor_test.go b/widget/editor_test.go index 707b6080..e09043a2 100644 --- a/widget/editor_test.go +++ b/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 cursor can reach the position after the final glyph (a space). - assertCaret(t, e, 0, 14, len("fflffl fflffl ")) + // The first line was broken by line wrapping, not a newline character, and has a trailing + // whitespace. However, we should never be able to reach the "other side" of such a trailing + // 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)) diff --git a/widget/index.go b/widget/index.go index b930e3d2..62bf2f82 100644 --- a/widget/index.go +++ b/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. - insertPositionAfter := gl.Flags&text.FlagClusterBreak != 0 && !breaksParagraph && gl.Runes > 0 + // a glyph cluster, has nonzero runes, and is not a hard newline. + 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 { - // Construct the text position _after_ gl. - pos := g.pos - pos.y = int(gl.Y) - pos.ascent = gl.Ascent - pos.descent = gl.Descent + if insertPositionsWithin { + // Construct the text positions _within_ gl. + g.pos.y = int(gl.Y) + g.pos.ascent = gl.Ascent + g.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) - pos.runes += runesPerPosition - pos.lineCol.col += runesPerPosition - g.positions = append(g.positions, pos) + g.pos.x = gl.X + adjust + perRune*fixed.Int26_6(i) + g.pos.runes += runesPerPosition + g.pos.lineCol.col += runesPerPosition + 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 } } diff --git a/widget/index_test.go b/widget/index_test.go index 1bf8836f..0b42cb1f 100644 --- a/widget/index_test.go +++ b/widget/index_test.go @@ -103,11 +103,12 @@ func getGlyphs(fontSize, minWidth, lineWidth int, align text.Alignment, str stri }, })) params := text.Parameters{ - PxPerEm: fixed.I(fontSize), - Alignment: align, - MinWidth: minWidth, - MaxWidth: lineWidth, - Locale: english, + PxPerEm: fixed.I(fontSize), + Alignment: align, + MinWidth: minWidth, + MaxWidth: lineWidth, + Locale: english, + WrapPolicy: text.WrapWords, } shaper.LayoutString(params, str) for g, ok := shaper.NextGlyph(); ok; g, ok = shaper.NextGlyph() { @@ -120,30 +121,34 @@ func getGlyphs(fontSize, minWidth, lineWidth int, align text.Alignment, str stri // for empty lines and the empty string. func TestIndexPositionWhitespace(t *testing.T) { type testcase struct { - name string - str string - align text.Alignment - expected []combinedPos + name string + str string + lineWidth int + align text.Alignment + expected []combinedPos } for _, tc := range []testcase{ { - name: "empty string", - str: "", + 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)}, }, }, { - name: "just hard newline", - str: "\n", + 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}}, }, }, { - name: "trailing newline", - str: "a\n", + 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}}, @@ -151,8 +156,9 @@ func TestIndexPositionWhitespace(t *testing.T) { }, }, { - name: "just blank line", - str: "\n\n", + 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}}, @@ -160,9 +166,10 @@ func TestIndexPositionWhitespace(t *testing.T) { }, }, { - name: "middle aligned blank lines", - str: "\n\n\nabc", - align: text.Middle, + 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}}, @@ -174,8 +181,9 @@ func TestIndexPositionWhitespace(t *testing.T) { }, }, { - name: "blank line", - str: "a\n\nb", + 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},