~eliasnaur/gio

0e5ec18a82e9244b81f0b414cda056c5f6155133 — Chris Waldon 1 year, 2 months ago 2a5f895
text: fix 32-bit glyph id packing

This commit fixes a problem in the unpacking of text.GlyphID on 32 bit architectures.
Incorrectly casting to an `int` on those platforms resulted in truncating the faceIndex
to always be zero. To catch mistakes like this in the future, I've added tests for this
problem that should be run by our new 32-bit CI testing.

Signed-off-by: Chris Waldon <christopher.waldon.dev@gmail.com>
2 files changed, 51 insertions(+), 3 deletions(-)

M text/gotext_test.go
M text/shaper.go
M text/gotext_test.go => text/gotext_test.go +50 -0
@@ 1,11 1,14 @@
package text

import (
	"fmt"
	"math"
	"reflect"
	"strconv"
	"testing"

	nsareg "eliasnaur.com/font/noto/sans/arabic/regular"
	"github.com/go-text/typesetting/font"
	"github.com/go-text/typesetting/shaping"
	"golang.org/x/image/font/gofont/goregular"
	"golang.org/x/image/math/fixed"


@@ 740,3 743,50 @@ func TestClosestFontByWeight(t *testing.T) {
		}
	}
}

func TestGlyphIDPacking(t *testing.T) {
	const maxPPem = fixed.Int26_6((1 << sizebits) - 1)
	type testcase struct {
		name      string
		ppem      fixed.Int26_6
		faceIndex int
		gid       font.GID
		expected  GlyphID
	}
	for _, tc := range []testcase{
		{
			name: "zero value",
		},
		{
			name:      "10 ppem faceIdx 1 GID 5",
			ppem:      fixed.I(10),
			faceIndex: 1,
			gid:       5,
			expected:  284223755780101,
		},
		{
			name:      maxPPem.String() + " ppem faceIdx " + strconv.Itoa(math.MaxUint16) + " GID " + fmt.Sprintf("%d", int64(math.MaxUint32)),
			ppem:      maxPPem,
			faceIndex: math.MaxUint16,
			gid:       math.MaxUint32,
			expected:  18446744073709551615,
		},
	} {
		t.Run(tc.name, func(t *testing.T) {
			actual := newGlyphID(tc.ppem, tc.faceIndex, tc.gid)
			if actual != tc.expected {
				t.Errorf("expected %d, got %d", tc.expected, actual)
			}
			actualPPEM, actualFaceIdx, actualGID := splitGlyphID(actual)
			if actualPPEM != tc.ppem {
				t.Errorf("expected ppem %d, got %d", tc.ppem, actualPPEM)
			}
			if actualFaceIdx != tc.faceIndex {
				t.Errorf("expected faceIdx %d, got %d", tc.faceIndex, actualFaceIdx)
			}
			if actualGID != tc.gid {
				t.Errorf("expected gid %d, got %d", tc.gid, actualGID)
			}
		})
	}
}

M text/shaper.go => text/shaper.go +1 -3
@@ 4,7 4,6 @@ package text

import (
	"bufio"
	"fmt"
	"io"
	"strings"
	"unicode/utf8"


@@ 471,7 470,6 @@ const (
// newGlyphID encodes a face and a glyph id into a GlyphID.
func newGlyphID(ppem fixed.Int26_6, faceIdx int, gid font.GID) GlyphID {
	if gid&^((1<<gidbits)-1) != 0 {
		fmt.Println(gid)
		panic("glyph id out of bounds")
	}
	if faceIdx&^((1<<facebits)-1) != 0 {


@@ 488,7 486,7 @@ func newGlyphID(ppem fixed.Int26_6, faceIdx int, gid font.GID) GlyphID {

// splitGlyphID is the opposite of newGlyphID.
func splitGlyphID(g GlyphID) (fixed.Int26_6, int, font.GID) {
	faceIdx := int(g) >> (gidbits + sizebits)
	faceIdx := int(uint64(g) >> (gidbits + sizebits))
	ppem := fixed.Int26_6((g & ((1<<sizebits - 1) << gidbits)) >> gidbits)
	gid := font.GID(g) & (1<<gidbits - 1)
	return ppem, faceIdx, gid