ref: 3a2cb0f881e7dc20b7714d2051254310471edee0
parent: 4f9760e752cb2bf07ea6f934e3a28200edbaba4a
author: Dave Arnold <[email protected]>
date: Sun Sep 29 12:17:02 EDT 2013
Fix Savannah bug #39295. The bug was caused by switching to the initial hintmap (the one in effect when `moveto' executes) just before drawing the final element in the charstring. This ensured that the path was closed (in both Character Space and Device Space). But if the final element was a curve and if the final hintmap was different enough from the initial one, then the curve was visibly distorted. The first part of the fix is to draw the final curve using the final hintmap as specified by the charstring. This corrects the distortion but does not ensure closing in Device Space. It may require the rasterizer to automatically generate an extra closing line. Depending on the hintmap differences, this line could be from zero to a couple pixels in length. The second part of the fix covers the case where the charstring subpath is closed with an explicit line. We now modify that line's end point to avoid the distortion. Some glyphs in the bug report font (TexGyreHeros-Regular) that show the change are: 25ppem S (98) 24ppem eight (52) 25.5ppem p (85) Curves at the *end* of a subpath are no longer distorted. However, some of these glyphs have bad hint substitutions in the middle of a subpath, and these are not affected. The patch has been tested with a set of 106 fonts that shipped with Adobe Creative Suite 4, together with 756 Open Source CFF fonts from Google Fonts. There are 1.5 million glyphs, of which some 20k are changed with the fix. A sampling of a few hundred of these changes have been examined more closely, and the changes look good (or at least acceptable). * src/cff/cf2hints.h (CF2_GlyphPathRec): New element `pathIsClosing' to indicate that we synthesize a closepath line. * src/cff/cf2hints.c (cf2_glyphpath_init): Updated. (cf2_glyphpath_pushPrevElem): If closing, use first hint map (for `lineto' operator) and adjust hint zone. For synthesized closing lines, use end point in first hint zone. (cf2_glyphpath_lineTo): Take care of synthesized closing lines. In particular, shift the detection of zero-length lines from character space to device space. (cf2_glyphpath_closeOpenPath): Remove assertion. Updated.
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,56 @@
+2013-09-29 Dave Arnold <[email protected]>
+
+ Fix Savannah bug #39295.
+
+ The bug was caused by switching to the initial hintmap (the one in
+ effect when `moveto' executes) just before drawing the final element
+ in the charstring. This ensured that the path was closed (in both
+ Character Space and Device Space). But if the final element was a
+ curve and if the final hintmap was different enough from the initial
+ one, then the curve was visibly distorted.
+
+ The first part of the fix is to draw the final curve using the final
+ hintmap as specified by the charstring. This corrects the
+ distortion but does not ensure closing in Device Space. It may
+ require the rasterizer to automatically generate an extra closing
+ line. Depending on the hintmap differences, this line could be from
+ zero to a couple pixels in length.
+
+ The second part of the fix covers the case where the charstring
+ subpath is closed with an explicit line. We now modify that line's
+ end point to avoid the distortion.
+
+ Some glyphs in the bug report font (TexGyreHeros-Regular) that show
+ the change are:
+
+ 25ppem S (98)
+ 24ppem eight (52)
+ 25.5ppem p (85)
+
+ Curves at the *end* of a subpath are no longer distorted. However,
+ some of these glyphs have bad hint substitutions in the middle of a
+ subpath, and these are not affected.
+
+ The patch has been tested with a set of 106 fonts that shipped with
+ Adobe Creative Suite 4, together with 756 Open Source CFF fonts from
+ Google Fonts. There are 1.5 million glyphs, of which some 20k are
+ changed with the fix. A sampling of a few hundred of these changes
+ have been examined more closely, and the changes look good (or at
+ least acceptable).
+
+ * src/cff/cf2hints.h (CF2_GlyphPathRec): New element `pathIsClosing'
+ to indicate that we synthesize a closepath line.
+
+ * src/cff/cf2hints.c (cf2_glyphpath_init): Updated.
+ (cf2_glyphpath_pushPrevElem): If closing, use first hint map (for
+ `lineto' operator) and adjust hint zone.
+ For synthesized closing lines, use end point in first hint zone.
+ (cf2_glyphpath_lineTo): Take care of synthesized closing lines. In
+ particular, shift the detection of zero-length lines from character
+ space to device space.
+ (cf2_glyphpath_closeOpenPath): Remove assertion.
+ Updated.
+
2013-09-25 Werner Lemberg <[email protected]>
* src/autofit/aflatin.c (af_{grek,cyrl}_uniranges): Fix arrays.@@ -226,7 +279,7 @@
Better tracing of loaded glyphs.
- Previously, the loading of a glyph was traced at level 4, if at all.
+ Previously, the loading of a glyph was traced at level 4, if at all.
With this change, all font loading routines emit a tracing message
at level 1, making it easier to select tracing output (for example
using F2_DEBUG="any:1 afhints:7 aflatin:7").
@@ -272,7 +325,7 @@
(af_cjk_metrics_init_blues): Replace AF_CJK_MAX_TEST_CHARACTERS with
AF_BLUE_STRING_MAX_LEN.
Change loops to use offsets (in file `afblue.h') into the new arrays
- `af_blue_stringsets' and `af_blue_strings' (in file `afblue.c').
+ `af_blue_stringsets' and `af_blue_strings' (in file `afblue.c').
Instead of three dimensions (as used in the old blue string array)
we now use properties to do the same, saving one loop nesting level.
@@ -400,7 +453,7 @@
* src/base/ftbbox.c (BBox_Conic_Check): Remove redundant checks for
extremum at the segment ends, which are already within the bbox.
- Slightly modify calculations.
+ Slightly modify calculations.
2013-08-15 Alexei Podtelezhnikov <[email protected]>
@@ -407,7 +460,7 @@
[base] Finish experimental (disabled) BBox_Cubic_Check implementation.
* src/base/ftbbox.c (BBox_Cubic_Check): Scale arguments to improve
- accuracy and avoid overflows.
+ accuracy and avoid overflows.
2013-08-13 Alexei Podtelezhnikov <[email protected]>
@@ -742,10 +795,10 @@
| |
o---------------o
- as round. (`o' and `x' are on and off points, respectively).
+ as round. (`o' and `x' are on and off points, respectively).
This is a major change which should improve the rendering results
- enormously for many TrueType fonts, especially in the range approx.
+ enormously for many TrueType fonts, especially in the range approx.
20-40ppem, fixing the appearance of many overshoots.
* src/autofit/aflatin.c (af_latin_metrics_init_blues): Look at the
@@ -826,7 +879,7 @@
[smooth] Improve performance.
- Provide a work-around for an ARM-specific performance bug in GCC.
+ Provide a work-around for an ARM-specific performance bug in GCC.
This speeds up the rasterizer by more than 5%.
Also slightly optimize `set_gray_cell' and `gray_record_cell' (which
@@ -1588,7 +1641,7 @@
[sfnt] Clean up bitmap code.
* src/sfnt/ttsbit.c: Deleted.
- * src/sfnt/ttsbit0.c: Renamed to `ttsbit.c'.
+ * src/sfnt/ttsbit0.c: Renamed to `ttsbit.c'.
* rules.mk (SFNT_DRV_H): Updated.
2013-05-10 Werner Lemberg <[email protected]>
--- a/src/cff/cf2hints.c
+++ b/src/cff/cf2hints.c
@@ -645,8 +645,27 @@
firstHintEdge->csCoord );
}
- /* discard any hints that overlap in device space; this can occur */
- /* because locked hints have been moved to align with blue zones */
+ /*
+ * Discard any hints that overlap in device space; this can occur
+ * because locked hints have been moved to align with blue zones.
+ *
+ * TODO: Although we might correct this later during adjustment, we
+ * don't currently have a way to delete a conflicting hint once it has
+ * been inserted. See v2.030 MinionPro-Regular, 12 ppem darkened,
+ * initial hint map for second path, glyph 945 (the perispomeni (tilde)
+ * in U+1F6E, Greek omega with psili and perispomeni). Darkening is
+ * 25. Pair 667,747 initially conflicts in design space with top edge
+ * 660. This is because 667 maps to 7.87, and the top edge was
+ * captured by a zone at 8.0. The pair is later successfully inserted
+ * in a zone without the top edge. In this zone it is adjusted to 8.0,
+ * and no longer conflicts with the top edge in design space. This
+ * means it can be included in yet a later zone which does have the top
+ * edge hint. This produces a small mismatch between the first and
+ * last points of this path, even though the hint masks are the same.
+ * The density map difference is tiny (1/256).
+ *
+ */
+
if ( indexInsert > 0 )
{/* we are inserting after an existing edge */
@@ -1035,6 +1054,7 @@
glyphpath->moveIsPending = TRUE;
glyphpath->pathIsOpen = FALSE;
+ glyphpath->pathIsClosing = FALSE;
glyphpath->elemIsQueued = FALSE;
}
@@ -1175,13 +1195,17 @@
/*
* Push the cached element (glyphpath->prevElem*) to the outline
* consumer. When a darkening offset is used, the end point of the
- * cached element may be adjusted to an intersection point or it may be
- * connected by a line to the current element. This calculation must
- * use a HintMap that was valid at the time the element was saved. For
- * the first point in a subpath, that is a saved HintMap. For most
- * elements, it just means the caller has delayed building a HintMap
- * from the current HintMask.
+ * cached element may be adjusted to an intersection point or we may
+ * synthesize a connecting line to the current element. If we are
+ * closing a subpath, we may also generate a connecting line to the start
+ * point.
*
+ * This is where Character Space (CS) is converted to Device Space (DS)
+ * using a hint map. This calculation must use a HintMap that was valid
+ * at the time the element was saved. For the first point in a subpath,
+ * that is a saved HintMap. For most elements, it just means the caller
+ * has delayed building a HintMap from the current HintMask.
+ *
* Transform each point with outerTransform and call the outline
* callbacks. This is a general 3x3 transform:
*
@@ -1250,16 +1274,32 @@
params.op = CF2_PathOpLineTo;
/* note: pt2 and pt3 are unused */
- cf2_glyphpath_hintPoint( glyphpath,
- hintmap,
- ¶ms.pt1,
- glyphpath->prevElemP1.x,
- glyphpath->prevElemP1.y );
- glyphpath->callbacks->lineTo( glyphpath->callbacks, ¶ms );
+ if ( close )
+ {+ /* use first hint map if closing */
+ cf2_glyphpath_hintPoint( glyphpath,
+ &glyphpath->firstHintMap,
+ ¶ms.pt1,
+ glyphpath->prevElemP1.x,
+ glyphpath->prevElemP1.y );
+ }
+ else
+ {+ cf2_glyphpath_hintPoint( glyphpath,
+ hintmap,
+ ¶ms.pt1,
+ glyphpath->prevElemP1.x,
+ glyphpath->prevElemP1.y );
+ }
- glyphpath->currentDS = params.pt1;
+ /* output only non-zero length lines */
+ if ( params.pt0.x != params.pt1.x || params.pt0.y != params.pt1.y )
+ {+ glyphpath->callbacks->lineTo( glyphpath->callbacks, ¶ms );
+ glyphpath->currentDS = params.pt1;
+ }
break;
case CF2_PathOpCubeTo:
@@ -1296,11 +1336,24 @@
/* note: at the end of a subpath, we might do both, so use `nextP0' */
/* before we change it, below */
- cf2_glyphpath_hintPoint( glyphpath,
- hintmap,
- ¶ms.pt1,
- nextP0->x,
- nextP0->y );
+ if ( close )
+ {+ /* if we are closing the subpath, then nextP0 is in the first */
+ /* hint zone */
+ cf2_glyphpath_hintPoint( glyphpath,
+ &glyphpath->firstHintMap,
+ ¶ms.pt1,
+ nextP0->x,
+ nextP0->y );
+ }
+ else
+ {+ cf2_glyphpath_hintPoint( glyphpath,
+ hintmap,
+ ¶ms.pt1,
+ nextP0->x,
+ nextP0->y );
+ }
if ( params.pt1.x != glyphpath->currentDS.x ||
params.pt1.y != glyphpath->currentDS.y )
@@ -1511,6 +1564,16 @@
}
+ /*
+ * The functions cf2_glyphpath_{moveTo,lineTo,curveTo,closeOpenPath} are+ * called by the interpreter with Character Space (CS) coordinates. Each
+ * path element is placed into a queue of length one to await the
+ * calculation of the following element. At that time, the darkening
+ * offset of the following element is known and joins can be computed,
+ * including possible modification of this element, before mapping to
+ * Device Space (DS) and passing it on to the outline consumer.
+ *
+ */
FT_LOCAL_DEF( void )
cf2_glyphpath_moveTo( CF2_GlyphPath glyphpath,
CF2_Fixed x,
@@ -1548,10 +1611,46 @@
{CF2_Fixed xOffset, yOffset;
FT_Vector P0, P1;
+ FT_Bool newHintMap;
+ /*
+ * New hints will be applied after cf2_glyphpath_pushPrevElem has run.
+ * In case this is a synthesized closing line, any new hints should be
+ * delayed until this path is closed (`cf2_hintmask_isNew' will be
+ * called again before the next line or curve).
+ */
- /* can't compute offset of zero length line, so ignore them */
- if ( glyphpath->currentCS.x == x && glyphpath->currentCS.y == y )
+ /* true if new hint map not on close */
+ newHintMap = cf2_hintmask_isNew( glyphpath->hintMask ) &&
+ !glyphpath->pathIsClosing;
+
+ /*
+ * Zero-length lines may occur in the charstring. Because we cannot
+ * compute darkening offsets or intersections from zero-length lines,
+ * it is best to remove them and avoid artifacts. However, zero-length
+ * lines in CS at the start of a new hint map can generate non-zero
+ * lines in DS due to hint substitution. We detect a change in hint
+ * map here and pass those zero-length lines along.
+ */
+
+ /*
+ * Note: Find explicitly closed paths here with a conditional
+ * breakpoint using
+ *
+ * !gp->pathIsClosing && gp->start.x == x && gp->start.y == y
+ *
+ */
+
+ if ( glyphpath->currentCS.x == x &&
+ glyphpath->currentCS.y == y &&
+ !newHintMap )
+ /*
+ * Ignore zero-length lines in CS where the hint map is the same
+ * because the line in DS will also be zero length.
+ *
+ * Ignore zero-length lines when we synthesize a closing line because
+ * the close will be handled in cf2_glyphPath_pushPrevElem.
+ */
return;
cf2_glyphpath_computeOffset( glyphpath,
@@ -1597,7 +1696,7 @@
glyphpath->prevElemP1 = P1;
/* update current map */
- if ( cf2_hintmask_isNew( glyphpath->hintMask ) )
+ if ( newHintMap )
cf2_hintmap_build( &glyphpath->hintMap,
glyphpath->hStemHintArray,
glyphpath->vStemHintArray,
@@ -1703,29 +1802,29 @@
{if ( glyphpath->pathIsOpen )
{- FT_ASSERT( cf2_hintmap_isValid( &glyphpath->firstHintMap ) );
+ /*
+ * A closing line in Character Space line is always generated below
+ * with `cf2_glyphPath_lineTo'. It may be ignored later if it turns
+ * out to be zero length in Device Space.
+ */
+ glyphpath->pathIsClosing = TRUE;
- /* since we need to apply an offset to the implicit lineto, we make */
- /* it explicit here */
cf2_glyphpath_lineTo( glyphpath,
glyphpath->start.x,
glyphpath->start.y );
- /* Draw previous element (the explicit LineTo we just created, */
- /* above) and connect it to the start point, but with the offset we */
- /* saved from the first element. */
- /* Use the saved HintMap, too. */
- FT_ASSERT( glyphpath->elemIsQueued );
+ /* empty the final element from the queue and close the path */
+ if ( glyphpath->elemIsQueued )
+ cf2_glyphpath_pushPrevElem( glyphpath,
+ &glyphpath->hintMap,
+ &glyphpath->offsetStart0,
+ glyphpath->offsetStart1,
+ TRUE );
- cf2_glyphpath_pushPrevElem( glyphpath,
- &glyphpath->firstHintMap,
- &glyphpath->offsetStart0,
- glyphpath->offsetStart1,
- TRUE );
-
/* reset state machine */
glyphpath->moveIsPending = TRUE;
glyphpath->pathIsOpen = FALSE;
+ glyphpath->pathIsClosing = FALSE;
glyphpath->elemIsQueued = FALSE;
}
}
--- a/src/cff/cf2hints.h
+++ b/src/cff/cf2hints.h
@@ -204,6 +204,7 @@
#endif
FT_Bool pathIsOpen; /* true after MoveTo */
+ FT_Bool pathIsClosing; /* true when synthesizing closepath line */
FT_Bool darken; /* true if stem darkening */
FT_Bool moveIsPending; /* true between MoveTo and offset MoveTo */
@@ -229,7 +230,8 @@
FT_Vector currentCS;
/* current point, device space */
FT_Vector currentDS;
- FT_Vector start; /* start point of subpath */
+ /* start point of subpath, character space */
+ FT_Vector start;
/* the following members constitute the `queue' of one element */
FT_Bool elemIsQueued;