Skip to content

Commit

Permalink
Tx fixes (adobe-type-tools#397)
Browse files Browse the repository at this point in the history 
* [tx][varread] Fix issue adobe-type-tools#272: Fix inconsistency of use of regionCount variable in tx.varread.varread.c

This update makes no change in function: it simply renames variables and fields to avoid confusion by developers.

* [tx][tx_test.py] Add test for reading and snapshotting simple CFF2 VF font to test fix for issue adobe-type-tools#272
  • Loading branch information
@readroberts @schriftgestalt
readroberts authored and schriftgestalt committed May 18, 2019
1 parent 66b2889 commit 71a31df
Show file tree
Hide file tree
Showing 7 changed files with 118 additions and 35 deletions.
78 changes: 78 additions & 0 deletions Tests/tx_data/expected_output/test_simple_cff2_vf.pfa
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Binary file added Tests/tx_data/input/test_simple_cff2_vf.otf
View file
Binary file not shown.
13 changes: 13 additions & 0 deletions Tests/tx_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

UFO2_NAME = 'ufo2.ufo'
TYPE1_NAME = 'type1.pfa'
CFF2_SIMPLE_VF_NAME = 'test_simple_cff2_vf.otf'


def _get_expected_path(file_name):
Expand Down Expand Up @@ -241,3 +242,15 @@ def test_dump_6_n_type1():
actual_path = runner(CMD + ['-o', '6', 'n', '-f', TYPE1_NAME])
expected_path = _get_expected_path('type1.dump6n.txt')
assert differ([expected_path, actual_path]) is True


# ----------
# Test CFF2 font functions
# ----------

def test_varread():
# Test read CCF2 VF, write snapshot.
actual_path = runner(CMD + ['-o', 't1', '-f', CFF2_SIMPLE_VF_NAME])
new_file = CFF2_SIMPLE_VF_NAME.rsplit('.', 1)[0] + '.pfa'
expected_path = _get_expected_path(new_file)
assert differ([expected_path, actual_path]) is True
6 changes: 3 additions & 3 deletions afdko/Tools/Programs/public/lib/api/varread.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,13 +187,13 @@ unsigned short var_getIVSRegionCount(var_itemVariationStore ivs);
ivs - a pointer to the IVS data.
*/

unsigned short var_getIVSRegionCountForIndex(var_itemVariationStore ivs, unsigned short vsIndex);
unsigned short var_getIVDRegionCountForIndex(var_itemVariationStore ivs, unsigned short vsIndex);

/* var_getIVSRegionCountForIndex() returns the number of sub-regions applicable to an IVS sub-table.
/* var_getIVDRegionCountForIndex() returns the number of sub-regions applicable to an IVS sub-table.
ivs - a pointer to the IVS data.
vsIndex - IVS sub-table index.
vsIndex - IVS ItemVariationData sub-table index.
*/

long var_getIVSRegionIndices(var_itemVariationStore ivs, unsigned short vsIndex, unsigned short *regionIndices, long regionCount);
Expand Down
18 changes: 7 additions & 11 deletions afdko/Tools/Programs/public/lib/source/cffread/cffread.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ struct cfrCtx_ /* Context */
float *UDV; /* From client */
Fixed ndv[CFF2_MAX_AXES]; /* normalized weight vector */
float scalars[CFF2_MAX_MASTERS]; /* scalar values for regions */
unsigned short regionCount; /* number of all regions */
unsigned short regionListCount; /* number of all regions */
unsigned short regionIndices[CFF2_MAX_MASTERS]; /* region indices for the current vsindex */

unsigned short axisCount;
Expand Down Expand Up @@ -1062,17 +1062,13 @@ static void saveIntArray(cfrCtx h, size_t max, long *cnt, long *array,

static int setNumMasters(cfrCtx h, unsigned short vsindex)
{
int numRegions = h->stack.numRegions = var_getIVSRegionCountForIndex(h->cff2.varStore, vsindex);
int numMasters = h->stack.numRegions = var_getIVDRegionCountForIndex(h->cff2.varStore, vsindex);

if (numRegions > CFF2_MAX_MASTERS) {
message(h, "too many regions %d for vsindex %d", numRegions, vsindex);
numRegions = 0;
}
if (!var_getIVSRegionIndices(h->cff2.varStore, vsindex, h->cff2.regionIndices, (long)numRegions)) {
if (!var_getIVSRegionIndices(h->cff2.varStore, vsindex, h->cff2.regionIndices, h->cff2.regionListCount)) {
message(h, "inconsistent region indices detected in item variation store subtable %d", vsindex);
numRegions = 0;
numMasters = 0;
}
return numRegions;
return numMasters;
}

/* -------------------------------- VarStore ------------------------------- */
Expand Down Expand Up @@ -1102,8 +1098,8 @@ static void readVarStore(cfrCtx h)
h->cff2.varStore = var_loadItemVariationStore(&h->cb.shstm, (unsigned long)vstoreStart, length, 0);
if (!h->cff2.varStore)
return;
h->cff2.regionCount = var_getIVSRegionCount(h->cff2.varStore);
if (h->cff2.regionCount > CFF2_MAX_MASTERS)
h->cff2.regionListCount = var_getIVSRegionCount(h->cff2.varStore);
if (h->cff2.regionListCount > CFF2_MAX_MASTERS)
fatal(h, cfrErrGeometry);
h->top.varStore = h->cff2.varStore;
/* pre-calculate scalars for all regions for the current weight vector */
Expand Down
6 changes: 1 addition & 5 deletions afdko/Tools/Programs/public/lib/source/t2cstr/t2cstr.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1414,11 +1414,7 @@ static void convertToAbsolute(t2cCtx h, float x1, float y1, abfBlendArg* blendAr
static void setNumMasters(t2cCtx h)
{
unsigned short vsindex = h->glyph->info->blendInfo.vsindex;
h->stack.numRegions = var_getIVSRegionCountForIndex(h->aux->varStore, vsindex);
if (h->stack.numRegions > CFF2_MAX_MASTERS) {
message(h, "too many regions %d for vsindex %d", h->stack.numRegions, vsindex);
h->stack.numRegions = 0;
}
h->stack.numRegions = var_getIVDRegionCountForIndex(h->aux->varStore, vsindex);
h->glyph->info->blendInfo.numRegions = h->stack.numRegions;
if (!var_getIVSRegionIndices(h->aux->varStore, vsindex, h->regionIndices, h->stack.numRegions)) {
message(h, "inconsistent region indices detected in item variation store subtable %d", vsindex);
Expand Down
32 changes: 16 additions & 16 deletions afdko/Tools/Programs/public/lib/source/varread/varread.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -824,8 +824,8 @@ unsigned short var_getIVSRegionCount(var_itemVariationStore ivs)
return (unsigned short)ivs->regionList.regionCount;
}

/* get the region count for a given variation store index */
unsigned short var_getIVSRegionCountForIndex(var_itemVariationStore ivs, unsigned short vsIndex)
/* get the region count for the ItemVariationData table with a given variation store index */
unsigned short var_getIVDRegionCountForIndex(var_itemVariationStore ivs, unsigned short vsIndex)
{
if (!ivs || vsIndex >= ivs->dataList.ivdSubtables.cnt)
return 0;
Expand Down Expand Up @@ -946,7 +946,7 @@ static void lookupIndexMap(indexMap *map, unsigned short gid, indexPair *index)
}
}

long var_getIVSRegionIndices(var_itemVariationStore ivs, unsigned short vsIndex, unsigned short *regionIndices, long regionCount)
long var_getIVSRegionIndices(var_itemVariationStore ivs, unsigned short vsIndex, unsigned short *regionIndices, long regionListCount)
{
itemVariationDataSubtableList *dataList = &ivs->dataList;
itemVariationDataSubtable *subtable;
Expand All @@ -957,12 +957,12 @@ long var_getIVSRegionIndices(var_itemVariationStore ivs, unsigned short vsIndex,

subtable = &dataList->ivdSubtables.array[vsIndex];

if (regionCount < subtable->regionIndices.cnt)
if (regionListCount < subtable->regionIndices.cnt)
return 0;

for (i = 0; i < subtable->regionIndices.cnt; i++) {
unsigned short index = subtable->regionIndices.array[i];
if (index >= regionCount) {
if (index >= regionListCount) {
return 0;
}
regionIndices[i] = index;
Expand All @@ -971,7 +971,7 @@ long var_getIVSRegionIndices(var_itemVariationStore ivs, unsigned short vsIndex,
return subtable->regionIndices.cnt;
}

static float var_applyDeltasForIndexPair(ctlSharedStmCallbacks *sscb, var_itemVariationStore ivs, indexPair *pair, float *scalars, long regionCount)
static float var_applyDeltasForIndexPair(ctlSharedStmCallbacks *sscb, var_itemVariationStore ivs, indexPair *pair, float *scalars, long regionListCount)
{
itemVariationDataSubtableList *dataList = &ivs->dataList;
float netAdjustment = .0f;
Expand All @@ -986,7 +986,7 @@ static float var_applyDeltasForIndexPair(ctlSharedStmCallbacks *sscb, var_itemVa
}

subtable = &dataList->ivdSubtables.array[pair->outerIndex];
if (subtable->regionCount > regionCount) {
if (subtable->regionCount > regionListCount) {
sscb->message(sscb, "out of range region count in item variation store subtable");
return netAdjustment;
}
Expand All @@ -998,7 +998,7 @@ static float var_applyDeltasForIndexPair(ctlSharedStmCallbacks *sscb, var_itemVa
return netAdjustment;
}

subRegionCount = var_getIVSRegionIndices(ivs, pair->outerIndex, regionIndices, regionCount);
subRegionCount = var_getIVSRegionIndices(ivs, pair->outerIndex, regionIndices, regionListCount);
if (subRegionCount == 0) {
sscb->message(sscb, "out of range region index found in item variation store subtable");
}
Expand All @@ -1013,7 +1013,7 @@ static float var_applyDeltasForIndexPair(ctlSharedStmCallbacks *sscb, var_itemVa
return netAdjustment;
}

static float var_applyDeltasForGid(ctlSharedStmCallbacks *sscb, var_itemVariationStore ivs, indexMap *map, unsigned short gid, float *scalars, long regionCount)
static float var_applyDeltasForGid(ctlSharedStmCallbacks *sscb, var_itemVariationStore ivs, indexMap *map, unsigned short gid, float *scalars, long regionListCount)
{
indexPair pair;

Expand All @@ -1025,7 +1025,7 @@ static float var_applyDeltasForGid(ctlSharedStmCallbacks *sscb, var_itemVariat
else
lookupIndexMap(map, gid, &pair);

return var_applyDeltasForIndexPair(sscb, ivs, &pair, scalars, regionCount);
return var_applyDeltasForIndexPair(sscb, ivs, &pair, scalars, regionListCount);
}

/* HVAR / vmtx tables */
Expand Down Expand Up @@ -1196,11 +1196,11 @@ int var_lookuphmtx(ctlSharedStmCallbacks *sscb, var_hmtx hmtx, unsigned short ax

/* modify the default metrics if the font has variable font tables */
if (hmtx->ivs && scalars && (axisCount > 0)) {
long regionCount = hmtx->ivs->regionList.regionCount;
long regionListCount = hmtx->ivs->regionList.regionCount;

metrics->width += var_applyDeltasForGid(sscb, hmtx->ivs, &hmtx->widthMap, gid, scalars, regionCount);
metrics->width += var_applyDeltasForGid(sscb, hmtx->ivs, &hmtx->widthMap, gid, scalars, regionListCount);
if (hmtx->lsbMap.offset > 0) /* if side bearing variation data are provided, index map must exist */
metrics->sideBearing += var_applyDeltasForGid(sscb, hmtx->ivs, &hmtx->lsbMap, gid, scalars, regionCount);
metrics->sideBearing += var_applyDeltasForGid(sscb, hmtx->ivs, &hmtx->lsbMap, gid, scalars, regionListCount);
}

return 0;
Expand Down Expand Up @@ -1422,11 +1422,11 @@ int var_lookupvmtx(ctlSharedStmCallbacks *sscb, var_vmtx vmtx, unsigned short ax

/* modify the default metrics if the font has variable font tables */
if (vmtx->ivs && scalars && (axisCount > 0)) {
long regionCount = vmtx->ivs->regionList.regionCount;
long regionListCount = vmtx->ivs->regionList.regionCount;

metrics->width += var_applyDeltasForGid(sscb, vmtx->ivs, &vmtx->widthMap, gid, scalars, regionCount);
metrics->width += var_applyDeltasForGid(sscb, vmtx->ivs, &vmtx->widthMap, gid, scalars, regionListCount);
if (vmtx->tsbMap.offset > 0) /* if side bearing variation data are provided, index map must exist */
metrics->sideBearing += var_applyDeltasForGid(sscb, vmtx->ivs, &vmtx->tsbMap, gid, scalars, regionCount);
metrics->sideBearing += var_applyDeltasForGid(sscb, vmtx->ivs, &vmtx->tsbMap, gid, scalars, regionListCount);
}

return 0;
Expand Down

0 comments on commit 71a31df

Please sign in to comment.