fontforge/fontforge — pull request #1431 by tshinnic (2014-06-18 02:41:18+00:00)
Title: Imported SVG path cmd "close path" skipped sometimes

TL;DR version:
- a bug could skip some SVG path string "close path" commands;
- that bug required another added remedial misfeature that
forcefully closed all paths, even desired open paths;
- this fix forces command interpretation as in SVG specification,
which may be an unexpected difference from previous results

As this is a possibly breaking change to SVG imports, I wanted to present
the rationales and details behind this small change, but which alters
how SVG path strings are parsed and interpreted. I truly think the fixed
code actions are "more correct" in SVG terms, but may be different
from the results people saw previously when importing from SVG.

There are two aspects to be mentioned that will serve well as background
to the below discussion

Most (?) fonts these days are outline fonts. The character is described
by the enclosed space drawn by outlines, and these outlines are by
definition closed paths.

The SVG world can be mysterious to the occasional visitor. For instance,
strokes that have no width until one is specified, no visible appearance
until the stroke color (!) is defined, a partial set of primitive shapes
and even more primitive set of drawing commands describing 'paths'.

FontForge naturally favors thinking about characters as outlines and that
becomes the dominant mindset. While there is provision for handling
stroked characters, it is as an add-on to the "main event".

Adding support for importing SVG files and elements must have been yet
another example of GW helping someone to use yet another description of
glyphs. As much as was needed of the SVG spec was coded up - enough to
get data from, say, Inkscape, into FontForge for the real work of
generating useful fonts.

With the emphasis on outlines, the focus on and satisfaction with "do
the glyphs look right", and the reference in passing to SVG, it is not
unreasonable to imagine that a misunderstanding or two might have been
coded in and lingered on in the fontforge/svg.c source file.

I needed to import an SVG font file that contained stroked characters.
Stroked characters, because the goal 1000+ abstract characters[1] were
defined using a fairly small set of rigidly-defined subcomponents, which
were combined using a simple SVG application. The subcomponents were
best described as simple stroked shapes. Indeed, the end-user
non-profit will undoubtedly want to revise stylistic issues - something
best done from the ground subcomponents on up, and easily done with the
application (which tracked the derivation of characters from components).

I hit a problem right away with FontForge where certain subcomponent
drawings were distorted upon import. An abstract character which looks
much like "<<", two open angles pointing left, became instead one closed
triangle followed by an open angle.

FontForge was forcing open paths to be closed... sometimes.

Routine SVGParsePath() in fontforge/svg.c has some quite strange code at
the beginning when handling the SVG path move commands, 'M' and 'm'. If
there was a previous path being drawn by this path string, it checks how
near the current point is to the very first point of the path.

If the ending point is near enough to the path's starting point, the
code says "ah, this must have been meant to be a closed path, where this
latest line joins with the first line", and does some magic to patch the
path so that the ending point is moved to the starting point - forcing a
closed path. This could be considered reasonable, and certainly better
for an outlined character to not have an unintended gap destroying the
desired glyph.

Otherwise, if the ending point is not close to the starting point, the
code does something quite overt. The code *creates* a *new* line joining
the end point with the starting point. If the SVG path was not closed,
FontForge forces it closed by introducing a line not present in the SVG
commands. Hey, go for broke, we want a closed path for this outlined
character!

Except, it may not be a character in outline form, but a plain SVG
drawing made up of strokes, and with some open paths.

My first fix was to remove the code creating a new line to forcing all
open paths closed. This worked, and checking the "<<" glyph now showed
two open angles as desired.

But other glyphs were now distorted. Quite infuriating, for now a few
glyphs previously closed paths were now missing a line... sometimes.

More investigation found a truly odd bit of code, which looked very much
like a misunderstanding of SVG path commands. A bug, in fact, but that
had produced the *need* for the above-mentioned strange actions when
handling 'M' and 'm' move commands. A bug engendering a another bug -
we've all done this before! :-)

SVG path strings are intentionally succinct[2] and commands are
single letters such as 'M', 'l', 'z', etc. Almost every command
takes number parameters such as x/y coordinates or radius values.
The only command not to take parameters is the "close path" command
in the 'Z' and 'z' variants.

Path strings may be simplified by omitting unnecessary spacing.
No space is needed between a command letter and following number,
nor between a number and the next command letter in the string.
(Consecutive numbers must be separated by spaces/comma).

The single parsing ambiguity is created by 'Z' not having a
following number. What is required when a 'Z' is followed by
another command? Checking the grammar in the SVG 1.1 specification
finds that it is not required to have any separation between a
'Z' and the next command letter.

Indeed, it can be seen that FontForge itself generates path
strings without such a separator, such as this example from an
output SVG file:
d="M200 600l-100 200l-100 -200h200zM400 600l-100 200l-100 -200h200z"

In svg.c routine SVGParsePath() there is a loop that parses an
SVG path string. At the top of the loop the code is looking
for the next command.

If a command letter is _not_ seen then the next number parameters
will be processed as part of the previous command remembered in
variable 'type'. (This is another path string simplification -
more points can be specified without having to repeat the command
letter).

If a command letter _is_ seen then variable 'type' is set to that
letter value, which guides how following parameters are processed
in the remainder of the loop's code.

However, the check for command letter is done using:
while ( isalpha(*path))
type = *path++;
This has the unwanted effect that only the last command letter of
consecutive command letters will be retained. In the legal case
of "ZM" the 'Z' command will be ignored. A desired "path close"
command will be ignored.

And note that the 'Z' command as found at the _end_ of many path
strings _will_ be acted upon. The final shape drawn _will_ be
closed as desired. It is earlier shapes may not be closed as
desired. :-(

The code should read:
if ( isalpha(*path)) {
type = *path++;
}
This forces the parsing loop to act upon every drawing command
found in the SVG path string.

Once this initial bug is corrected the accidental need to force
open paths to be closed is gone. A path requested closed through
specifying the "close path" command _will_ be closed.

And now a path that was supposed to remain an open path, *because*
no "close path" command was specified, _will_ remain an open path.

[1] http://en.wikipedia.org/wiki/Blissymbols
[2] http://www.w3.org/TR/SVG/paths.html#PathData