Rhino box bug

[chenkasirer]

This is to address two issues related to conversion to/from RhinoBox.

CompasBox => RhinoBox
Was missing the offsetting of the frame point to account for the difference in frame location in box in COMPAS and Rhino (center vs. corner)

RhinoBox => CompasBox
Was missing scaling the offset values by the axes, causing a discrepancy when converting a transformed (rotated) Box.

This might be considered a breaking change since client code compensating for this will now behave differently. But open for debate.

• Breaking change: bug fix or new feature that involve incompatible API changes. (?)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

• I added a line to the CHANGELOG.md file in the Unreleased section under the most fitting heading (e.g. Added, Changed, Removed).
• I ran all tests on my computer and it's all green (i.e. invoke test).
• I ran lint on my computer and there are no errors (i.e. invoke lint).

[tomvanmele]

this is not really related to the content of this PR, but we should find a way to test this kind of thing as part of the automated testing framework. since rhino3dm is installable from PyPi (https://pypi.org/project/rhino3dm/), perhaps we can define some sample data that can be used for this?

[chenkasirer]

this is not really related to the content of this PR, but we should find a way to test this kind of thing as part of the automated testing framework. since rhino3dm is installable from PyPi (https://pypi.org/project/rhino3dm/), perhaps we can define some sample data that can be used for this?

I agree. Wasn't aware of rhino3dm, pretty cool. I'll look into it and try to add some tests on this PR.

[chenkasirer]

So this is trickier than I imagined. I could mock Rhino.Geometry and get it to import some mock package which actually just wraps rhino3dm, but the types in rhion3dm don't expose the full API available for the types inside Rhino. For example, the constructor used in compas_rhino.conversions to create a Rhino Box is commented out in the binding source of rhino3dm.

# bnd_box.h

  BND_Box(const class BND_BoundingBox& bbox);
  //BND_Box(Plane basePlane, Interval xSize, Interval ySize, Interval zSize)
  //BND_Box(Plane basePlane, IEnumerable<Point3d> points)
  //BND_Box(Plane basePlane, GeometryBase geometry)
  //BND_Box(Plane basePlane, BoundingBox boundingbox)

Maybe this is due to lack of whatever dark magic layer which wraps around constructor overloading. But nor sure what can be done about it. We could maybe compile our own shared library but that seems like a lot of work, and will not be available on the test pipeline. @gonzalocasas @brgcode what do you think?

[tomvanmele]

not sure we talking about the same thing. what i had in mind is that we export some geometries from Rhino as 3dm files and add them to the collection test samples that can be read during the testing process using rhino3dm, but perhaps that is not possible...

[gonzalocasas]

LGTM!

About the unit testing conversation, let's keep it separate from this PR, maybe move to a github issue or discussion.