search

ajstarks / svgo

Go Language Library for SVG generation
Other
2.14k stars 169 forks source link

Adding Floating Point Precision #20

Closed swill closed 8 years ago

swill commented 9 years ago

Added floating point precision by introducing an svg.FloatDecimals variable into the constructor (defaulting to 2, so it is backwards compatible), which allows you to modify the floating point precision at any point as you draw the SVG file. I have introduced new functions for the majority of the functionality which support floating points. For example: Circle(x int, y int, r int, s ...string) now has a sister function with the following stub CircleF(x float64, y float64, r float64, s ...string). Both can be used interchangeably in an SVG file.

swill commented 9 years ago

Sorry, I should have mentioned this. This pull request is to fix #18.

stanim commented 9 years ago

I proposed an alternative approach which replaces manually added code with automatic code conversion by modifying the syntax tree. Please read issue https://github.com/ajstarks/svgo/issues/9 for my explanation and motivation.

@swill Thanks for your contribution. I hope you don't mind this discussion, because in the end we both agree that it would be nice to use svgo with floating points coordinates. Whatever @ajstarks chooses it will be a win-win situation for the both of us.

swill commented 9 years ago

@stanim Yes, we both needed something to solve for floating point support and we both implemented successful strategies.

I don't like the fact that I have duplicate function declarations in mine, but I do like that I can modify the floating point precision on the fly (I am developing CAD drawings, so that is valuable to me).

I like that your approach maintains a single function for each operation, but I don't like that it depends on source code outside of the svgo package.

There are pros and cons to each. I created the pull request so @ajstarks could more easily review what I had done. I will likely adopt whatever @ajstarks thinks is best because I don't really want to have a competing implementation of the library because it reduces confidence and this is a solid lib. I just needed to solve the problem, as did you... :)

stanim commented 9 years ago

If @ajstarks agrees, I can also make an implementation with %.*f to modify the floating precision on the fly, just as your implementation. I'm also generating CAD drawings, but I still don't understand the need for a varying precision. For example in AutoCAD you specify the precision before exporting a drawing as DXF, but the precision doesn't change within the drawing. (I never encountered CAD software which does this.) As said, this is not a limitation of my approach, it is more a design choice which I leave to @ajstarks.

If my approach is chosen, the float package(s) can also be hosted on the svgo repository (or an officially blessed svgof32 and svgof64 repository). So that might take maybe both your concerns away.

For me it is more important that svgo fully supports floats. So either which approach will be chosen, is fine for me.

ajstarks commented 9 years ago

@stanim can you please add the precision change. I'm leaning toward two packages: svgo for ints and legacy code and svgof for floating point as generated by the gofloat package.

stanim commented 9 years ago

As this pull request might be closed, I prefer to keep the discussion central at issue https://github.com/ajstarks/svgo/issues/18.