From: Ben Harris <bjh21@bjh21.me.uk>
Date: Wed, 29 Jan 2025 22:10:40 +0000 (+0000)
Subject: Sort elements of the FeatureIndices array by feature tag
X-Git-Tag: bedstead-3.251~10
X-Git-Url: https://www.chiark.greenend.org.uk/ucgi/~bjharris/git?a=commitdiff_plain;h=2cee520a532013788711892f409056761e10187e;p=bedstead.git

Sort elements of the FeatureIndices array by feature tag

The OpenType spec (version 1.9.1) says, "The FeatureRecord array
should be sorted alphabetically by feature tag."  Bedstead was instead
emitting them in the order in which they appeared in the gsub_features
array, which was determined by the order the lookups need to occur in.
In particular, the 'cvXX' lookups need to occur after the 'ssXX'
lookups so as to override them properly.

Fixing this required some care to keep all the indices straight, but I
think I've managed it.
---

diff --git a/bedstead.c b/bedstead.c
index 14b1deb..c32125d 100644
--- a/bedstead.c
+++ b/bedstead.c
@@ -3875,22 +3875,41 @@ docmap(int pid, int eid, int format)
 	printf("</cmap_format_%d>\n", format);
 }
 
+static int
+compare_features_by_tag(const void *va, const void *vb)
+{
+	struct gsub_feature const * const *ap = va, * const *bp = vb;
+
+	return namecmp((*ap)->tag, (*bp)->tag);
+}
+
 static void
 dogsub(void)
 {
 	int i, j;
+	struct gsub_feature const *sorted[lenof(gsub_features)];
 
+	for (i = 0; i < lenof(gsub_features); i++)
+		sorted[i] = &gsub_features[i];
+	qsort(sorted, lenof(sorted), sizeof(sorted[0]),
+	      &compare_features_by_tag);
 	printf("<GSUB>\n");
 	TTXS("Version", "0x00010000");
 	printf("<ScriptList>\n");
+	/*
+	 * The FeatureList should be sorted alphabetically by tag, but
+	 * the LookupList has to be in the order in which the lookups
+	 * should be applied.  The FeatureIndices can be in any order
+	 * we like.  This requires some care with the indexing.
+	 */
 	for (i = 0; i < lenof(gsub_scripts); i++) {
 		printf("<ScriptRecord>\n");
 		TTXS("ScriptTag", gsub_scripts[i].tag);
 		printf("<Script><DefaultLangSys>\n");
 		TTXI("ReqFeatureIndex", 0xffff); /* No required feature. */
-		for (j = 0; j < lenof(gsub_features); j++)
-			if (gsub_features[j].scripts & gsub_scripts[i].flag) {
-				printf("<!-- %s --> ", gsub_features[j].tag);
+		for (j = 0; j < lenof(sorted); j++)
+			if (sorted[j]->scripts & gsub_scripts[i].flag) {
+				printf("<!-- %s --> ", sorted[j]->tag);
 				TTXI("FeatureIndex", j);
 			}
 		printf("</DefaultLangSys></Script>\n");
@@ -3898,34 +3917,37 @@ dogsub(void)
 	}
 	printf("</ScriptList>\n");
 	printf("<FeatureList>\n");
-	for (i = 0; i < lenof(gsub_features); i++) {
+	for (i = 0; i < lenof(sorted); i++) {
+		struct gsub_feature const *feat = sorted[i];
+		int featidx = (feat - gsub_features);
 		printf("<FeatureRecord>\n");
-		TTXS("FeatureTag", gsub_features[i].tag);
+		TTXS("FeatureTag", feat->tag);
 		printf("<Feature>\n");
-		if (gsub_features[i].name != NULL) {
-			if (gsub_features[i].tag[0] == 's') {
+		if (feat->name != NULL) {
+			if (feat->tag[0] == 's') {
 				printf("<FeatureParamsStylisticSet>\n");
 				TTXI("Version", 0);
-				TTXI("UINameID", NAMEBASE_GSUB + i);
+				TTXI("UINameID", NAMEBASE_GSUB + featidx);
 				printf("</FeatureParamsStylisticSet>\n");
 			} else {
 				int nparam = 0;
 				while (nparam < MAXSUBNAME &&
-				       gsub_features[i].subnames[nparam])
+				       feat->subnames[nparam])
 					nparam++;
 				printf("<FeatureParamsCharacterVariants>\n");
 				TTXI("Format", 0);
-				TTXI("FeatUILabelNameID", NAMEBASE_GSUB + i);
+				TTXI("FeatUILabelNameID",
+				     NAMEBASE_GSUB + featidx);
 				TTXI("FeatUITooltipTextNameID", 0);
 				TTXI("SampleTextNameID", 0);
 				TTXI("NumNamedParameters", nparam);
 				TTXI("FirstParamUILabelNameID",
-				     NAMEBASE_GSUB_SUB + MAXSUBNAME * i);
+				     NAMEBASE_GSUB_SUB + MAXSUBNAME * featidx);
 				printf("</FeatureParamsCharacterVariants>\n");
 			}
 		}
 		/* We only have one GSUB lookup per feature, thankfully. */
-		TTXI("LookupListIndex", i);
+		TTXI("LookupListIndex", featidx);
 		printf("</Feature>\n");
 		printf("</FeatureRecord>\n");
 	}