SofaDefrost / STLIB

Sofa Template Library
GNU Lesser General Public License v3.0
17 stars 22 forks source link

problem with function getOrientedBoxFromTransform #20

Open ChristianDuriez opened 5 years ago

ChristianDuriez commented 5 years ago

There is a problem in the new implementation of getOrientedBoxFromTransorm, it creates a bug on step3 of the TRIPOD tutorial

https://github.com/SofaDefrost/STLIB/commit/67c1d510a39e25ec11901308702b0df814635248#r32599793

@damienmarchal could you have a look at it ?

euler Rotation is a vec3 rotation is a quat

Why are you mixing them ? The good design, I think, should be to provide the user a function that transforms euler to quat...but then, inside the functions, always use quat...

damienmarchal commented 5 years ago

Thanks for reporting the problems and the suggestion.

The general design guidlines in c++ would be to have the following:

void aFunction(Quaternion& rotation);
void aFunction(EulerAngles& rotation)
{
     aFunction(Quaternion::fromEulerAngles(rotation));
} 

In python the situation is a bit different as we cannot override function signature and this is not a typed language. In recent STLIB component the design seems to be the following:

def aFunction(rotation):
      qrotation = getRotationFromParameter(rotation)  
      ...

with:
def getRotationFromParameter(rotation):
     if len(rotation) == 3:  # This is euler angle in degree
            return Quaternion::createFromEulerAngle(rotation)
     if len(rotation) == 4:  # This is a quaternion
            return rotation
     raise Exception("This is not a valid rotation")

Eulalie was pointing that handling degree vs radian is a mess. Degree are not SI but everything in Sofa is un Degree. So what to do ? On my side I like approaches like these: https://pypi.org/project/numericalunits/ But this may be overkill.