Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SVG import: Fontforge adds a ton of extra points whenever circles are involved #1702

Closed
tayloraswift opened this issue Aug 31, 2014 · 10 comments · Fixed by #1708
Closed

SVG import: Fontforge adds a ton of extra points whenever circles are involved #1702

tayloraswift opened this issue Aug 31, 2014 · 10 comments · Fixed by #1708

Comments

@tayloraswift
Copy link

When copying and pasting a glyph from Inkscape that has an svg circle in it (like the tittle on the 'i'), Fontforge will distort the circle and add a bunch of unnecessary points to the other paths copied with it—like the stem of the 'i'. The problem gets worse the larger the glyph is scaled. If you convert the circle to a path in Inkscape, the glyph imports fine.
screenshot 060

Here's a test svg with two 'i's—they're identical except the blue one has an svg circle as its tittle. (Sorry apparently github doesn't let you upload svgs)

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<!-- Created with Inkscape (http://www.inkscape.org/) -->

<svg
   xmlns:dc="http://purl.org/dc/elements/1.1/"
   xmlns:cc="http://creativecommons.org/ns#"
   xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
   xmlns:svg="http://www.w3.org/2000/svg"
   xmlns="http://www.w3.org/2000/svg"
   xmlns:sodipodi="http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd"
   xmlns:inkscape="http://www.inkscape.org/namespaces/inkscape"
   width="1264.8572"
   height="1495.954"
   id="svg11173"
   version="1.1"
   inkscape:version="0.91pre2 r"
   viewBox="0 0 1264.8572 1495.954"
   sodipodi:docname="test i.svg">
  <defs
     id="defs11175" />
  <sodipodi:namedview
     id="base"
     pagecolor="#ffffff"
     bordercolor="#666666"
     borderopacity="1.0"
     inkscape:pageopacity="0.0"
     inkscape:pageshadow="2"
     inkscape:zoom="0.49497475"
     inkscape:cx="356.02968"
     inkscape:cy="719.12431"
     inkscape:document-units="px"
     inkscape:current-layer="layer1"
     showgrid="false"
     inkscape:snap-nodes="false"
     fit-margin-top="0"
     fit-margin-left="0"
     fit-margin-right="0"
     fit-margin-bottom="0"
     inkscape:window-width="1920"
     inkscape:window-height="1176"
     inkscape:window-x="0"
     inkscape:window-y="24"
     inkscape:window-maximized="1" />
  <metadata
     id="metadata11178">
    <rdf:RDF>
      <cc:Work
     rdf:about="">
    <dc:format>image/svg+xml</dc:format>
    <dc:type
       rdf:resource="http://purl.org/dc/dcmitype/StillImage" />
    <dc:title></dc:title>
      </cc:Work>
    </rdf:RDF>
  </metadata>
  <g
     inkscape:label="Layer 1"
     inkscape:groupmode="layer"
     id="layer1"
     transform="translate(-117.10462,442.11756)">
    <g
       id="g11822"
       style="fill:#6bfaff;fill-opacity:1">
      <ellipse
     ry="116.04749"
     rx="115.66302"
     style="opacity:1;fill:#6bfaff;fill-opacity:1;fill-rule:nonzero;stroke:none;stroke-width:3;stroke-linecap:butt;stroke-linejoin:miter;stroke-miterlimit:4;stroke-dasharray:none;stroke-dashoffset:0;stroke-opacity:0.17840379"
     id="path7626-6-5-9-8-0-7-0-7-4-04-9-9-9-7-6-72-8-5-2"
     cx="344.09424"
     cy="-326.07007"
     d="M 459.75726,-326.07007 A 115.66302,116.04749 0 0 1 344.09424,-210.02258 115.66302,116.04749 0 0 1 228.43122,-326.07007 115.66302,116.04749 0 0 1 344.09424,-442.11756 115.66302,116.04749 0 0 1 459.75726,-326.07007 Z" />
      <path
     id="path7444-2-0-5-6-8-7-4-5-1-5-6-5-3-8-1-0-7-2-9-6-6-6-6-2-8-0-8-9-5-6-11-2"
     style="opacity:1;fill:#6bfaff;fill-opacity:1;fill-rule:nonzero;stroke:none;stroke-width:3;stroke-linecap:butt;stroke-linejoin:miter;stroke-miterlimit:4;stroke-dasharray:none;stroke-dashoffset:0;stroke-opacity:0.47887328"
     d="m 456.99061,865.49758 c 0.988,97.41 6.122,102.3 149.174,133.349 l 0,48.92902 -489.06,0 0,-48.92902 c 147.123,-31.049 156.02,-35.939 157.012,-133.349 l 0,-640.63597 c -0.456,-48.643 -5.674,-62.502 -138.233,-89.49 l 0.957,-37.680997 291.458,-71.809 28.692,26.787 z"
     inkscape:connector-curvature="0"
     sodipodi:nodetypes="cccccccccccc" />
    </g>
    <g
       id="g11818"
       style="fill:#ff47c7;fill-opacity:1">
      <path
     id="ellipse11803"
     d="m 1235.5544,-320.00916 a 115.66302,116.04749 0 0 1 -115.663,116.0475 115.66302,116.04749 0 0 1 -115.6631,-116.0475 115.66302,116.04749 0 0 1 115.6631,-116.04749 115.66302,116.04749 0 0 1 115.663,116.04749 z"
     style="opacity:1;fill:#ff47c7;fill-opacity:1;fill-rule:nonzero;stroke:none;stroke-width:3;stroke-linecap:butt;stroke-linejoin:miter;stroke-miterlimit:4;stroke-dasharray:none;stroke-dashoffset:0;stroke-opacity:0.17840379"
     inkscape:connector-curvature="0" />
      <path
     sodipodi:nodetypes="cccccccccccc"
     inkscape:connector-curvature="0"
     d="m 1232.7878,871.5585 c 0.988,97.41 6.122,102.3 149.174,133.349 l 0,48.929 -489.06002,0 0,-48.929 c 147.12302,-31.049 156.02002,-35.939 157.01202,-133.349 l 0,-640.63597 c -0.456,-48.643 -5.674,-62.502 -138.23302,-89.49 l 0.957,-37.681 291.45802,-71.809002 28.692,26.787 z"
     style="opacity:1;fill:#ff47c7;fill-opacity:1;fill-rule:nonzero;stroke:none;stroke-width:3;stroke-linecap:butt;stroke-linejoin:miter;stroke-miterlimit:4;stroke-dasharray:none;stroke-dashoffset:0;stroke-opacity:0.47887328"
     id="path11805" />
    </g>
  </g>
</svg>
@tshinnic
Copy link
Contributor

How does it look after you have used menu item " Element / Simplify... / Simplify " on both shapes? When I did that it made the circle 'simpler' with only the 4 on-curve points I'd expect. Also notice how it works on the curve at the base of the 'i's. Both of them get simplified to be smoother.

@tayloraswift
Copy link
Author

Simplifying does not remove all the extra points. I tried it. The serifs should be defined with only two points. And it does nothing about the distorted tittle

@tshinnic
Copy link
Contributor

The left tittle here is the one that is defined with an <ellipse> from Inkscape, and the right one is a <path> generated by Inkscape. I agree the left one is ugly. Here is before a 'simplify':
kelvinsong 1 before
And after a simplify. The right one is simpler, but the left one is no less ugly.
kelvinsong 1 after

I did wonder about that <ellipse> definition though. It included the expected cx/cy/rx/ry attributes, but also included a path attribute d="..." ! That's rather strange. And when I edited the <ellipse> into a <path> using that d="...." attribute, I got a good-looking circle. So Inkscape is doing something strange also.

Oh, and I also experimented with changing rx and ry to be the same value, but the left tittle didn't look any better for that.

Let me go peek at the code struggling with <ellipse>...

I'm afraid, though, that I don't understand about "The serifs should be defined with only two points." Can you explain more what is different than you expect?

@tayloraswift
Copy link
Author

it ought to look like this:
screenshot 063
Which is what I get if I first convert the circle to a path in Inkscape

&& what is an F-SVG ?

@tshinnic
Copy link
Contributor

"F-SVG" is just one of the labels used here, assigned to issues to categorize them - 'F' for 'feature' (related to a particular area of FF) , 'SVG' for the SVG handling feature.

I'm looking at the SVG code now. Since I'm not that familar with the internals it'll take me a bit of time. I can see that the control points for the splines making up the 'circle' are not set correctly. They are too far away from the on-curve point and so the curves are flattened - that's why it is ugly.

@tayloraswift
Copy link
Author

k thankss!!

@tshinnic
Copy link
Contributor

tshinnic commented Sep 1, 2014

Umm yeah... I think I've got a solution for SVG circles/ellipses not being understood correctly, but it is a bit ambitious. Is there anyone here up on both splines and bezier math?

fontforge/svg.c routine SVGParseEllipse() is called to handle both <circle> and <ellipse> SVG elements. Parameters cx, cy, and either rx and ry for ellipses or just r for circles drives the processing.

The existing code tries to handle these with quadratic beziers, and setting the spline control points to corners of an imaginary rectangle around the shape. But... _everyone knows_ that quadratic beziers can't approximate ellipses, right? No wonder the result was so ugly,

So you need cubic beziers and figure out what to set the control points to, given the rx and ry values. Research finds articles throwing around "magic numbers" and formulas like

     radius *  4/3 * ( sqrt(2) - 1 )  =  0.5522847498307933984022516322796

Since by inspection of well-done circles (Inkscape) I'd derived numbers like 0.55 and 0.548 finding those articles made me happy.

The idea is to change out the control point construction in lines 1799-1821 of SVGParseEllipse(), which creates only 4 control points for quadratic bezier curvers, for code which uses cubics and 8 control points, two per on-curve point. And then the import results are fine.

In lieu of a PR just yet, here's the changed section:

1799     /* a magic number to make cubic beziers approximate circles     */
1800     /*            4/3 * ( sqrt(2) - 1 ) = 0.55228...                */
1801     /*   also     4/3 * tan(t/4)   where t = 90 b/c we do 4 curves  */
1802     double magic = 0.5522847498307933984022516322796;
1803     /* offset from on-curve point for control points                */
1804     double drx = rx * magic;
1805     double dry = ry * magic;
1806     /* we need cubic beziers to approximate circles/ellipses        */
1807     int do_order2 = 0;  // was 1 true meaning quadratic 2nd order   */
1808     cur = chunkalloc(sizeof(SplineSet));
1809     cur->first = SplinePointCreate(cx-rx,cy);
1810     cur->first->nextcp.x = cx-rx; cur->first->nextcp.y = cy+dry;
1811     cur->first->prevcp.x = cx-rx; cur->first->prevcp.y = cy-dry;
1812     cur->first->noprevcp = cur->first->nonextcp = false;
1813     cur->last = SplinePointCreate(cx,cy+ry);
1814     cur->last->prevcp.x = cx-drx; cur->last->prevcp.y = cy+ry;
1815     cur->last->nextcp.x = cx+drx; cur->last->nextcp.y = cy+ry;
1816     cur->last->noprevcp = cur->last->nonextcp = false;
1817     SplineMake(cur->first,cur->last,do_order2);
1818     sp = SplinePointCreate(cx+rx,cy);
1819     sp->prevcp.x = cx+rx; sp->prevcp.y = cy+dry;
1820     sp->nextcp.x = cx+rx; sp->nextcp.y = cy-dry;
1821     sp->nonextcp = sp->noprevcp = false;
1822     SplineMake(cur->last,sp,do_order2);
1823     cur->last = sp;
1824     sp = SplinePointCreate(cx,cy-ry);
1825     sp->prevcp.x = cx+drx; sp->prevcp.y = cy-ry;
1826     sp->nextcp.x = cx-drx; sp->nextcp.y = cy-ry;
1827     sp->nonextcp = sp->noprevcp = false;
1828     SplineMake(cur->last,sp,do_order2);
1829     SplineMake(sp,cur->first,do_order2);
1830     cur->last = cur->first;

What do y'all need to know to trust all this?

References:
"How to create circle with Bézier curves?"
"Bézier Circles and Bézier Ellipses"
and not too strangely searching using that constant finds lots of hits (truncate a few digits off the end and you'll still find hits!)

@tayloraswift
Copy link
Author

Thanks! BTW can't you like use fontforge's own circle tool for that ?

@tshinnic
Copy link
Contributor

tshinnic commented Sep 1, 2014

I've tested some more and will create a PR with this fix. Here are example circle and ellipse, imported without the change and then with the change:
 

Strangely, with a recent build (that includes the change) the other aspects that Kelvin was complaining about have gotten better. And I don't know why - it is certainly nothing to do with the SVG circle/ellipse element change Unless forcing one component to use cubic bezier curves improved the whole picture?

Has something about fixing up spline curves changed for the better lately? It looks better than even "simplify" can do. Here is the original SVG file imported again. Compare with the original picture from above, repeated below after the recent picture:
     kelvinsong 1 latest
kelvinsong 1 before

Splines split minds...

tshinnic added a commit to tshinnic/fontforge that referenced this issue Sep 1, 2014
See fontforge#1702 "SVG import: Fontforge adds a ton of extra points whenever
circles are involved" for additional discussions and illustrations.

Imported SVG circle and ellipse elements were being implemented using
quadratic bezier splines, which is known to be incorrect, as cubic
beziers are needed to even approximate these curves.  As part of using
quadratics, the control points were spaced too widely apart and thus
were flattening sections of the curves.

fontforge/svg.c routine SVGParseEllipse() is changed to use cubic splines.
Additionally the control point distances are determined using a magic
constant from formulas described on the web that give a very good
approximation to the desired curves when using only 4 splines.

Investigation finds that the existing UI circle curve drawing feature
within FF already uses cubics (and showing that quadratics were rejected
as inadequate) and use of a number near to the ideal magic constant.
(fontforgeexe/cvshapes.c :: CVMouseDownShape() and struct ellipse3)
jtanx pushed a commit to jtanx/fontforge that referenced this issue Sep 3, 2014
See fontforge#1702 "SVG import: Fontforge adds a ton of extra points whenever
circles are involved" for additional discussions and illustrations.

Imported SVG circle and ellipse elements were being implemented using
quadratic bezier splines, which is known to be incorrect, as cubic
beziers are needed to even approximate these curves.  As part of using
quadratics, the control points were spaced too widely apart and thus
were flattening sections of the curves.

fontforge/svg.c routine SVGParseEllipse() is changed to use cubic splines.
Additionally the control point distances are determined using a magic
constant from formulas described on the web that give a very good
approximation to the desired curves when using only 4 splines.

Investigation finds that the existing UI circle curve drawing feature
within FF already uses cubics (and showing that quadratics were rejected
as inadequate) and use of a number near to the ideal magic constant.
(fontforgeexe/cvshapes.c :: CVMouseDownShape() and struct ellipse3)
@tayloraswift
Copy link
Author

thank you sm!!!!!!!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants