Fix line wrapping, spacing, and bidi ordering in shaping

shaping/input.go

@@ -448,6 +448,7 @@ func (seg *Segmenter) splitByFace(faces Fontmap) {
 }
 
 func splitByFace(input Input, availableFaces Fontmap, buffer []Input, isLast bool) []Input {
+	input.Face = nil
 	currentInput := input
 	for i := input.RunStart; i < input.RunEnd; i++ {
 		r := input.Text[i]

shaping/input_test.go

@@ -792,3 +792,15 @@ func Test_enforceLang(t *testing.T) {
 		}
 	}
 }
+
+func TestSplitByFaceIgnoresInputFace(t *testing.T) {
+	oldFace := loadOpentypeFont(t, "../font/testdata/Roboto-Regular.ttf")
+	selected := loadOpentypeFont(t, "../font/testdata/UbuntuMono-R.ttf")
+	for _, text := range []string{" x", "  ", "\u200b"} {
+		input := Input{Text: []rune(text), RunEnd: len([]rune(text)), Face: oldFace}
+		runs := SplitByFace(input, fixedFontmap{selected})
+		if len(runs) != 1 || runs[0].Face != selected {
+			t.Fatalf("%q: inherited face was not ignored", text)
+		}
+	}
+}

shaping/lru.go

@@ -12,26 +12,26 @@ const defaultFontCacheSize = 32
 // fontEntry holds a single key-value pair for an LRU cache.
 type fontEntry struct {
 	next, prev *fontEntry
-	key        *font.Font
+	key        *font.Face
 	v          *harfbuzz.Font
 }
 
 // fontLRU is a least-recently-used cache for harfbuzz fonts built from
-// font.Fonts. It uses a doubly-linked list to track how recently elements have
+// font.Faces. A Face carries the variation coordinates. It uses a doubly-linked list to track how recently elements have
 // been used and a map to store element data for quick access.
 type fontLRU struct {
 	// This implementation is derived from the one here under the terms of the UNLICENSE:
 	//
 	// https://git.sr.ht/~eliasnaur/gio/tree/e768fe347a732056031100f2c66987d6db258ea4/item/text/lru.go
-	m          map[*font.Font]*fontEntry
+	m          map[*font.Face]*fontEntry
 	head, tail *fontEntry
 
 	// the actual cache size is [maxSizeOffset] + [defaultFontCacheSize]
 	maxSizeOffset int
 }
 
 // Get fetches the value associated with the given key, if any.
-func (l *fontLRU) Get(k *font.Font) (*harfbuzz.Font, bool) {
+func (l *fontLRU) Get(k *font.Face) (*harfbuzz.Font, bool) {
 	if lt, ok := l.m[k]; ok {
 		l.remove(lt)
 		l.insert(lt)
@@ -42,9 +42,9 @@ func (l *fontLRU) Get(k *font.Font) (*harfbuzz.Font, bool) {
 
 // Put inserts the given value with the given key, evicting old
 // cache entries if necessary.
-func (l *fontLRU) Put(k *font.Font, v *harfbuzz.Font) {
+func (l *fontLRU) Put(k *font.Face, v *harfbuzz.Font) {
 	if l.m == nil {
-		l.m = make(map[*font.Font]*fontEntry)
+		l.m = make(map[*font.Face]*fontEntry)
 		l.head = new(fontEntry)
 		l.tail = new(fontEntry)
 		l.head.prev = l.tail
@@ -53,7 +53,7 @@ func (l *fontLRU) Put(k *font.Font, v *harfbuzz.Font) {
 	val := &fontEntry{key: k, v: v}
 	l.m[k] = val
 	l.insert(val)
-	if len(l.m) > (defaultFontCacheSize + l.maxSizeOffset) {
+	for len(l.m) > (defaultFontCacheSize + l.maxSizeOffset) {
 		oldest := l.tail.next
 		l.remove(oldest)
 		delete(l.m, oldest.key)

shaping/output.go

@@ -3,6 +3,8 @@
 package shaping
 
 import (
+	"unicode"
+
 	"github.com/go-text/typesetting/bidi"
 	"github.com/go-text/typesetting/di"
 	"github.com/go-text/typesetting/font"
@@ -146,6 +148,7 @@ func (b Bounds) LineThickness() fixed.Int26_6 {
 
 // Output describes the dimensions and content of shaped text.
 type Output struct {
+	noExtents bool // the caller did not request glyph dimensions from the shaper
 	// Advance is the distance the Dot has advanced.
 	// It is typically positive for horizontal text, negative for vertical.
 	Advance fixed.Int26_6
@@ -211,13 +214,14 @@ func (o *Output) RecomputeAdvance() {
 // if a white space character ends the run.
 // Any end letter spacing (on the last glyph) is also removed
 // The paragraphDir is the text direction of the overall paragraph containing o.
-// If the paragraphDir is different then o's Direction, this method has no effect
-// because the trailing space in this run will always be internal to the paragraph.
+// For runs opposing the paragraph direction, it removes only the resolved
+// bidi whitespace that UAX #9 L1 moves to the line end.
 //
 // TODO: should we take into account multiple spaces ?
-func (o *Output) advanceSpaceAware(paragraphDir di.Direction) fixed.Int26_6 {
+func (o *Output) advanceSpaceAware(paragraphDir di.Direction, text []rune) fixed.Int26_6 {
 	L := len(o.Glyphs)
-	if L == 0 || paragraphDir != o.Direction {
+	sameProgression := paragraphDir.Progression() == o.Direction.Progression()
+	if L == 0 || paragraphDir.Axis() != o.Direction.Axis() || (!sameProgression && o.Level == 0) {
 		return o.Advance
 	}
 
@@ -228,18 +232,47 @@ func (o *Output) advanceSpaceAware(paragraphDir di.Direction) fixed.Int26_6 {
 	} else {
 		lastG = o.Glyphs[0]
 	}
-	if o.Direction.IsVertical() {
-		if lastG.Height == 0 {
-			return o.Advance - lastG.Advance
-		}
-	} else { // horizontal
-		if lastG.Width == 0 {
-			return o.Advance - lastG.Advance
+	if o.isWhitespace(lastG, text) {
+		if !sameProgression {
+			// L1 cannot split a cluster or reset non-L1 spaces such as NBSP.
+			start := lastG.TextIndex()
+			for _, r := range text[start : start+lastG.RunesCount()] {
+				if !isBidiWhitespace(r) {
+					return o.Advance
+				}
+			}
 		}
+		return o.Advance - lastG.Advance
+	}
+	if !sameProgression {
+		return o.Advance
 	}
 	return o.Advance - lastG.endLetterSpacing
 }
 
+// isWhitespace reports whether g is a space. With extents, a non zero width or
+// height rules a glyph out. Without them, the source text decides, so a glyph
+// with omitted extents is not mistaken for a space.
+func (o *Output) isWhitespace(g Glyph, text []rune) bool {
+	if !o.noExtents && ((!o.Direction.IsVertical() && g.Width != 0) || (o.Direction.IsVertical() && g.Height != 0)) {
+		return false
+	}
+	// A missing glyph renders .notdef, not an empty space.
+	if o.noExtents && g.GlyphID == 0 {
+		return false
+	}
+	start, count := g.TextIndex(), g.RunesCount()
+	if start < 0 || count <= 0 || start > len(text)-count {
+		return false
+	}
+	for _, r := range text[start : start+count] {
+		if !unicode.IsSpace(r) || r == '\u1680' { // Ogham space mark is visible
+			return false
+		}
+	}
+	return true
+}
+
 // RecalculateAll updates the all other fields of the Output
 // to match the current contents of the Glyphs field.
 // This method will fail with UnimplementedDirectionError if the Output

shaping/output_test.go

@@ -337,13 +337,15 @@ func TestLine_AdjustBaseline(t *testing.T) {
 func TestAdvanceSpaceAware(t *testing.T) {
 	type testcase struct {
 		name         string
+		text         string
 		paragraphDir di.Direction
 		run          Output
 		expected     fixed.Int26_6
 	}
 	for _, tc := range []testcase{
 		{
 			name:         "matching ltr no whitespace",
+			text:         "x",
 			paragraphDir: di.DirectionLTR,
 			expected:     10,
 			run: Output{
@@ -362,6 +364,7 @@ func TestAdvanceSpaceAware(t *testing.T) {
 		},
 		{
 			name:         "matching ltr with whitespace",
+			text:         " ",
 			expected:     0,
 			paragraphDir: di.DirectionLTR,
 			run: Output{
@@ -380,6 +383,7 @@ func TestAdvanceSpaceAware(t *testing.T) {
 		},
 		{
 			name:         "matching rtl no whitespace",
+			text:         "x",
 			expected:     10,
 			paragraphDir: di.DirectionRTL,
 			run: Output{
@@ -398,6 +402,7 @@ func TestAdvanceSpaceAware(t *testing.T) {
 		},
 		{
 			name:         "matching rtl with whitespace",
+			text:         " ",
 			expected:     0,
 			paragraphDir: di.DirectionRTL,
 			run: Output{
@@ -416,6 +421,7 @@ func TestAdvanceSpaceAware(t *testing.T) {
 		},
 		{
 			name:         "mismatched ltr no whitespace",
+			text:         "x",
 			expected:     10,
 			paragraphDir: di.DirectionLTR,
 			run: Output{
@@ -434,6 +440,7 @@ func TestAdvanceSpaceAware(t *testing.T) {
 		},
 		{
 			name:         "mismatched ltr with whitespace",
+			text:         " ",
 			expected:     10,
 			paragraphDir: di.DirectionLTR,
 			run: Output{
@@ -452,6 +459,7 @@ func TestAdvanceSpaceAware(t *testing.T) {
 		},
 		{
 			name:         "mismatched rtl no whitespace",
+			text:         "x",
 			expected:     10,
 			paragraphDir: di.DirectionRTL,
 			run: Output{
@@ -470,6 +478,7 @@ func TestAdvanceSpaceAware(t *testing.T) {
 		},
 		{
 			name:         "mismatched rtl with whitespace",
+			text:         " ",
 			expected:     10,
 			paragraphDir: di.DirectionRTL,
 			run: Output{
@@ -488,7 +497,7 @@ func TestAdvanceSpaceAware(t *testing.T) {
 		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
-			actual := tc.run.advanceSpaceAware(tc.paragraphDir)
+			actual := tc.run.advanceSpaceAware(tc.paragraphDir, []rune(tc.text))
 			if actual != tc.expected {
 				t.Errorf("expected advance %d, got %d", tc.expected, actual)
 			}

shaping/shaping.go

@@ -26,6 +26,9 @@ type HarfbuzzShaper struct {
 // It is safe to adjust the size after using the shaper, though shrinking
 // it may result in many evictions on the next shaping.
 func (h *HarfbuzzShaper) SetFontCacheSize(size int) {
+	if size < 0 {
+		size = 0
+	}
 	h.fonts.maxSizeOffset = size - defaultFontCacheSize
 }
 
@@ -90,13 +93,14 @@ func (t *HarfbuzzShaper) shape(input Input, skipExtents bool) Output {
 	t.buf.Props.Script = input.Script
 
 	// reuse font when possible
-	font, ok := t.fonts.Get(input.Face.Font)
+	font, ok := t.fonts.Get(input.Face)
 	if !ok { // create a new font and cache it
 		font = harfbuzz.NewFont(input.Face)
-		t.fonts.Put(input.Face.Font, font)
+		t.fonts.Put(input.Face, font)
 	}
-	// adjust the user provided fields
-	font.XScale = int32(input.Size.Ceil()) << scaleShift
+	// adjust the user provided fields. Size is in 26.6 units, the same
+	// scaleShift the shaping results below are divided by.
+	font.XScale = int32(input.Size)
 	font.YScale = font.XScale
 
 	if L := len(input.FontFeatures); cap(t.features) < L {
@@ -149,16 +153,17 @@ func (t *HarfbuzzShaper) shape(input Input, skipExtents bool) Output {
 		glyphs[i].XBearing = fixed.I(int(extents.XBearing)) >> scaleShift
 		glyphs[i].YBearing = fixed.I(int(extents.YBearing)) >> scaleShift
 	}
-	countClusters(glyphs, input.RunEnd, input.Direction.Progression())
+	countClusters(glyphs, end, input.Direction.Progression())
 	out := Output{
+		noExtents: skipExtents,
 		Glyphs:    glyphs,
 		Direction: input.Direction,
 		Face:      input.Face,
 		Size:      input.Size,
 		Level:     input.Level,
 	}
-	out.Runes.Offset = input.RunStart
-	out.Runes.Count = input.RunEnd - input.RunStart
+	out.Runes.Offset = start
+	out.Runes.Count = end - start
 
 	if isSideways {
 		// set the Direction to the correct value.

shaping/shaping_test.go

@@ -746,3 +746,56 @@ func TestSpaceReplacement(t *testing.T) {
 	tu.Assert(t, out.Glyphs[2].Width.Round() == 0)
 	tu.Assert(t, out.Glyphs[2].GlyphID == font.EmptyGlyph)
 }
+
+func TestShapeClampsRunRange(t *testing.T) {
+	text := []rune("Hello")
+	out := (&HarfbuzzShaper{}).Shape(Input{Text: text, RunStart: 1, RunEnd: len(text) + 3, Face: benchEnFace, Size: fixed.I(16)})
+	tu.Assert(t, out.Runes == Range{Offset: 1, Count: len(text) - 1})
+	tu.Assert(t, len(out.Glyphs) == len(text)-1 && out.Glyphs[0].RuneCount == 1)
+}
+
+func TestShapeFontCacheKeyedOnFace(t *testing.T) {
+	face1 := loadOpentypeFont(t, "../font/testdata/Selawik-VF-Subset.ttf")
+	face2 := font.NewFace(face1.Font)
+	face2.SetVariations([]font.Variation{{Tag: ot.MustNewTag("wght"), Value: 700}})
+
+	var shaper HarfbuzzShaper
+	text := []rune("Hello")
+	input := Input{Text: text, RunEnd: len(text), Face: face1, Size: fixed.I(20), Script: language.Latin}
+	regular := shaper.Shape(input)
+	input.Face = face2
+	bold := shaper.Shape(input)
+	tu.Assert(t, regular.Advance != bold.Advance)
+}
+
+func TestShapeFractionalSize(t *testing.T) {
+	text := []rune("Hello")
+	var shaper HarfbuzzShaper
+	input := Input{Text: text, RunEnd: len(text), Face: benchEnFace, Size: fixed.I(13), Script: language.Latin}
+	at13 := shaper.Shape(input)
+	input.Size = fixed.I(25) / 2
+	at12dot5 := shaper.Shape(input)
+	tu.Assert(t, at12dot5.Advance < at13.Advance)
+}
+
+func TestShrinkingFontCache(t *testing.T) {
+	face := loadOpentypeFont(t, "../font/testdata/UbuntuMono-R.ttf")
+	for _, size := range []int{1, 0, -1} {
+		var shaper HarfbuzzShaper
+		input := Input{Text: []rune("a"), RunEnd: 1, Size: fixed.I(16), Script: language.Latin}
+		for i := 0; i < 4; i++ {
+			input.Face = font.NewFace(face.Font)
+			shaper.Shape(input)
+		}
+		shaper.SetFontCacheSize(size)
+		input.Face = font.NewFace(face.Font)
+		shaper.Shape(input)
+		want := size
+		if want < 0 {
+			want = 0
+		}
+		if got := len(shaper.fonts.m); got != want {
+			t.Fatalf("size %d: retained %d fonts, want %d", size, got, want)
+		}
+	}
+}