search

meerk40t / svgelements

SVG Parsing for Elements, Paths, and other SVG Objects.
MIT License
127 stars 28 forks source link

Fix union_bbox() #195

Closed ptomulik closed 1 year ago

ptomulik commented 1 year ago

Hi, it seems that there are two problems with union_bbox():

  1. It transforms bounding boxes of shapes instead of transforming shapes. This seems invalid, consider rotated bounding box of a circle vs bounding box of a rotated circle - the results are different.
  2. When a nested group (i.e a group within a group) defines a transformation, this transformation is applied twice.

I believe this PR fixes both of these issues.

tatarize commented 1 year ago

The tests failed due to self.assertEqual it should be self.assertAlmostEqual

-1
-1.0000000000000002
ptomulik commented 1 year ago

The tests failed due to self.assertEqual it should be self.assertAlmostEqual

-1
-1.0000000000000002

Done. The funny thing is it worked on Linux but not on other OSes. :)

tatarize commented 1 year ago

Likely something with order of operations at the far end of the weird trash at the end of the floats. I'd likely be more surprised if they'd get the same results. As far as I'm concerned the last bit in the mantissa is random.

tatarize commented 1 year ago

This PR is pretty bullet proof. I checked the blame and the tests and the logic behind it. All seems quite reasonable.