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

_qcurveto_to_svg fails with specific paths #304

Closed
sakiodre opened this issue Jul 14, 2023 · 4 comments · Fixed by #305
Closed

_qcurveto_to_svg fails with specific paths #304

sakiodre opened this issue Jul 14, 2023 · 4 comments · Fixed by #305

Comments

@sakiodre
Copy link

sakiodre commented Jul 14, 2023

While using nanoemoji to build a font, I encountered a case where nanoemoji.write_part_file fails with this error:

  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "C:\dev\Python311\Lib\site-packages\nanoemoji\write_part_file.py", line 54, in <module>
    app.run(main)
  File "C:\dev\Python311\Lib\site-packages\absl\app.py", line 308, in run
    _run_main(main, args)
  File "C:\dev\Python311\Lib\site-packages\absl\app.py", line 254, in _run_main
    sys.exit(main(argv))
             ^^^^^^^^^^
  File "C:\dev\Python311\Lib\site-packages\nanoemoji\write_part_file.py", line 42, in main
    parts.add(svg)
  File "C:\dev\Python311\Lib\site-packages\nanoemoji\parts.py", line 168, in add
    self._add(as_shape(shape))
  File "C:\dev\Python311\Lib\site-packages\nanoemoji\parts.py", line 143, in _add
    norm = self.normalize(shape)
           ^^^^^^^^^^^^^^^^^^^^^
  File "C:\dev\Python311\Lib\site-packages\nanoemoji\parts.py", line 128, in normalize
    SVGPath(d=path).apply_transform(Affine2D.identity()),
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\dev\Python311\Lib\site-packages\picosvg\svg_types.py", line 275, in apply_transform
    return target.update_path(cmds, inplace=True)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\dev\Python311\Lib\site-packages\picosvg\svg_types.py", line 696, in update_path
    for cmd, args in svg_cmds:
  File "C:\dev\Python311\Lib\site-packages\picosvg\svg_pathops.py", line 101, in svg_commands
    for svg_cmd, svg_args in _SKIA_CMD_TO_SVG_CMD[cmd](points):
  File "C:\dev\Python311\Lib\site-packages\picosvg\svg_pathops.py", line 55, in _qcurveto_to_svg
    yield ("Q", control_pt + end_pt)
                ~~~~~~~~~~~^~~~~~~~
TypeError: can only concatenate tuple (not "NoneType") to tuple

When debugging, the last point is None:
image

Minimal reproducible svg file:

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16">
  <defs/>
  <path fill="#e3e3e3" d="M8.805,4.1 Q8.805,2.146 8.055,2.146 Q7.255,2.146 7.255,4.158 Q7.255,6.053 8.04,6.058 Q8.8,6.056 8.805,4.1 Z"/>
</svg>

Command:

python.exe -m nanoemoji.write_part_file -v 0 --reuse_tolerance 0.1 --wh 1200 --output_file parts.json ee49.svg

Minimal reproducible code using picosvg:

from picosvg.svg_types import SVGPath
from picosvg.svg_transform import Affine2D

path = "M0.117,0.055 Q0.117,0.029 0.107,0.029 Q0.097,0.029 0.097,0.055 Q0.097,0.081 0.107,0.081 Q0.117,0.081 0.117,0.055 Z"
SVGPath(d=path).apply_transform(Affine2D.identity())

This only occurs when the viewbox size is 16x16, the SVG renders fine in all browsers and editors, is this a bug or I'm just misusing it?

Edit:
After some debugging, it looks like the root problem is in the skia_path:

path = "M0.117,0.055 Q0.117,0.029 0.107,0.029 Q0.097,0.029 0.097,0.055 Q0.097,0.081 0.107,0.081 Q0.117,0.081 0.117,0.055 Z"
svg_path = SVGPath(d=path)
svg_cmds = svg_path.as_cmd_seq()
skia = skia_path(svg_cmds, fill_rule="nonzero")

for cmd, points in skia.segments:
    print(cmd, points)

Output (has None in points):

qCurveTo ((0.11699999868869781, 0.028999999165534973), (0.09700000286102295, 0.028999999165534973), (0.09700000286102295, 0.08100000023841858), (0.11699999868869781, 0.08100000023841858), None)
closePath ()

It looks like there's a problem in the method close of pathops._pathops.Path class, here is the full log of each iteration of the commands, notice in the last command "Z", it makes the last point in segments None:

Command M (0.117, 0.055)
  moveTo ((0.11699999868869781, 0.054999999701976776),)
  endPath ()

Command Q (0.117, 0.029, 0.107, 0.029)
  moveTo ((0.11699999868869781, 0.054999999701976776),)
  qCurveTo ((0.11699999868869781, 0.028999999165534973), (0.10700000077486038, 0.028999999165534973))
  endPath ()

Command Q (0.097, 0.029, 0.097, 0.055)
  moveTo ((0.11699999868869781, 0.054999999701976776),)
  qCurveTo ((0.11699999868869781, 0.028999999165534973), (0.09700000286102295, 0.028999999165534973), (0.09700000286102295, 0.054999999701976776))
  endPath ()

Command Q (0.097, 0.081, 0.107, 0.081)
  moveTo ((0.11699999868869781, 0.054999999701976776),)
  qCurveTo ((0.11699999868869781, 0.028999999165534973), (0.09700000286102295, 0.028999999165534973), (0.09700000286102295, 0.08100000023841858), (0.10700000077486038, 0.08100000023841858))
  endPath ()

Command Q (0.117, 0.081, 0.117, 0.055)
  moveTo ((0.11699999868869781, 0.054999999701976776),)
  qCurveTo ((0.11699999868869781, 0.028999999165534973), (0.09700000286102295, 0.028999999165534973), (0.09700000286102295, 0.08100000023841858), (0.11699999868869781, 0.08100000023841858), (0.11699999868869781, 0.054999999701976776))
  endPath ()

Command Z ()
  qCurveTo ((0.11699999868869781, 0.028999999165534973), (0.09700000286102295, 0.028999999165534973), (0.09700000286102295, 0.08100000023841858), (0.11699999868869781, 0.08100000023841858), None)
  closePath ()
@sakiodre
Copy link
Author

As a workaround, add a check if end_pt is None works just fine, I'm not sure if this is the proper way to handle this:

def _qcurveto_to_svg(points) -> SVGCommandGen:
    for control_pt, end_pt in pathops.decompose_quadratic_segment(points):
+        if end_pt is None: continue
        yield ("Q", control_pt + end_pt)

@anthrotype
Copy link
Member

thanks for the details bug report, yes this is a know issue introduced by latest skia-pathops 0.8, see googlefonts/nanoemoji#455.
You can temporarily workaround by pinning skia-pathops version to the one immediately preceding (e.g. pip install skia-pathops==0.7.4).
It's not a bug, but just haven't had the time to update picosvg to take the new feature into account.

@anthrotype
Copy link
Member

add a check if end_pt is None

no that would not be the correct fix, I'm afraid. Please pin the version for now, I will fix it soonish

@sakiodre
Copy link
Author

Thank you, i just tested and it works with the 0.7.4 version

anthrotype added a commit that referenced this issue Aug 4, 2023
Fixes #304
Fixes googlefonts/nanoemoji#455
Fixes googlefonts/noto-emoji#429

since skia-pathops v0.8.0, the Path.segments (SegmentPenIterator) may yield segments with an on-curve point set to None when closed contour is only comprised of quadratic beziers and all the on-curve points can be implied as in-between consecutive off-curve points (special TrueType quadratic spline), this was to match FontTools pen protocol, which the SegmentPenIterator is supposed to work with.
Picosvg was using this interface for converting from pathops.Path to SVG path.d strings, and uncaught TypeError was being raised when that happened. However, it turns out picosvg can avoid the SegmentPenIterator altogether when converting from pathops.Path to SVG, because the Path's RawPathIterator (i.e. iterating over the path itself as opposed to Path.segments) already yields (verb, points) for individual segment that matches what SVG expects (in this particular case, a move, a list of quadratic bezier segments each with one off-curve point, and a close command, no fonttools-style implied points anywhere). This way we can translate between SVG<=>pathops.Path in a more straightforward way (since they are closed to one another than to fonttools pen protocol more geared to font format specifics).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants