From 73787b8478b793725f679564baa0ec2f1c61527d Mon Sep 17 00:00:00 2001 From: Chris Waldon Date: Wed, 5 Apr 2023 10:16:39 -0400 Subject: [PATCH] text,widget: minimize loss of positional precision in shaping This commit combs through the logic of computing glyph sizes and positions, attempting to remove all unnecessary rounding and truncation. This is in an effort to help text display consistently when different-length strings are displayed near one another. The specific problem prompting this change was end-aligned text stacked in rows with a common suffix. If the rows displayed different values, they would shape such that those final glyphs were at different fractional x coordinates, and then they would be aligned with rounding that could display them at different x positions in spite of the fact that both suffixes are the same glyphs. By removing rounding from Alignment.Align, the largest problem is fixed, but I'm also removing other unnecessary loss of precision that can circumstantially contribute to this sort of visual issue. Signed-off-by: Chris Waldon --- text/gotext.go | 19 +++++++++++-------- text/text.go | 4 ++-- widget/index_test.go | 22 +++++++++++----------- widget/label.go | 11 ++++++++--- 4 files changed, 32 insertions(+), 24 deletions(-) diff --git a/text/gotext.go b/text/gotext.go index 38eaa722..c78ce664 100644 --- a/text/gotext.go +++ b/text/gotext.go @@ -563,16 +563,15 @@ func (s *shaperImpl) Shape(pathOps *op.Ops, gs []Glyph) clip.PathSpec { } ppem, faceIdx, gid := splitGlyphID(g.ID) face := s.orderer.faceFor(faceIdx) - ppemInt := ppem.Round() - scaleFactor := float32(ppemInt) / float32(face.Upem()) + scaleFactor := fixedToFloat(ppem) / float32(face.Upem()) glyphData := face.GlyphData(gid) switch glyphData := glyphData.(type) { case api.GlyphOutline: outline := glyphData // Move to glyph position. pos := f32.Point{ - X: float32(g.X-x)/64 - float32(g.Offset.X)/64, - Y: -float32(g.Offset.Y) / 64, + X: fixedToFloat((g.X - x) - g.Offset.X), + Y: -fixedToFloat(g.Offset.Y), } builder.Move(pos.Sub(lastPos)) lastPos = pos @@ -617,6 +616,10 @@ func (s *shaperImpl) Shape(pathOps *op.Ops, gs []Glyph) clip.PathSpec { return builder.End() } +func fixedToFloat(i fixed.Int26_6) float32 { + return float32(i) / 64.0 +} + // Bitmaps returns an op.CallOp that will display all bitmap glyphs within gs. // The positioning of the bitmaps uses the same logic as Shape(), so the returned // CallOp can be added at the same offset as the path data returned by Shape() @@ -656,10 +659,10 @@ func (s *shaperImpl) Bitmaps(ops *op.Ops, gs []Glyph) op.CallOp { imgOp = bitmapData.img imgSize = bitmapData.size } - off := op.Offset(image.Point{ - X: ((g.X - x) - g.Offset.X).Round(), - Y: g.Offset.Y.Round() - g.Ascent.Round(), - }).Push(ops) + off := op.Affine(f32.Affine2D{}.Offset(f32.Point{ + X: fixedToFloat((g.X - x) - g.Offset.X), + Y: fixedToFloat(g.Offset.Y - g.Ascent), + })).Push(ops) cl := clip.Rect{Max: imgSize}.Push(ops) glyphSize := image.Rectangle{ diff --git a/text/text.go b/text/text.go index 2185da43..b6d7cac0 100644 --- a/text/text.go +++ b/text/text.go @@ -100,9 +100,9 @@ func (a Alignment) Align(dir system.TextDirection, width fixed.Int26_6, maxWidth } switch a { case Middle: - return fixed.I(((mw - width) / 2).Floor()) + return (mw - width) / 2 case End: - return fixed.I((mw - width).Floor()) + return (mw - width) case Start: return 0 default: diff --git a/widget/index_test.go b/widget/index_test.go index f3d5af88..532c6338 100644 --- a/widget/index_test.go +++ b/widget/index_test.go @@ -168,10 +168,10 @@ func TestIndexPositionWhitespace(t *testing.T) { {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}}, {x: fixed.Int26_6(832), y: 54, ascent: fixed.Int26_6(968), descent: fixed.Int26_6(216), runes: 2, lineCol: screenPos{line: 2}}, - {x: fixed.Int26_6(0), y: 73, ascent: fixed.Int26_6(968), descent: fixed.Int26_6(216), runes: 3, lineCol: screenPos{line: 3}}, - {x: fixed.Int26_6(570), y: 73, ascent: fixed.Int26_6(968), descent: fixed.Int26_6(216), runes: 4, lineCol: screenPos{line: 3, col: 1}}, - {x: fixed.Int26_6(1140), y: 73, ascent: fixed.Int26_6(968), descent: fixed.Int26_6(216), runes: 5, lineCol: screenPos{line: 3, col: 2}}, - {x: fixed.Int26_6(1652), y: 73, ascent: fixed.Int26_6(968), descent: fixed.Int26_6(216), runes: 6, lineCol: screenPos{line: 3, col: 3}}, + {x: fixed.Int26_6(6), y: 73, ascent: fixed.Int26_6(968), descent: fixed.Int26_6(216), runes: 3, lineCol: screenPos{line: 3}}, + {x: fixed.Int26_6(576), y: 73, ascent: fixed.Int26_6(968), descent: fixed.Int26_6(216), runes: 4, lineCol: screenPos{line: 3, col: 1}}, + {x: fixed.Int26_6(1146), y: 73, ascent: fixed.Int26_6(968), descent: fixed.Int26_6(216), runes: 5, lineCol: screenPos{line: 3, col: 2}}, + {x: fixed.Int26_6(1658), y: 73, ascent: fixed.Int26_6(968), descent: fixed.Int26_6(216), runes: 6, lineCol: screenPos{line: 3, col: 3}}, }, }, { @@ -380,7 +380,7 @@ func TestIndexPositionLines(t *testing.T) { glyphs: bidiLTRTextOpp, expectedLines: []lineInfo{ { - xOff: fixed.Int26_6(3072), + xOff: fixed.Int26_6(3107), yOff: 22, glyphs: 15, width: fixed.Int26_6(7133), @@ -388,7 +388,7 @@ func TestIndexPositionLines(t *testing.T) { descent: fixed.Int26_6(756), }, { - xOff: fixed.Int26_6(2304), + xOff: fixed.Int26_6(2335), yOff: 56, glyphs: 15, width: fixed.Int26_6(7905), @@ -396,7 +396,7 @@ func TestIndexPositionLines(t *testing.T) { descent: fixed.Int26_6(756), }, { - xOff: fixed.Int26_6(1408), + xOff: fixed.Int26_6(1427), yOff: 90, glyphs: 18, width: fixed.Int26_6(8813), @@ -404,7 +404,7 @@ func TestIndexPositionLines(t *testing.T) { descent: fixed.Int26_6(756), }, { - xOff: fixed.Int26_6(8192), + xOff: fixed.Int26_6(8206), yOff: 117, glyphs: 4, width: fixed.Int26_6(2034), @@ -419,7 +419,7 @@ func TestIndexPositionLines(t *testing.T) { glyphs: bidiRTLTextOpp, expectedLines: []lineInfo{ { - xOff: fixed.Int26_6(384), + xOff: fixed.Int26_6(404), yOff: 22, glyphs: 20, width: fixed.Int26_6(9836), @@ -427,7 +427,7 @@ func TestIndexPositionLines(t *testing.T) { descent: fixed.Int26_6(756), }, { - xOff: fixed.Int26_6(1408), + xOff: fixed.Int26_6(1439), yOff: 56, glyphs: 19, width: fixed.Int26_6(8801), @@ -435,7 +435,7 @@ func TestIndexPositionLines(t *testing.T) { descent: fixed.Int26_6(756), }, { - xOff: fixed.Int26_6(4352), + xOff: fixed.Int26_6(4388), yOff: 90, glyphs: 13, width: fixed.Int26_6(5852), diff --git a/widget/label.go b/widget/label.go index 93ac4739..10af00bc 100644 --- a/widget/label.go +++ b/widget/label.go @@ -5,6 +5,7 @@ package widget import ( "image" + "gioui.org/f32" "gioui.org/io/semantic" "gioui.org/layout" "gioui.org/op" @@ -88,7 +89,7 @@ type textIterator struct { // linesSeen tracks the quantity of line endings this iterator has seen. linesSeen int // lineOff tracks the origin for the glyphs in the current line. - lineOff image.Point + lineOff f32.Point // padding is the space needed outside of the bounds of the text to ensure no // part of a glyph is clipped. padding image.Rectangle @@ -146,6 +147,10 @@ func (it *textIterator) processGlyph(g text.Glyph, ok bool) (_ text.Glyph, visib return g, ok && !below } +func fixedToFloat(i fixed.Int26_6) float32 { + return float32(i) / 64.0 +} + // paintGlyph buffers up and paints text glyphs. It should be invoked iteratively upon each glyph // until it returns false. The line parameter should be a slice with // a backing array of sufficient size to buffer multiple glyphs. @@ -157,12 +162,12 @@ func (it *textIterator) paintGlyph(gtx layout.Context, shaper *text.Shaper, glyp _, visibleOrBefore := it.processGlyph(glyph, true) if it.visible { if len(line) == 0 { - it.lineOff = image.Point{X: glyph.X.Floor(), Y: int(glyph.Y)}.Sub(it.viewport.Min) + it.lineOff = f32.Point{X: fixedToFloat(glyph.X), Y: float32(glyph.Y)}.Sub(layout.FPt(it.viewport.Min)) } line = append(line, glyph) } if glyph.Flags&text.FlagLineBreak != 0 || cap(line)-len(line) == 0 || !visibleOrBefore { - t := op.Offset(it.lineOff).Push(gtx.Ops) + t := op.Affine(f32.Affine2D{}.Offset(it.lineOff)).Push(gtx.Ops) path := shaper.Shape(line) outline := clip.Outline{Path: path}.Op().Push(gtx.Ops) it.material.Add(gtx.Ops)