Skip to content

Commit

Permalink
shaping: (wrapping) fix and stress test bidi run ordering
Browse files Browse the repository at this point in the history 
This commit adds a test for visual bidi run ordering and fixes the implementation
to pass it. The test itself is migrated from Gio's codebase, where it has been
verifying this functionality for years. It caught some problems with the new
implementation of this visual ordering algorithm, which are also fixed in this
commit.

Signed-off-by: Chris Waldon <[email protected]>
  • Loading branch information
@whereswaldon
whereswaldon committed Dec 16, 2024
1 parent 9191b59 commit da213d9
Show file tree
Hide file tree
Showing 2 changed files with 170 additions and 56 deletions.
121 changes: 66 additions & 55 deletions shaping/wrapping.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,11 @@ type WrapConfig struct {
// breaking in between UAX#14 line breaking candidates, or "within words" in
// many scripts.
BreakPolicy LineBreakPolicy
// DisableTrailingWhitespaceTrim turns off a feature that automatically sets the
// advance of trailing whitespace on a line to zero. In display contexts, you
// usually want this feature enabled, but for text editors it is frequently
// desirable to allow trailing whitespace to occupy space itself.
DisableTrailingWhitespaceTrim bool
}

// LineBreakPolicy specifies when considering a line break within a "word" or UAX#14
Expand Down Expand Up @@ -843,77 +848,83 @@ type WrappedLine struct {
NextLine int
}

// resolveBidi inverts the visual index of runs in line[start:end] and tracks the lowest and highest
// known visual positions.
func resolveBidi(line Line, start, end, maxVisual, minVisual int) (newMaxVisual, newMinVisual int) {
// Resove bidi from line[bidiStart:idx]
for bidiIdx := range line[start:end] {
// resolveBidi inverts the visual index of runs in line[start:end].
func resolveBidi(line Line, start, end int) {
// Resove bidi from line[start:end] by swapping pairs of visual indices across the midpoint
// of the range.
elementsCount := end - start
for bidiIdx := range line[start : start+elementsCount/2] {
absIndex := start + bidiIdx
visualIndex := len(line) - 1 - absIndex
line[absIndex].VisualIndex = int32(visualIndex)
if visualIndex > maxVisual {
maxVisual = absIndex
absIndex2 := start + (elementsCount - bidiIdx) - 1
line[absIndex].VisualIndex, line[absIndex2].VisualIndex = line[absIndex2].VisualIndex, line[absIndex].VisualIndex
}
}

func computeBidiOrdering(dir di.Direction, finalLine Line) {
// Here we populate the VisualIndex of each run.
bidiStart := -1
for idx, run := range finalLine {
basePosition := idx
if dir.Progression() == di.TowardTopLeft {
basePosition = len(finalLine) - 1 - idx
}
if visualIndex < minVisual {
minVisual = absIndex
finalLine[idx].VisualIndex = int32(basePosition)
if run.Direction == dir {
if bidiStart != -1 {
resolveBidi(finalLine, bidiStart, idx)
bidiStart = -1
}
} else if bidiStart == -1 {
bidiStart = idx
}
}
return maxVisual, minVisual
if bidiStart != -1 {
resolveBidi(finalLine, bidiStart, len(finalLine))
}
}

func (l *LineWrapper) postProcessLine(finalLine Line, done bool) (WrappedLine, bool) {
if len(finalLine) > 0 {
// Here we populate the VisualIndex of each run and track the index of the first
// and last visual runs for the purpose of trimming off trailing whitespace.
bidiStart := -1
maxVisualRunIdx := -1
minVisualRunIdx := len(finalLine)
for idx, run := range finalLine {
if run.Direction == l.config.Direction {
if bidiStart != -1 {
maxVisualRunIdx, minVisualRunIdx = resolveBidi(finalLine, bidiStart, idx, maxVisualRunIdx, minVisualRunIdx)
bidiStart = -1
}
finalLine[idx].VisualIndex = int32(idx)
if idx > maxVisualRunIdx {
maxVisualRunIdx = idx
}
if idx < minVisualRunIdx {
minVisualRunIdx = idx
computeBidiOrdering(l.config.Direction, finalLine)
if !l.config.DisableTrailingWhitespaceTrim {
// Here we find the last visual run in the line.
goalIdx := len(finalLine) - 1
if l.config.Direction.Progression() == di.TowardTopLeft {
goalIdx = 0
}
for logicalIdx, run := range finalLine {
if run.VisualIndex == int32(goalIdx) {
goalIdx = logicalIdx
break
}
} else if bidiStart == -1 {
bidiStart = idx
}
}
if bidiStart != -1 {
maxVisualRunIdx, minVisualRunIdx = resolveBidi(finalLine, bidiStart, len(finalLine), maxVisualRunIdx, minVisualRunIdx)
}
// This next block locates the first/last visual glyph on the line and
// zeroes its advance if it is whitespace.
var finalVisualRun *Output
var finalVisualGlyph *Glyph
if l.config.Direction.Progression() == di.FromTopLeft {
finalVisualRun = &finalLine[maxVisualRunIdx]
} else {
finalVisualRun = &finalLine[minVisualRunIdx]
}
if L := len(finalVisualRun.Glyphs); L > 0 {
// This next block locates the first/last visual glyph on the line and
// zeroes its advance if it is whitespace.
var finalVisualRun *Output
var finalVisualGlyph *Glyph
if l.config.Direction.Progression() == di.FromTopLeft {
finalVisualGlyph = &finalVisualRun.Glyphs[L-1]
finalVisualRun = &finalLine[goalIdx]
} else {
finalVisualGlyph = &finalVisualRun.Glyphs[0]
finalVisualRun = &finalLine[goalIdx]
}

if finalVisualRun.Direction.IsVertical() {
if finalVisualGlyph.Height == 0 {
finalVisualGlyph.YAdvance = 0
if L := len(finalVisualRun.Glyphs); L > 0 {
if l.config.Direction.Progression() == di.FromTopLeft {
finalVisualGlyph = &finalVisualRun.Glyphs[L-1]
} else {
finalVisualGlyph = &finalVisualRun.Glyphs[0]
}
} else { // horizontal
if finalVisualGlyph.Width == 0 {
finalVisualGlyph.XAdvance = 0

if finalVisualRun.Direction.IsVertical() {
if finalVisualGlyph.Height == 0 {
finalVisualGlyph.YAdvance = 0
}
} else { // horizontal
if finalVisualGlyph.Width == 0 {
finalVisualGlyph.XAdvance = 0
}
}
finalVisualRun.RecomputeAdvance()
}
finalVisualRun.RecomputeAdvance()
}

finalLogicalRun := finalLine[len(finalLine)-1]
Expand Down
105 changes: 104 additions & 1 deletion shaping/wrapping_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2239,7 +2239,10 @@ func TestGraphemeBreakingRegression(t *testing.T) {
for i, run := range shaped {
runs[i] = run.copy()
}
lines, truncated := wrapper.WrapParagraph(WrapConfig{BreakPolicy: Always}, maxWidth, bidiText2, NewSliceIterator(runs))
lines, truncated := wrapper.WrapParagraph(WrapConfig{
BreakPolicy: Always,
DisableTrailingWhitespaceTrim: true,
}, maxWidth, bidiText2, NewSliceIterator(runs))
checkRuneCounts(t, bidiText2, lines, truncated)

for i, baseLine := range lines[:len(lines)-1] {
Expand Down Expand Up @@ -3385,3 +3388,103 @@ func TestLineWrapPostProcess(t *testing.T) {
}
})
}

func TestComputeBidiOrdering(t *testing.T) {
type testcase struct {
name string
input []Output
direction di.Direction
expectedVisualOrder []int
}
for _, tc := range []testcase{
{
name: "ltr",
direction: di.DirectionLTR,
input: []Output{
{Direction: di.DirectionLTR},
{Direction: di.DirectionLTR},
{Direction: di.DirectionLTR},
},
expectedVisualOrder: []int{0, 1, 2},
},
{
name: "rtl",
direction: di.DirectionRTL,
input: []Output{
{Direction: di.DirectionRTL},
{Direction: di.DirectionRTL},
{Direction: di.DirectionRTL},
},
expectedVisualOrder: []int{2, 1, 0},
},
{
name: "bidi-ltr",
direction: di.DirectionLTR,
input: []Output{
{Direction: di.DirectionLTR},
{Direction: di.DirectionRTL},
{Direction: di.DirectionRTL},
{Direction: di.DirectionRTL},
{Direction: di.DirectionLTR},
},
expectedVisualOrder: []int{0, 3, 2, 1, 4},
},
{
name: "bidi-ltr-complex",
direction: di.DirectionLTR,
input: []Output{
{Direction: di.DirectionRTL},
{Direction: di.DirectionRTL},
{Direction: di.DirectionLTR},
{Direction: di.DirectionRTL},
{Direction: di.DirectionRTL},
{Direction: di.DirectionLTR},
{Direction: di.DirectionRTL},
{Direction: di.DirectionRTL},
{Direction: di.DirectionLTR},
{Direction: di.DirectionRTL},
{Direction: di.DirectionRTL},
},
expectedVisualOrder: []int{1, 0, 2, 4, 3, 5, 7, 6, 8, 10, 9},
},
{
name: "bidi-rtl",
direction: di.DirectionRTL,
input: []Output{
{Direction: di.DirectionRTL},
{Direction: di.DirectionLTR},
{Direction: di.DirectionLTR},
{Direction: di.DirectionLTR},
{Direction: di.DirectionRTL},
},
expectedVisualOrder: []int{4, 1, 2, 3, 0},
},
{
name: "bidi-rtl-complex",
direction: di.DirectionRTL,
input: []Output{
{Direction: di.DirectionLTR},
{Direction: di.DirectionLTR},
{Direction: di.DirectionRTL},
{Direction: di.DirectionLTR},
{Direction: di.DirectionLTR},
{Direction: di.DirectionRTL},
{Direction: di.DirectionLTR},
{Direction: di.DirectionLTR},
{Direction: di.DirectionRTL},
{Direction: di.DirectionLTR},
{Direction: di.DirectionLTR},
},
expectedVisualOrder: []int{9, 10, 8, 6, 7, 5, 3, 4, 2, 0, 1},
},
} {
t.Run(tc.name, func(t *testing.T) {
computeBidiOrdering(tc.direction, tc.input)
for visualIndex, logicalIndex := range tc.expectedVisualOrder {
if tc.input[logicalIndex].VisualIndex != int32(visualIndex) {
t.Errorf("line[%d]: expected visual index %v, got %v", logicalIndex, visualIndex, tc.input[logicalIndex].VisualIndex)
}
}
})
}
}

0 comments on commit da213d9

Please sign in to comment.