search

fontello / svg2ttf

SVG -> TTF font convertor
MIT License
520 stars 80 forks source link

yMax and usWinAscent calculation error #105

Closed yisibl closed 3 years ago

yisibl commented 3 years ago

This will cause the line height in Windows to be too large. https://github.com/thx/iconfont-plus/issues/1953#issuecomment-834044570

image

I guess it is a problem introduced by(v2.1.0) https://github.com/fontello/svg2ttf/issues/25 , and there is a problem with the conversion of the cubic2quad module.

In the result returned by cubic2quad, 32147.26312251112 is abnormal.

[ 'Q', 566.970047, 32147.26312251112, 567.27723, 693.5669409999996 ]

https://github.com/fontello/svg2ttf/blob/df30be9043ccb9d00e1c9087b21f084a8fface4f/lib/svg.js#L96-L111

Test file and test case in:

https://github.com/fontello/svg2ttf/pull/102/commits/ae425be18907eb9e93ceca2ecfd8ac73dcba77fd

<?xml version="1.0" standalone="no"?>
<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd" >
<svg>
    <metadata>
        iconfont
    </metadata>
    <defs>
        <font id="iconfont" horiz-adv-x="1024">
            <font-face font-family="iconfont" font-weight="500" font-stretch="normal" units-per-em="1024" ascent="896" descent="-128" />
            <missing-glyph />
            <glyph glyph-name="up" unicode="&#58880;" d="M566.970047-68.244874V693.362153c0 0.204788 0.307182 0.307182 0.307183 0.204788l259.773733-287.42013c13.61841-14.949533 33.79004-21.195571 52.835335-16.178261a56.521522 56.521522 0 0 1 39.524108 40.548048 61.02686 61.02686 0 0 1-13.208834 56.623916L552.634879 878.285827A53.654488 53.654488 0 0 1 513.110771 896a53.654488 53.654488 0 0 1-39.524107-17.816567L117.767301 485.092633A60.207708 60.207708 0 0 1 102.408192 443.41825a59.798132 59.798132 0 0 1 16.89502-40.957625c21.912329-22.219511 56.521522-21.502753 77.614698 1.638305L456.691644 691.314272h0.307182v-760.787875c0-31.230189 22.833876-56.521522 51.401818-58.364614 15.359109-0.819152 30.308642 5.119703 41.367201 16.383049a61.231649 61.231649 0 0 1 17.099808 43.210294z" horiz-adv-x="1024" />
        </font>
    </defs>
</svg>

TTX file

<?xml version="1.0" encoding="UTF-8"?>
<ttFont sfntVersion="\x00\x01\x00\x00" ttLibVersion="4.22">
  <GlyphOrder>
    <GlyphID id="0" name="glyph00000"/>
    <GlyphID id="1" name="up"/>
  </GlyphOrder>

  <head>
    <tableVersion value="1.0"/>
    <fontRevision value="1.0"/>
    <checkSumAdjustment value="0xd03dc53e"/>
    <magicNumber value="0x5f0f3cf5"/>
    <flags value="00000000 00001011"/>
    <unitsPerEm value="1024"/>
    <created value="Sat May  8 11:35:08 2021"/>
    <modified value="Sat May  8 11:35:08 2021"/>
    <xMin value="0"/>
    <yMin value="-130"/>
    <xMax value="1024"/>
    <yMax value="32148"/>
    <macStyle value="00000000 00000000"/>
    <lowestRecPPEM value="8"/>
    <fontDirectionHint value="2"/>
    <indexToLocFormat value="0"/>
    <glyphDataFormat value="0"/>
  </head>

  <hhea>
    <tableVersion value="0x00010000"/>
    <ascent value="896"/>
    <descent value="-128"/>
    <lineGap value="0"/>
    <advanceWidthMax value="1024"/>
    <minLeftSideBearing value="0"/>
    <minRightSideBearing value="0"/>
    <xMaxExtent value="1024"/>
    <caretSlopeRise value="1"/>
    <caretSlopeRun value="0"/>
    <caretOffset value="0"/>
    <reserved0 value="0"/>
    <reserved1 value="0"/>
    <reserved2 value="0"/>
    <reserved3 value="0"/>
    <metricDataFormat value="0"/>
    <numberOfHMetrics value="2"/>
  </hhea>

  <OS_2>
    <version value="1"/>
    <xAvgCharWidth value="1024"/>
    <usWeightClass value="400"/>
    <usWidthClass value="5"/>
    <fsType value="00000000 00000000"/>
    <ySubscriptXSize value="649"/>
    <ySubscriptYSize value="716"/>
    <ySubscriptXOffset value="0"/>
    <ySubscriptYOffset value="143"/>
    <ySuperscriptXSize value="649"/>
    <ySuperscriptYSize value="716"/>
    <ySuperscriptXOffset value="0"/>
    <ySuperscriptYOffset value="491"/>
    <yStrikeoutSize value="50"/>
    <yStrikeoutPosition value="264"/>
    <sFamilyClass value="0"/>
    <panose>
      <bFamilyType value="2"/>
      <bSerifStyle value="0"/>
      <bWeight value="5"/>
      <bProportion value="3"/>
      <bContrast value="0"/>
      <bStrokeVariation value="0"/>
      <bArmStyle value="0"/>
      <bLetterForm value="0"/>
      <bMidline value="0"/>
      <bXHeight value="0"/>
    </panose>
    <ulUnicodeRange1 value="00000000 00000000 00000000 00000000"/>
    <ulUnicodeRange2 value="00000000 00000000 00000000 00000000"/>
    <ulUnicodeRange3 value="00000000 00000000 00000000 00000000"/>
    <ulUnicodeRange4 value="00000000 00000000 00000000 00000000"/>
    <achVendID value="PfEd"/>
    <fsSelection value="00000000 01000000"/>
    <usFirstCharIndex value="58880"/>
    <usLastCharIndex value="58880"/>
    <sTypoAscender value="896"/>
    <sTypoDescender value="-128"/>
    <sTypoLineGap value="92"/>
    <usWinAscent value="32148"/>
    <usWinDescent value="130"/>
    <ulCodePageRange1 value="00000000 00000000 00000000 00000001"/>
    <ulCodePageRange2 value="00000000 00000000 00000000 00000000"/>
  </OS_2>
  <glyf>
    <TTGlyph name="glyph00000"/><!-- contains no outline data -->
    <TTGlyph name="up" xMin="0" yMin="-130" xMax="924" yMax="32148">
      <contour>
        <pt x="567" y="-68" on="1"/>
        <pt x="567" y="693" on="1"/>
        <pt x="567" y="32147" on="0"/>
        <pt x="567" y="694" on="1"/>
        <pt x="827" y="406" on="1"/>
        <pt x="837" y="395" on="0"/>
        <pt x="866" y="386" on="0"/>
        <pt x="894" y="394" on="0"/>
        <pt x="916" y="416" on="0"/>
        <pt x="923" y="445" on="0"/>
        <pt x="916" y="476" on="0"/>
        <pt x="906" y="487" on="1"/>
        <pt x="553" y="878" on="1"/>
        <pt x="537" y="896" on="0"/>
        <pt x="489" y="896" on="0"/>
        <pt x="474" y="878" on="1"/>
        <pt x="118" y="485" on="1"/>
        <pt x="102" y="467" on="0"/>
        <pt x="103" y="420" on="0"/>
        <pt x="136" y="386" on="0"/>
        <pt x="181" y="387" on="0"/>
        <pt x="197" y="404" on="1"/>
        <pt x="457" y="691" on="1"/>
        <pt x="457" y="-69" on="1"/>
        <pt x="457" y="-93" on="0"/>
        <pt x="487" y="-126" on="0"/>
        <pt x="508" y="-128" on="1"/>
        <pt x="532" y="-129" on="0"/>
        <pt x="567" y="-93" on="0"/>
      </contour>
      <instructions/>
    </TTGlyph>
  </glyf>
</ttFont>
yisibl commented 3 years ago

I found that if the accuracy is greater than 0.153, there will be problems. Can we use a smaller accuracy? https://github.com/fontello/svg2ttf/blob/d8a426247c6f61b00e4cdd5bd36ec0c57b404444/index.js#L167-L171

puzrin commented 3 years ago

I found that if the accuracy is greater than 0.153, there will be problems. Can we use a smaller accuracy?

There are no clear criteria how to select accuracy. Without criteria, this is "magical number". Tuning magical numbers is not good idea.

I'd suggest 2 alternate approaches:

  1. Update source file, if possible (split that bad curve manually to 2-4 segments)
  2. Increase resolution from 1000px to 2000px or 4000px

Increasing existing accuracy demand will cause increase of segments (=> font size) and is not nice.

puzrin commented 3 years ago

Or... if you know better accuracy criteria - that's discussable. I'm sure, better algorythm possible, but had no time to investigate.

I mean, visual criteria is ~ obvious. Visual error should be < 0.5px for 1000*1000px bounding box. But turning that into math metrics is not so easy.

yisibl commented 3 years ago

Are there any side effects of smaller accuracy? I cannot change the SVG source files, because these are uploaded by other users. There should be many similar SVG files on our platform, and many of them are historical data.

yisibl commented 3 years ago

Maybe we can change the calculation method of usWinAscent, in nanoemoji os2.usWinAscent = ascender + linegap, yMax is not currently considered.

In terms of vertical measurement, do icons need to consider clipping?

    # These are ufo2ft's fallback WinAscent/WinDescent, good enough for now.
    # https://github.com/googlefonts/ufo2ft/blob/a5267d135d5a8dba7e6a5d3ac44139afa6159429/Lib/ufo2ft/fontInfoData.py#L241-L247
    # TODO: Set to the actual global yMin/yMax to prevent any clipping?
    assert os2.usWinAscent == ascender + linegap
    assert os2.usWinDescent == abs(descender)  # always positive

This is svg2ttf: https://github.com/fontello/svg2ttf/blob/d8a426247c6f61b00e4cdd5bd36ec0c57b404444/lib/ttf/tables/os2.js#L62-L67

In addition, cu2qu uses 0.001 as the default approximation error: https://github.com/googlefonts/cu2qu/issues/76

@anthrotype is an expert in fonts, I hope he can provide some help.

puzrin commented 3 years ago

Are there any side effects of smaller accuracy?

Yes. Smaller accuracy => more splits => higher font size. Problem is, everybody will be affected, and you will not give any guarantees. It will be still possible to draw crazy arc or third-order bezier curve to break everything.

Ideally, "accuracy" should depend on concrete segment.

I cannot change the SVG source files, because these are uploaded by other users. There should be many similar SVG files on our platform, and many of them are historical data.

If you don't care about font size, you can set TTF glyph height to ~ 4000 instead of 500-1000 as you have now. Result will be similar as accuracy value change. You can scale your src by svgpath without loss of precision.

I'm open to any alternate suggestions with strong math background. It would be nice to drop use of current "magical values".

yisibl commented 3 years ago

This is the original SVG file. Is it because the size is too small (8x10)? If so, can we use different accuracy number when the SVG size is small?

<svg width="8" height="10" viewBox="0 0 8 10" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
    <g fill="none" fill-rule="evenodd">
        <path d="M4.537 9.417V1.979c0-.002.003-.003.003-.002l2.537 2.807c.133.146.33.207.516.158a.552.552 0 0 0 .386-.396.596.596 0 0 0-.129-.553L4.397.173A.524.524 0 0 0 4.011 0a.524.524 0 0 0-.386.174L.15 4.013A.588.588 0 0 0 0 4.42a.584.584 0 0 0 .165.4c.214.217.552.21.758-.016L3.46 1.999h.003v7.43c0 .305.223.552.502.57.15.008.296-.05.404-.16a.598.598 0 0 0 .167-.422z" fill="#41D2D9" />
    </g>
</svg>
yisibl commented 3 years ago

Another idea is that we can get the SVG BoundingBox, calculate the height, and then make usWinAscent not greater than the BoundingBox's height. Is this feasible?

puzrin commented 3 years ago

This is the original SVG file. Is it because the size is too small (8x10)? If so, can we use different accuracy number when the SVG size is small?

Since font glyph usually are 500*500...4000*4000 int px, better approach would be to scale up your image BEFORE converting to second order bezier and rounding. Use svgpath for that.

Another idea is that we can get the SVG BoundingBox, calculate the height, and then make usWinAscent not greater than the BoundingBox's height. Is this feasible?

Problem is not with bounding box, but with processing of third order bezier & arcs. It's technically possible write those such way to break any static limit (then, after rounding you will get something bad).

yisibl commented 3 years ago

Found a new issue, this font cannot be displayed in Mac Chrome and Firefox. https://at.alicdn.com/t/project/37546/21049af9-0257-43a6-8c7e-2fd08733737d.html

Chrome DevTools says: OTS parsing error: head: Failed to parse table

puzrin commented 3 years ago

@yisibl if that's not related to current issue, please fill new one. With minimalistic sample how to reproduce (source SVG + reduce glyphs count as much as possible). You provided only final result, but without proof why this is caused by svg2ttf (i don't decline, but there may be multiple reasons of broken fonts).

PS. FF shows more concrete error: downloadable font: head: Bad x dimension in the font bounding box (-39, -13440) (font-family: "iconfont" style:normal weight:400 stretch:100 src index:2) source: https://at.alicdn.com/t/font_37546_hgtmiiq9vft.woff?t=1620616818531

yisibl commented 3 years ago

@yisibl if that's not related to current issue, please fill new one. With minimalistic sample how to reproduce (source SVG + reduce glyphs count as much as possible). You provided only final result, but without proof why this is caused by svg2ttf (i don't decline, but there may be multiple reasons of broken fonts).

PS. FF shows more concrete error: downloadable font: head: Bad x dimension in the font bounding box (-39, -13440) (font-family: "iconfont" style:normal weight:400 stretch:100 src index:2) source: https://at.alicdn.com/t/font_37546_hgtmiiq9vft.woff?t=1620616818531

I have tried some fixes here. If the above problem cannot be solved, I will reopen an issue. https://github.com/fontello/svg2ttf/pull/102/commits/46c0df27ad44936c16e94fa67a5a3694afde8a89

In any case, too large usWinAscent and usWinDescent is definitely a problem.

yisibl commented 3 years ago

fontbakery check-notofonts iconfont.ttf output:

Rationale: A font's winAscent and winDescent values should be greater than the head table's yMax, abs(yMin) values. If they are less than these values, clipping can occur on Windows platforms (https://github.com/RedHatBrand/Overpass/issues/33).

  If the font includes tall/deep writing systems such as Arabic or
  Devanagari, the winAscent and winDescent can be greater than the yMax and
  abs(yMin) to accommodate vowel marks.

  When the win Metrics are significantly greater than the upm, the
  linespacing can appear too loose. To counteract this, enabling the OS/2
  fsSelection bit 7 (Use_Typo_Metrics), will force Windows to use the OS/2
  typo values instead. This means the font developer can control the
  linespacing with the typo values, whilst avoiding clipping by setting the
  win values to values greater than the yMax and abs(yMin).

FAIL: OS/2.usWinDescent value 38262 is too large. It should be less than double the yMin. Current absolute yMin value is 727 [code: descent] FAIL: ots-sanitize returned an error code (1). Output follows: ERROR: head: Bad x dimension in the font bounding box (-39, -13440) ERROR: head: Failed to parse table Failed to sanitize file!

yisibl commented 3 years ago

Some users have encountered similar problems a long time ago: https://github.com/thx/iconfont-plus/issues/661

rlidwka commented 3 years ago

In the result returned by cubic2quad, 32147.26312251112 is abnormal. [ 'Q', 566.970047, 32147.26312251112, 567.27723, 693.5669409999996 ]

That is an error in cubic2quad error detection algorithm. Right over here:

https://github.com/fontello/cubic2quad/blob/master/index.js#L198-L199

We check every(ish) point on the cubic curve to match quadratic. We do not check any point on the quadratic curve to match cubic.

The problem is: if middle point of cubic curve is within error from its origin, approximation will be good no matter what (quadratic control point may be on the moon for all it cares).

Nice picture here:

image

Your SVG file has all 4 cubic curve points very close together, so the difference between them falls well within the accuracy.

console.log(require('cubic2quad')(566.970047,693.362153,566.970047,693.566941,567.277229,693.669335,567.27723,693.566941,1))
rlidwka commented 3 years ago

Here's the exact curve:

image

<!DOCTYPE html>
<html>
<body>

<svg height="600" width="600" viewBox="566.5 693 1 2">
    <circle id="pointA" cx="566.970047" cy="693.566941" r="0.01" />
    <circle id="pointB" cx="567.277229" cy="693.669335" r="0.01" />  </g>

  <path d="M 566.970047 693.362153 C 566.970047 693.566941 567.277229 693.669335 567.27723 693.566941" stroke="red" stroke-width="0.005" fill="none" />
  <path d="M 566.970047 693.362153 Q 566.970047 32147.26312251112 567.27723 693.5669409999996" stroke="blue" stroke-width="0.005" fill="none" />
</svg>

</body>
</html>

try here: https://www.w3schools.com/graphics/tryit.asp?filename=trysvg_path2

rlidwka commented 3 years ago

fixed in https://github.com/fontello/cubic2quad/commit/44c53ad7b1a5321196bc7df84307bab9200df073

yisibl commented 3 years ago

fixed in fontello/cubic2quad@44c53ad

Awesome, thank you for your amazing work!

yisibl commented 3 years ago

I update a test case that caused the error:

head.yMin = 27274
head.xMax = -13440

shenheweitongguo.svg.zip

image

rlidwka commented 3 years ago

I update a test case that caused the error:

What is this test for? Does it still show any issues after cubic2quad update?

yisibl commented 3 years ago

@rlidwka This is used to test the version of cubic2quad before 1.2.0. see also: https://github.com/yisibl/svg2ttf/pull/1/files#diff-a561630bb56b82342bc66697aee2ad96efddcbc9d150665abd6fb7ecb7c0ab2fR246-R260

svg2ttf must have more test cases based on font tables.

yisibl commented 3 years ago

I have deployed to pre-release for testing(usWinAscent/usWinDescent does not depend on the value of yMax/yMin), and it seems to be completely resolved. The generated ttf file is also smaller.

before: https://at.alicdn.com/t/project/37546/21049af9-0257-43a6-8c7e-2fd08733737d.html after: https://at.alicdn.com/t/project/37546/f49c07e1-448c-4e92-80a0-7544b91a29e8.html

image

puzrin commented 3 years ago

Can we close this?

yisibl commented 3 years ago

Can we close this?

Need to wait, please let me test it online for a few days.

I encountered a new failed test case: https://github.com/yisibl/svg2ttf/pull/1/commits/56b273660993af64057cd1787d68baf0cf6b43e6

@rlidwka PTAL, thanks!

rlidwka commented 3 years ago

@yisibl in your example glyph has the following instruction near the end of the path:

a86173505653221920 86173505653221920 0 0 1 34.50966791-4.60128904

Does this arc have a good reason to be there? Looks like an error in the test file.

yisibl commented 3 years ago

@yisibl in your example glyph has the following instruction near the end of the path:

a86173505653221920 86173505653221920 0 0 1 34.50966791-4.60128904

Does this arc have a good reason to be there? Looks like an error in the test file.

Yes, we have some icons with very strange paths. This causes the generated font to fail the ots check, and an error will be reported in Chrome.

I think for this kind of SVG should throw an error.

Here is another example: https://at.alicdn.com/t/project/2567398/0794db68-7c83-4d28-8e2f-0dc819833051.html Test SVG font in: https://github.com/yisibl/svg2ttf/pull/2/files#diff-3ec56eb2d7d4c0768bcdb3b4db2508775c2efb3cfb976c7f09f8618aa2ab291d

puzrin commented 3 years ago

I think for this kind of SVG should throw an error.

I do not agree.

  1. That's a validation process, without strict criterias. Not related to font generation. Font generator just do what was ordered.
  2. This can be implemented separate, on app level. I would reconsider join only after this feature became stable (if that's technically possible at all).

Also, i'd suggest to focus on release, instead of "new features". Let's solve minimal mandatory set - svg2ttf should produce good result from good data. Is current master ok or something was missed?

yisibl commented 3 years ago

@puzrin I think 5.3 can be released. Special SVG errors can wait for me to write test cases more concisely and concretely.

puzrin commented 3 years ago

Thanks. I will release.

After thinking a bit, we could use some of chrome/mozilla criteria to check obvious (most annoying) artefacts:

Can you test that somehow? I mean, create dummy svg with small square and move that square to up/down/left/right until chrome/mozilla fails?

puzrin commented 3 years ago

Released 6.0.0, also added special thanks to changelog.

As far as i understand, we have 3 pending issues with notable added value:

If nothing critical missed, I'd suggest consider close existing issues/prs as "too noisy" and create new ones (more organized) to discuss details.

yisibl commented 3 years ago

Thank you very much for your summary, let us discuss it in a separate issue.