From 0e5ec18a82e9244b81f0b414cda056c5f6155133 Mon Sep 17 00:00:00 2001
From: Chris Waldon <christopher.waldon.dev@gmail.com>
Date: Fri, 28 Apr 2023 13:39:22 -0400
Subject: [PATCH] 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>
---
 text/gotext_test.go | 50 +++++++++++++++++++++++++++++++++++++++++++++
 text/shaper.go      |  4 +---
 2 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/text/gotext_test.go b/text/gotext_test.go
index 5b633361..fac94b97 100644
--- a/text/gotext_test.go
+++ b/text/gotext_test.go
@@ -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)
+			}
+		})
+	}
+}
diff --git a/text/shaper.go b/text/shaper.go
index 989992f5..6f02701a 100644
--- a/text/shaper.go
+++ b/text/shaper.go
@@ -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