orbingol / NURBS-Python

Object-oriented pure Python B-Spline and NURBS library
https://onurraufbingol.com/NURBS-Python/
MIT License
636 stars 156 forks source link

Centre of rotation cannot be set #134

Closed sphh closed 3 years ago

sphh commented 3 years ago

Describe the bug When rotating an object, the centre of rotation is a point of that object's domain. For me, that's somehow counter-intuitive. But what is definitely missing is the possibility to set the centre of rotation.

Additional Details (Optional) Would you accept a PR as this one? It does not break the old behaviour, but adds the possibility to specify the centre of rotation:

--- geomdl/operations.py.ORIG
+++ geomdl/operations.py
@@ -1475,6 +1475,7 @@

     Keyword Arguments:
         * ``axis``: rotation axis; x, y, z correspond to 0, 1, 2 respectively. *Default: 2*
+        * ``center``: rotation center (x, y, z) or None. *Default: None*
         * ``inplace``: if False, operation applied to a copy of the object. *Default: False*

     :param obj: input geometry
@@ -1552,12 +1553,15 @@
     else:
         geom = obj

-    # Set a single origin
-    if geom[0].pdimension == 1:
-        params = geom[0].domain[0]
-    else:
-        params = [geom[0].domain[i][0] for i in range(geom[0].pdimension)]
-    origin = geom[0].evaluate_single(params)
+    # Set the origin
+    origin = kwargs.get('origin', None)
+    if origin is None:
+        # Set a single origin
+        if geom[0].pdimension == 1:
+            params = geom[0].domain[0]
+        else:
+            params = [geom[0].domain[i][0] for i in range(geom[0].pdimension)]
+        origin = geom[0].evaluate_single(params)

     # Start rotation
     for g in geom:
orbingol commented 3 years ago

Thanks for the suggestion @sphh. I have a feeling that adding such an option will confuse a lot of people and I can't see any use case that might use this feature. Please correct me if I am wrong.

sphh commented 3 years ago

Well, I believe it does make more sense to rotate any object at the origin of the coordinate system instead of a rather arbitrary point of the object. At least rotating it around the origin at least gives a predictable result. To be perfectly honest, I don't see any reason to not rotate it around the origin of the coordinate system. But since this is now the default behaviour, I kept it to avoid (nasty) surprises.

The use case if as follows:

  1. Get points on a surface. They are in their original position.
  2. Fit a surface through these points.
  3. Now you want to make a circular pattern, let's say around the z-axis.
  4. I would not know, how do to it with the current behaviour, because the fitted surface is rotated around a not very obvious (at least for me) centre.

What is the use case for the current implementation?

orbingol commented 3 years ago

The current implementation is the textbook definition of object rotation: translate the object to the coordinate center, rotate the object, and translate back to the original position. This will correctly rotate the object at the given angle about the selected axis. The object doesn't have to be positioned initially at the world origin.

sphh commented 3 years ago

If I rotate an object around the x-axis, I assume, that this is the x-axis through the origin of the coordinate system and not an axis parallel to the x-axis through a point, from which I don't know, where it is actually located. That is, how it is defined in the CAD world. I am sorry to say, that the textbook definition does not make any sense, because it is not predictable.

Anyway, my proposal does not change your default textbook definition. I just enhanced it with a very valuable functionality, so where is the problem?

orbingol commented 3 years ago

If I rotate an object around the x-axis, I assume, that this is the x-axis through the origin of the coordinate system and not an axis parallel to the x-axis through a point, from which I don't know, where it is actually located. That is, how it is defined in the CAD world. I am sorry to say, that the textbook definition does not make any sense, because it is not predictable.

Take a look at this: https://www.cs.helsinki.fi/group/goa/mallinnus/3dtransf/3drota.htm

Anyway, my proposal does not change your default textbook definition. I just enhanced it with a very valuable functionality, so where is the problem?

I still can't see any requirement for this feature. Thanks for the suggestion but I don't think it is necessary to work on it.

sphh commented 3 years ago

Thanks for that link. That's what I used to implement my little change.

Can you please tell me,

  1. How to rotate a surface around the y-axis going through the origin?
  2. Why a cylinder with its vertical axis coinciding with the z-axis does not stay at the same place, after rotating it around the z-axis?

The following picture shows a vertical cylinder, which axis is aligned with the z-axis (green).

After rotating it about the y-axis (blue), I would expect its axis to be coinciding with the x-axis, but it is not. So how would I achieve this using your code?

And after rotating it about the z-axis, I would expect it to stay in the very same place, but it does not (purple).

Figure_1

I know, that you rotate according to the textbook. But what is wrong to add an option to the rotate() function to specify a centre of rotation?

Here is the code for the picture shown above:

from geomdl import operations
from geomdl.shapes import surface
from geomdl import multi
from geomdl.visualization import VisMPL

# Generate vertical cylinder with r=0.25, h=1
cyl = surface.cylinder(0.25, 1)

# Rotate cylinder around y-axis, so that the new axis is parallel to the x-axis
cyl1 = operations.rotate(cyl, 90, axis=1)
cyl1.name = 'rotated about y-axis'

# Rotate cylinder around z-axis
cyl2 = operations.rotate(cyl, 90, axis=2)
cyl2.name = 'rotated about z-axis'

m = multi.SurfaceContainer(cyl, cyl1, cyl2)
m.vis = VisMPL.VisSurface(VisMPL.VisConfig(ctrlpts=False))
m.render()
sphh commented 3 years ago

Dear @orbingol, please can you tell me, how I rotate a geomdl object about the x, y or z coordinate axes through the origin?

Thank you for your help.