Tweak math ordering to fix #479

[llamasoft]

This fixes issue #479. I replaced all remaining instances of 24.5 being used in the numerator with the "inverted" form originally found in cam-gcode-raster.js.

[tbfleming]

You're replacing one set of corner cases with another set. Something will always be .00000...n away from a desired value.

[llamasoft]

That's not necessarily true. The case that led me here was having a bitmap DPI of 127.
In JavaScript, 25.4 / 127 gives 0.19999999999999998, but 1.0 / (127 / 25.4) gives an exact 0.2.
This difference between how document.js and cam-gcode-raster.js were handling the order of operations caused the resulting gcode to come out incorrect.

This pull request isn't as perfect as adding BigNumber support, but standardizing the arithmetic ordering should lead to more consistent results.

[tbfleming]

0.2 isn't exact and 0.19999... isn't wrong. Something is probably using floor where it could use round.

[llamasoft]

You're correct, 0.2 can never be exact because it's not representable as a floating point number.
That said, the inverted method gives exactly what JavaScript thinks 0.2 is, but the previous method doesn't: https://jsfiddle.net/50dvp9ox/

[tbfleming]

What about .3? .79? Other edge cases? Something's not handling .19999...x well. That's where it should be fixed to be more robust. Chances are there's a floor(a/b) somewhere that needs to be fixed.

[llamasoft]

In this situation, there is no explicit floor, just an implicit one.
Here's how the error propagates:

1. The image is loaded in loadImage from document.js. This calculates the image's scaling and stores it in transform2d. Because of the rounding error when doing PPI to PPM conversion, the resulting transform2d is [0.19999999999999998, 0, 0, 0.19999999999999998, 0, 0].
2. When it comes time to generate the raster GCode, cam-gcode-raster.js calculates the image's bounds using the image's pixel size, a PPI/PPM scaling factor, and transform2d from part 1. This results in imgBounds = {x1: 0, y1: 0, x2: 33, y2: 1.9999999999999998}.
3. Using the image bounds, the canvas size is calculated, resulting in w = 33 and h = 1.9999999999999998.
4. The canvas element expects its size to be defined in pixels, so it doesn't support floating point height/width. This results in a height of 1 and a width of 33. This is the implicit floor operation.
5. The image is mashed onto the undersized canvas, resulting in loss of quality and blurred pixels.
6. The canvas is passed to Raster2Gcode, which now only sees one row of pixels.

[jorgerobles]

Good job! So sorry to be naive, but a Math.ceil will suffice to the 158-159?

[tbfleming]

Yes. Good job!

@jorgerobles think about how you want it to behave for these cases:

• h = 1.99999...x
• h = 2.00000...x

[jorgerobles]

LOL.. then Math.Round :D

[cprezzi]

Just a question of understanding: The configured DPI is just used to calculate an initial scaling. When I change the size in the placement dialog, the theoretical DPI is changing.

But: When generatig the raster gcode every pixel has to make a certain move distance that is equal to the tool diameter. So where is the problem? 1.9999999...x should just be rounded to the max firmware prezision of the axes.

[llamasoft]

@cprezzi - It could, but the Raster2Gcode library doesn't work directly with the input image, it works with the re-scaled canvas version (to allow application of filters like brightness/contrast/grayscale) which is required to have an integer height/width. There's no way to force the canvas to have a fractional size because the canvas element doesn't believe in fractional pixels.

As for a solution, I'm kind of leaning towards @jorgerobles on this because of what @cprezzi mentioned regarding the tool diameter. The rastering works by imagining a grid with units every toolDiameter apart and generates the output by moving from the center of each grid square to the next. I think the same idea should apply here: if the input image has a center point for a pixel (i.e. at least half a pixel at the specified DPI), it should be included. This is consistent with using Math.round.

[llamasoft]

I've updated the PR to include the Math.round on the canvas size.
Additionally, I ran a test comparing DPI/DPM conversion accuracy using some of the methods I found in the codebase. It appears that "scaled" division is the most accurate option: https://jsfiddle.net/juL73esa/
The PR has been updated to use that method instead. It should help reduce the number of edge cases the canvas scaling has to handle.