From c455f0f342ef0f49e69a500c44faca5c3120ced0 Mon Sep 17 00:00:00 2001 From: Chris Waldon Date: Mon, 19 Dec 2022 10:49:57 -0500 Subject: [PATCH] text,widget: test and fix minWidth alignment This commit unifies and fixes the shaper's handling of the alignment minimum width. Previously it was only considered when the text was a single line, but in hindsight that was clearly a mistake. Now the maximum width of all shaped lines and the minimum width is used to set the text alignment. This commit also fixes an index test in package widget that was relying on the old (incorrect) alignment behavior. Signed-off-by: Chris Waldon --- text/gotext.go | 13 +++++---- text/gotext_test.go | 63 ++++++++++++++++++++++++++++++++++++++++++++ widget/index_test.go | 4 +-- 3 files changed, 73 insertions(+), 7 deletions(-) diff --git a/text/gotext.go b/text/gotext.go index 2a187a8f..e000d1c7 100644 --- a/text/gotext.go +++ b/text/gotext.go @@ -480,18 +480,21 @@ func (s *shaperImpl) LayoutRunes(params Parameters, minWidth, maxWidth int, lc s } textLines[i] = otLine } - alignWidth := maxWidth - if len(textLines) == 1 { - alignWidth = max(minWidth, textLines[0].width.Ceil()) - } calculateYOffsets(textLines) return document{ lines: textLines, alignment: params.Alignment, - alignWidth: alignWidth, + alignWidth: alignWidth(minWidth, textLines), } } +func alignWidth(minWidth int, lines []line) int { + for _, l := range lines { + minWidth = max(minWidth, l.width.Ceil()) + } + return minWidth +} + // Shape converts the provided glyphs into a path. func (s *shaperImpl) Shape(ops *op.Ops, gs []Glyph) clip.PathSpec { var lastPos f32.Point diff --git a/text/gotext_test.go b/text/gotext_test.go index a66be7e5..a1aad337 100644 --- a/text/gotext_test.go +++ b/text/gotext_test.go @@ -55,6 +55,69 @@ func TestEmptyString(t *testing.T) { } } +func TestAlignWidth(t *testing.T) { + lines := []line{ + {width: fixed.I(50)}, + {width: fixed.I(75)}, + {width: fixed.I(25)}, + } + for _, minWidth := range []int{0, 50, 100} { + width := alignWidth(minWidth, lines) + if width < minWidth { + t.Errorf("expected width >= %d, got %d", minWidth, width) + } + } +} + +func TestShapingAlignWidth(t *testing.T) { + ppem := fixed.I(10) + ltrFace, _ := opentype.Parse(goregular.TTF) + shaper := testShaper(ltrFace) + + type testcase struct { + name string + minWidth, maxWidth int + expected int + str string + } + for _, tc := range []testcase{ + { + name: "zero min", + maxWidth: 100, + str: "a\nb\nc", + expected: 22, + }, + { + name: "min == max", + minWidth: 100, + maxWidth: 100, + str: "a\nb\nc", + expected: 100, + }, + { + name: "min < max", + minWidth: 50, + maxWidth: 100, + str: "a\nb\nc", + expected: 50, + }, + { + name: "min < max, text > min", + minWidth: 50, + maxWidth: 100, + str: "aphabetic\nb\nc", + expected: 60, + }, + } { + t.Run(tc.name, func(t *testing.T) { + lines := shaper.LayoutString(Parameters{PxPerEm: ppem}, tc.minWidth, tc.maxWidth, english, tc.str) + if lines.alignWidth != tc.expected { + t.Errorf("expected line alignWidth to be %d, got %d", tc.expected, lines.alignWidth) + } + }) + } +} + // TestNewlineSynthesis ensures that the shaper correctly inserts synthetic glyphs // representing newline runes. func TestNewlineSynthesis(t *testing.T) { diff --git a/widget/index_test.go b/widget/index_test.go index cb3f6a8d..4cb99339 100644 --- a/widget/index_test.go +++ b/widget/index_test.go @@ -36,11 +36,11 @@ func makePosTestText(fontSize, lineWidth int, alignOpposite bool) (bidiLTR, bidi ltrParams.Alignment = text.End rtlParams.Alignment = text.Start } - shaper.LayoutString(ltrParams, 0, lineWidth, english, bidiSource) + shaper.LayoutString(ltrParams, lineWidth, lineWidth, english, bidiSource) for g, ok := shaper.NextGlyph(); ok; g, ok = shaper.NextGlyph() { bidiLTR = append(bidiLTR, g) } - shaper.LayoutString(rtlParams, 0, lineWidth, arabic, bidiSource) + shaper.LayoutString(rtlParams, lineWidth, lineWidth, arabic, bidiSource) for g, ok := shaper.NextGlyph(); ok; g, ok = shaper.NextGlyph() { bidiRTL = append(bidiRTL, g) }