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

Incorrect parsing of floating point values #18

Open
amcewen opened this issue Jul 16, 2021 · 2 comments
Open

Incorrect parsing of floating point values #18

amcewen opened this issue Jul 16, 2021 · 2 comments

Comments

@amcewen
Copy link
Contributor

amcewen commented Jul 16, 2021

While running the tests for #12 (Yay test code!) the new viewBox support was producing slightly different gcode for the hiking.svg test file.

Digging into it a bit I found that it was because the SVG height and width attributes were being truncated - I was getting a height of 791.716 for example, rather than the full 791.71631 in the SVG file. The viewBox code ended up scaling things a tiny bit, which threw the test code off.

The problem seems to be with the lines like this:
canvas_height = float(height_str) if height_str.isnumeric() else float(height_str[:-2])

Seems that isnumeric() returns False for strings containing decimals, and the code assumes they've got a mm or px, etc. at the end and chops off the last two characters - in the case of hiking.svg that's the last two decimal places of the number.

@amcewen
Copy link
Contributor Author

amcewen commented Jul 16, 2021

Checking the SVG spec, the width and height elements could be specified in values other than mm. From https://www.w3.org/TR/CSS21/syndata.html#value-def-length:

Absolute length units are fixed in relation to each other. They are mainly useful when the output environment is known. The absolute units consist of the physical units (in, cm, mm, pt, pc) and the px unit:

  • in: inches — 1in is equal to 2.54cm.
  • cm: centimeters
  • mm: millimeters
  • pt: points — the points used by CSS are equal to 1/72nd of 1in.
  • pc: picas — 1pc is equal to 12pt.
  • px: pixel units — 1px is equal to 0.75pt.

So ideally we'd parse them to handle any of these and convert them all to mm

amcewen pushed a commit to mcqn/SvgToGcode that referenced this issue Jul 19, 2021
amcewen pushed a commit to mcqn/SvgToGcode that referenced this issue Jul 19, 2021
…Lex#18

Lots of the Y dimensions have moved slightly - from a random sampling of a few dozen of the differing points they have all shifted by less than 0.001mm, so I'm assuming it's down to minor variations in the floating point arithmetic with the additional transformation.
@amcewen
Copy link
Contributor Author

amcewen commented Jul 19, 2021

I've added a new function to _parser_methods.py which can parse any of the valid lengths (and doesn't truncate the decimal part), and included some simple test cases for each of them. I doubt anyone will be specifying lengths in points or picas, but now they can 😀

That hasn't stopped the hiking.svg test case from failing. However, I've randomly sampled a couple of dozen points, and they all varied from the previous test case by less than 0.001mm, all in the Y dimension, so I'm assuming it's down to minor variations in the floating point arithmetic with the additional transformation.

The update to the hiking.svg test case is in a separate commit, in case you want to leave it out for further investigation.

Github seems to have added the commits for this to the existing PR automatically, so accepting that will fix this issue too.

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

No branches or pull requests

1 participant