diff --git a/ChangeLog b/ChangeLog index b45aa9024..16e6c7d8d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,56 @@ +2013-09-29 Dave Arnold + + 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 * 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,14 +453,14 @@ * 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 [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 @@ -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 diff --git a/src/cff/cf2hints.c b/src/cff/cf2hints.c index 96bd49f18..3ed714f44 100644 --- 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,12 +1195,16 @@ /* * 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 ); - - cf2_glyphpath_pushPrevElem( glyphpath, - &glyphpath->firstHintMap, - &glyphpath->offsetStart0, - glyphpath->offsetStart1, - TRUE ); + /* 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 ); /* reset state machine */ glyphpath->moveIsPending = TRUE; glyphpath->pathIsOpen = FALSE; + glyphpath->pathIsClosing = FALSE; glyphpath->elemIsQueued = FALSE; } } diff --git a/src/cff/cf2hints.h b/src/cff/cf2hints.h index c4fa922a3..f25d91bf8 100644 --- a/src/cff/cf2hints.h +++ b/src/cff/cf2hints.h @@ -204,6 +204,7 @@ FT_BEGIN_HEADER #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_BEGIN_HEADER 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;