Skip to content

Commit

Permalink
[CFF] Extract BlueValues as Fixed rather than Int.
Browse files Browse the repository at this point in the history 
This is a follow-up to commit 26a7f04,

  [cff] Make blend operator work with floats in private dicts.

which addressed the 'party baseline' bug.  However, the reporting user
indicated that the default location and some other points in design space
rendered OK, but other points in design space still had problems.  The most
obvious issue being that the x-heights of lower-case letters did not align;
see

  adobe-fonts/source-serif#121 (comment)

After some analysis we determined that this was due to an interaction
between `BlueValue` rounding and the zone-based algorithm.  In short, for a
point to be considered in a zone it must fall within the bounds of the zone.
(There is a slop factor in some cases, but only a very small one.)  In the
Adobe-contributed side of the code, point values are not integer-rounded,
instead they're kept as (some form of) fixed.  Rounding just the `BlueValues`
means that points that need to be considered within a zone will fall outside
of it at some points in design space.

The majority of this patch changes the storage and parsing of `BlueValues`
to keep them as `FT_Fixed`.  No significant code changes were needed because
the values are converted to `Fixed` anyway when stored in `CF_BlueRec`.  No
attempt was made to address problems in the older pshinter code beyond
converting the values from `FT_Fixed` to `FT_Short` when copying the private
dictionary.  (However, as the point values are also rounded in that code,
the problem is much less likely to occur, although inconsistency between
rounding and truncation could cause an analogous problem.)

* include/freetype/internal/cfftypes.h (CFF_PrivateRec): Use `FT_Fixed` for
`blue_values`, `other_blues`, `family_blues`, and `family_other_blues`.

* src/cff/cffload.c (cff_blend_doBlend): Updated.

* src/cff/cffobjs.c (CFF_fixedToInt): New macro.
(cff_make_private_dict): Use it.

* src/cff/cffparse.h (cff_kind_delta_fixed): New enum value.

* src/cff/cffparse.c (do_fixed): Updated.
(CFF_FIELD_DELTA, CFF_FIELD_DELTA_FIXED, CFF_DELTA_KIND): New set of macros,
replacing `CFF_FIELD_DELTA`.
(cff_parser_run): Updated to handle fixed-float deltas.

* src/cff/cfftoken.h: Updated to use `CFF_FIELD_DELTA_FIXED` for blue
values.

* src/psaux/psblues.c (cf2_blueToFixed): Removed, no longer needed.
(cf2_blues_init): Updated.

* src/pxaux/psft.c, src/pxaux/psft.h (cf2_getBlueValues, cf2_getOtherBlues,
cf2_getFamilyBlues, cf2_getFamilyOtherBlues): Updated signatures.

* src/psaux/psobjs.c (t1_make_subfont): Updated.
  • Loading branch information
@skef @devshgraphicsprogramming
skef authored and devshgraphicsprogramming committed May 7, 2024
1 parent 9bf6dfd commit 77017f0
Show file tree
Hide file tree
Showing 10 changed files with 149 additions and 119 deletions.
8 changes: 4 additions & 4 deletions include/freetype/internal/cfftypes.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,10 +248,10 @@ FT_BEGIN_HEADER
FT_Byte num_family_blues;
FT_Byte num_family_other_blues;

FT_Pos blue_values[14];
FT_Pos other_blues[10];
FT_Pos family_blues[14];
FT_Pos family_other_blues[10];
FT_Fixed blue_values[14];
FT_Fixed other_blues[10];
FT_Fixed family_blues[14];
FT_Fixed family_other_blues[10];

FT_Fixed blue_scale;
FT_Pos blue_shift;
Expand Down
8 changes: 4 additions & 4 deletions src/cff/cffload.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1379,10 +1379,10 @@
/* opcode in both CFF and CFF2 DICTs. See `cff_parse_num' for */
/* decode of this, which rounds to an integer. */
*subFont->blend_top++ = 255;
*subFont->blend_top++ = (FT_Byte)( sum >> 24 );
*subFont->blend_top++ = (FT_Byte)( sum >> 16 );
*subFont->blend_top++ = (FT_Byte)( sum >> 8 );
*subFont->blend_top++ = (FT_Byte)sum;
*subFont->blend_top++ = (FT_Byte)( (FT_UInt32)sum >> 24 );
*subFont->blend_top++ = (FT_Byte)( (FT_UInt32)sum >> 16 );
*subFont->blend_top++ = (FT_Byte)( (FT_UInt32)sum >> 8 );
*subFont->blend_top++ = (FT_Byte)( (FT_UInt32)sum );
}

/* leave only numBlends results on parser stack */
Expand Down
11 changes: 7 additions & 4 deletions src/cff/cffobjs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@
#include <freetype/internal/psaux.h>
#include <freetype/internal/services/svcfftl.h>

#define CFF_fixedToInt( x ) \
( (FT_Short)( ( (x) + 0x8000U ) >> 16 ) )

/**************************************************************************
*
Expand Down Expand Up @@ -124,19 +126,20 @@

count = priv->num_blue_values = cpriv->num_blue_values;
for ( n = 0; n < count; n++ )
priv->blue_values[n] = (FT_Short)cpriv->blue_values[n];
priv->blue_values[n] = CFF_fixedToInt( cpriv->blue_values[n] );

count = priv->num_other_blues = cpriv->num_other_blues;
for ( n = 0; n < count; n++ )
priv->other_blues[n] = (FT_Short)cpriv->other_blues[n];
priv->other_blues[n] = CFF_fixedToInt( cpriv->other_blues[n] );

count = priv->num_family_blues = cpriv->num_family_blues;
for ( n = 0; n < count; n++ )
priv->family_blues[n] = (FT_Short)cpriv->family_blues[n];
priv->family_blues[n] = CFF_fixedToInt( cpriv->family_blues[n] );

count = priv->num_family_other_blues = cpriv->num_family_other_blues;
for ( n = 0; n < count; n++ )
priv->family_other_blues[n] = (FT_Short)cpriv->family_other_blues[n];
priv->family_other_blues[n] =
CFF_fixedToInt( cpriv->family_other_blues[n] );

priv->blue_scale = cpriv->blue_scale;
priv->blue_shift = (FT_Int)cpriv->blue_shift;
Expand Down
87 changes: 62 additions & 25 deletions src/cff/cffparse.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -501,10 +501,10 @@
return cff_parse_real( *d, parser->limit, scaling, NULL );
else if ( **d == 255 )
{
FT_Fixed val = ( ( ( (FT_UInt32)*( d[0] + 1 ) << 24 ) |
( (FT_UInt32)*( d[0] + 2 ) << 16 ) |
( (FT_UInt32)*( d[0] + 3 ) << 8 ) |
(FT_UInt32)*( d[0] + 4 ) ) );
FT_Fixed val = (FT_Int32)( ( ( (FT_UInt32)*( d[0] + 1 ) << 24 ) |
( (FT_UInt32)*( d[0] + 2 ) << 16 ) |
( (FT_UInt32)*( d[0] + 3 ) << 8 ) |
(FT_UInt32)*( d[0] + 4 ) ) );

if ( scaling )
{
Expand Down Expand Up @@ -1031,10 +1031,14 @@
CFF_FIELD( code, name, id, cff_kind_string )
#define CFF_FIELD_BOOL( code, name, id ) \
CFF_FIELD( code, name, id, cff_kind_bool )
#define CFF_FIELD_DELTA( code, name, max, id ) \
CFF_FIELD_DELTA_KIND( code, name, max, id, cff_kind_delta )
#define CFF_FIELD_DELTA_FIXED( code, name, max, id ) \
CFF_FIELD_DELTA_KIND( code, name, max, id, cff_kind_delta_fixed )


#undef CFF_FIELD
#undef CFF_FIELD_DELTA
#undef CFF_FIELD_DELTA_KIND


#ifndef FT_DEBUG_LEVEL_TRACE
Expand Down Expand Up @@ -1067,15 +1071,15 @@
NULL, 0, 0 \
},

#define CFF_FIELD_DELTA( code, name, max, id ) \
{ \
cff_kind_delta, \
code | CFFCODE, \
FT_FIELD_OFFSET( name ), \
FT_FIELD_SIZE_DELTA( name ), \
NULL, \
max, \
FT_FIELD_OFFSET( num_ ## name ) \
#define CFF_FIELD_DELTA_KIND( code, name, max, id, kind ) \
{ \
kind, \
code | CFFCODE, \
FT_FIELD_OFFSET( name ), \
FT_FIELD_SIZE_DELTA( name ), \
NULL, \
max, \
FT_FIELD_OFFSET( num_ ## name ) \
},

static const CFF_Field_Handler cff_field_handlers[] =
Expand Down Expand Up @@ -1121,16 +1125,16 @@
id \
},

#define CFF_FIELD_DELTA( code, name, max, id ) \
{ \
cff_kind_delta, \
code | CFFCODE, \
FT_FIELD_OFFSET( name ), \
FT_FIELD_SIZE_DELTA( name ), \
NULL, \
max, \
FT_FIELD_OFFSET( num_ ## name ), \
id \
#define CFF_FIELD_DELTA_KIND( code, name, max, id, kind ) \
{ \
kind, \
code | CFFCODE, \
FT_FIELD_OFFSET( name ), \
FT_FIELD_SIZE_DELTA( name ), \
NULL, \
max, \
FT_FIELD_OFFSET( num_ ## name ), \
id \
},

static const CFF_Field_Handler cff_field_handlers[] =
Expand Down Expand Up @@ -1356,7 +1360,8 @@

/* check that we have enough arguments -- except for */
/* delta encoded arrays, which can be empty */
if ( field->kind != cff_kind_delta && num_args < 1 )
if ( field->kind != cff_kind_delta &&
field->kind != cff_kind_delta_fixed && num_args < 1 )
goto Stack_Underflow;

switch ( field->kind )
Expand Down Expand Up @@ -1471,6 +1476,38 @@
}
break;

case cff_kind_delta_fixed:
{
FT_Byte* qcount = (FT_Byte*)parser->object +
field->count_offset;

FT_Byte** data = parser->stack;


if ( num_args > field->array_max )
num_args = field->array_max;

FT_TRACE4(( " [" ));

/* store count */
*qcount = (FT_Byte)num_args;

val = 0;
while ( num_args > 0 )
{
val = ADD_LONG( val, cff_parse_fixed( parser, data++ ) );
*(FT_Long*)q = val;

FT_TRACE4(( " %f\n", (double)val / 65536 ));

q += field->size;
num_args--;
}

FT_TRACE4(( "]\n" ));
}
break;

default: /* callback or blend */
error = field->reader( parser );
if ( error )
Expand Down
1 change: 1 addition & 0 deletions src/cff/cffparse.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ FT_BEGIN_HEADER
cff_kind_string,
cff_kind_bool,
cff_kind_delta,
cff_kind_delta_fixed,
cff_kind_callback,
cff_kind_blend,

Expand Down
72 changes: 36 additions & 36 deletions src/cff/cfftoken.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,26 +80,26 @@
#undef CFFCODE
#define CFFCODE CFF_CODE_PRIVATE

CFF_FIELD_DELTA ( 6, blue_values, 14, "BlueValues" )
CFF_FIELD_DELTA ( 7, other_blues, 10, "OtherBlues" )
CFF_FIELD_DELTA ( 8, family_blues, 14, "FamilyBlues" )
CFF_FIELD_DELTA ( 9, family_other_blues, 10, "FamilyOtherBlues" )
CFF_FIELD_FIXED_1000( 0x109, blue_scale, "BlueScale" )
CFF_FIELD_NUM ( 0x10A, blue_shift, "BlueShift" )
CFF_FIELD_NUM ( 0x10B, blue_fuzz, "BlueFuzz" )
CFF_FIELD_NUM ( 10, standard_width, "StdHW" )
CFF_FIELD_NUM ( 11, standard_height, "StdVW" )
CFF_FIELD_DELTA ( 0x10C, snap_widths, 13, "StemSnapH" )
CFF_FIELD_DELTA ( 0x10D, snap_heights, 13, "StemSnapV" )
CFF_FIELD_BOOL ( 0x10E, force_bold, "ForceBold" )
CFF_FIELD_FIXED ( 0x10F, force_bold_threshold, "ForceBoldThreshold" )
CFF_FIELD_NUM ( 0x110, lenIV, "lenIV" )
CFF_FIELD_NUM ( 0x111, language_group, "LanguageGroup" )
CFF_FIELD_FIXED ( 0x112, expansion_factor, "ExpansionFactor" )
CFF_FIELD_NUM ( 0x113, initial_random_seed, "initialRandomSeed" )
CFF_FIELD_NUM ( 19, local_subrs_offset, "Subrs" )
CFF_FIELD_NUM ( 20, default_width, "defaultWidthX" )
CFF_FIELD_NUM ( 21, nominal_width, "nominalWidthX" )
CFF_FIELD_DELTA_FIXED( 6, blue_values, 14, "BlueValues" )
CFF_FIELD_DELTA_FIXED( 7, other_blues, 10, "OtherBlues" )
CFF_FIELD_DELTA_FIXED( 8, family_blues, 14, "FamilyBlues" )
CFF_FIELD_DELTA_FIXED( 9, family_other_blues, 10, "FamilyOtherBlues" )
CFF_FIELD_FIXED_1000 ( 0x109, blue_scale, "BlueScale" )
CFF_FIELD_NUM ( 0x10A, blue_shift, "BlueShift" )
CFF_FIELD_NUM ( 0x10B, blue_fuzz, "BlueFuzz" )
CFF_FIELD_NUM ( 10, standard_width, "StdHW" )
CFF_FIELD_NUM ( 11, standard_height, "StdVW" )
CFF_FIELD_DELTA ( 0x10C, snap_widths, 13, "StemSnapH" )
CFF_FIELD_DELTA ( 0x10D, snap_heights, 13, "StemSnapV" )
CFF_FIELD_BOOL ( 0x10E, force_bold, "ForceBold" )
CFF_FIELD_FIXED ( 0x10F, force_bold_threshold, "ForceBoldThreshold" )
CFF_FIELD_NUM ( 0x110, lenIV, "lenIV" )
CFF_FIELD_NUM ( 0x111, language_group, "LanguageGroup" )
CFF_FIELD_FIXED ( 0x112, expansion_factor, "ExpansionFactor" )
CFF_FIELD_NUM ( 0x113, initial_random_seed, "initialRandomSeed" )
CFF_FIELD_NUM ( 19, local_subrs_offset, "Subrs" )
CFF_FIELD_NUM ( 20, default_width, "defaultWidthX" )
CFF_FIELD_NUM ( 21, nominal_width, "nominalWidthX" )


#undef FT_STRUCTURE
Expand Down Expand Up @@ -129,22 +129,22 @@
#undef CFFCODE
#define CFFCODE CFF2_CODE_PRIVATE

CFF_FIELD_DELTA ( 6, blue_values, 14, "BlueValues" )
CFF_FIELD_DELTA ( 7, other_blues, 10, "OtherBlues" )
CFF_FIELD_DELTA ( 8, family_blues, 14, "FamilyBlues" )
CFF_FIELD_DELTA ( 9, family_other_blues, 10, "FamilyOtherBlues" )
CFF_FIELD_FIXED_1000( 0x109, blue_scale, "BlueScale" )
CFF_FIELD_NUM ( 0x10A, blue_shift, "BlueShift" )
CFF_FIELD_NUM ( 0x10B, blue_fuzz, "BlueFuzz" )
CFF_FIELD_NUM ( 10, standard_width, "StdHW" )
CFF_FIELD_NUM ( 11, standard_height, "StdVW" )
CFF_FIELD_DELTA ( 0x10C, snap_widths, 13, "StemSnapH" )
CFF_FIELD_DELTA ( 0x10D, snap_heights, 13, "StemSnapV" )
CFF_FIELD_NUM ( 0x111, language_group, "LanguageGroup" )
CFF_FIELD_FIXED ( 0x112, expansion_factor, "ExpansionFactor" )
CFF_FIELD_CALLBACK ( 22, vsindex, "vsindex" )
CFF_FIELD_BLEND ( 23, "blend" )
CFF_FIELD_NUM ( 19, local_subrs_offset, "Subrs" )
CFF_FIELD_DELTA_FIXED( 6, blue_values, 14, "BlueValues" )
CFF_FIELD_DELTA_FIXED( 7, other_blues, 10, "OtherBlues" )
CFF_FIELD_DELTA_FIXED( 8, family_blues, 14, "FamilyBlues" )
CFF_FIELD_DELTA_FIXED( 9, family_other_blues, 10, "FamilyOtherBlues" )
CFF_FIELD_FIXED_1000 ( 0x109, blue_scale, "BlueScale" )
CFF_FIELD_NUM ( 0x10A, blue_shift, "BlueShift" )
CFF_FIELD_NUM ( 0x10B, blue_fuzz, "BlueFuzz" )
CFF_FIELD_NUM ( 10, standard_width, "StdHW" )
CFF_FIELD_NUM ( 11, standard_height, "StdVW" )
CFF_FIELD_DELTA ( 0x10C, snap_widths, 13, "StemSnapH" )
CFF_FIELD_DELTA ( 0x10D, snap_heights, 13, "StemSnapV" )
CFF_FIELD_NUM ( 0x111, language_group, "LanguageGroup" )
CFF_FIELD_FIXED ( 0x112, expansion_factor, "ExpansionFactor" )
CFF_FIELD_CALLBACK ( 22, vsindex, "vsindex" )
CFF_FIELD_BLEND ( 23, "blend" )
CFF_FIELD_NUM ( 19, local_subrs_offset, "Subrs" )


/* END */
48 changes: 18 additions & 30 deletions src/psaux/psblues.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,6 @@
#define FT_COMPONENT cf2blues


/*
* For blue values, the FreeType parser produces an array of integers,
* while the Adobe CFF engine produces an array of fixed.
* Define a macro to convert FreeType to fixed.
*/
#define cf2_blueToFixed( x ) cf2_intToFixed( x )


FT_LOCAL_DEF( void )
cf2_blues_init( CF2_Blues blues,
CF2_Font font )
Expand All @@ -78,10 +70,10 @@
size_t numFamilyBlues;
size_t numFamilyOtherBlues;

FT_Pos* blueValues;
FT_Pos* otherBlues;
FT_Pos* familyBlues;
FT_Pos* familyOtherBlues;
FT_Fixed* blueValues;
FT_Fixed* otherBlues;
FT_Fixed* familyBlues;
FT_Fixed* familyOtherBlues;

size_t i;
CF2_Fixed emBoxBottom, emBoxTop;
Expand Down Expand Up @@ -138,13 +130,13 @@
emBoxTop = CF2_ICF_Top;
}

if ( cf2_getLanguageGroup( decoder ) == 1 &&
( numBlueValues == 0 ||
( numBlueValues == 4 &&
cf2_blueToFixed( blueValues[0] ) < emBoxBottom &&
cf2_blueToFixed( blueValues[1] ) < emBoxBottom &&
cf2_blueToFixed( blueValues[2] ) > emBoxTop &&
cf2_blueToFixed( blueValues[3] ) > emBoxTop ) ) )
if ( cf2_getLanguageGroup( decoder ) == 1 &&
( numBlueValues == 0 ||
( numBlueValues == 4 &&
blueValues[0] < emBoxBottom &&
blueValues[1] < emBoxBottom &&
blueValues[2] > emBoxTop &&
blueValues[3] > emBoxTop ) ) )
{
/*
* Construct hint edges suitable for synthetic ghost hints at top
Expand Down Expand Up @@ -189,10 +181,8 @@
/* bottom zones */
for ( i = 0; i < numBlueValues; i += 2 )
{
blues->zone[blues->count].csBottomEdge =
cf2_blueToFixed( blueValues[i] );
blues->zone[blues->count].csTopEdge =
cf2_blueToFixed( blueValues[i + 1] );
blues->zone[blues->count].csBottomEdge = blueValues[i];
blues->zone[blues->count].csTopEdge = blueValues[i + 1];

zoneHeight = SUB_INT32( blues->zone[blues->count].csTopEdge,
blues->zone[blues->count].csBottomEdge );
Expand Down Expand Up @@ -238,10 +228,8 @@

for ( i = 0; i < numOtherBlues; i += 2 )
{
blues->zone[blues->count].csBottomEdge =
cf2_blueToFixed( otherBlues[i] );
blues->zone[blues->count].csTopEdge =
cf2_blueToFixed( otherBlues[i + 1] );
blues->zone[blues->count].csBottomEdge = otherBlues[i];
blues->zone[blues->count].csTopEdge = otherBlues[i + 1];

zoneHeight = SUB_INT32( blues->zone[blues->count].csTopEdge,
blues->zone[blues->count].csBottomEdge );
Expand Down Expand Up @@ -299,7 +287,7 @@
for ( j = 0; j < numFamilyOtherBlues; j += 2 )
{
/* top edge */
flatFamilyEdge = cf2_blueToFixed( familyOtherBlues[j + 1] );
flatFamilyEdge = familyOtherBlues[j + 1];

diff = cf2_fixedAbs( SUB_INT32( flatEdge, flatFamilyEdge ) );

Expand All @@ -317,7 +305,7 @@
if ( numFamilyBlues >= 2 )
{
/* top edge */
flatFamilyEdge = cf2_blueToFixed( familyBlues[1] );
flatFamilyEdge = familyBlues[1];

diff = cf2_fixedAbs( SUB_INT32( flatEdge, flatFamilyEdge ) );

Expand All @@ -337,7 +325,7 @@
for ( j = 2; j < numFamilyBlues; j += 2 )
{
/* bottom edge */
flatFamilyEdge = cf2_blueToFixed( familyBlues[j] );
flatFamilyEdge = familyBlues[j];

/* adjust edges of top zone upward by twice darkening amount */
flatFamilyEdge += 2 * font->darkenY; /* bottom edge */
Expand Down
Loading

0 comments on commit 77017f0

Please sign in to comment.