deeplook / svglib

Read SVG files and convert them to other formats.
GNU Lesser General Public License v3.0
311 stars 80 forks source link

TypeError when using `em` units because `em_base` defaults to `None` #315

Closed stefanw closed 2 years ago

stefanw commented 2 years ago

Using em units in width and height of the svg tag like this:

<svg width="1em" height="1em" viewBox="0 0 32 32" xmlns="http://www.w3.org/2000/svg"></svg>

leads to a TypeError: unsupported operand type(s) for *: 'float' and 'NoneType'.

Full traceback
Traceback (most recent call last):
    drawing = svg2rlg(filename)
  File "svglib/svglib.py", line 1449, in svg2rlg
    drawing = svgRenderer.render(svg_root)
  File "svglib/svglib.py", line 545, in render
    main_group = self.renderSvg(node, outermost=True)
  File "svglib/svglib.py", line 836, in renderSvg
    width, height = self.shape_converter.convert_length_attrs(
  File "svglib/svglib.py", line 957, in convert_length_attrs
    return [
  File "svglib/svglib.py", line 958, in 
    convLength(getAttr(attr), attr_name=attr, em_base=em_base, default=default)
  File "svglib/svglib.py", line 321, in convertLength
    return float(text[:-2]) * em_base
TypeError: unsupported operand type(s) for *: 'float' and 'NoneType'

This seems to be caused by Svg2RlgShapeConverter.convert_length_attrs defaulting to em_base=None and the calling function not setting em_base. Would falling back to a general value e.g. 12 like it is done in Svg2RlgAttributeConverter.convertLength make sense here?

The idea here is that the width and height are set relative to the embedding document's font-size (which is – of course – not available here).

github-actions[bot] commented 2 years ago

Thank you for raising your first issue! Your help to improve svglib is much appreciated!

deeplook commented 2 years ago

Sounds reasonable to me. Are you able to run/test it ok with a value of 12?

stefanw commented 2 years ago

Yes, with a value of 12, it works. I opened PR #317 that fixes this by introducing a DEFAULT_FONT_SIZE variable and using it throughout the code where there was a magic 12 before. A test for em-dimensions is in there as well.