How to deal with transformation without scaling in CPACS & TiGL

[MarAlder]

Hey,

just for brainstorming different implementation ideas: we typically use the CPACS transformationType with scaling, translation, rotation.

For the new systems definition systemElements are referenced at aircraft level as component via the classical parentUID linking.

On the CPACS side, we should try to avoid scaling of system components at aircraft level. Components are predefined in their size, mass and center of gravity. If we allow scaling, it is not entirely clear whether this only refers to the geometry, or whether the mass and center of gravity are also changed, etc. It is therefore best for CPACS users to create a new predefined system element.

Now to TiGL implementation. We could have a new cpacs type transformationRTType (RT = rotation translation; open for better names) and generate CPACSTransformationRT. From this we could add a custom class CCPACSTransformationRT similar to CCPACSTransformation. This influences CTiglRelativelyPositionedComponent and many other TiGL classes as well.

Should we adopt CTiglRelativelyPositionedComponent to handle also CPACSTransformationRT? Or could you imagine ways to combine CPACSTransformation and CPACSTransformationRT in a common CCPACSTransformation? For the implementation, I'm thinking about pros and cons of different approaches and therefore wanted to collect some additional ideas here. In the worst case we can stick to scaling, but maybe we find more elegant solutions to deal with different types of transformation? Any ideas or suggestions?

[joergbrech]

Good question! I would probably need to experiment a bit to get a feel for the pros and cons. I wonder if it suffices to have a CCPACSTransformation constructor that accepts a CPACSTransformationRT?

struct CPACSTransformationSE3
{
    // an element of the special Euclidean group SE(3)
    Vec3 translation;
    Vec3 rotation;
};

// no inheritence, because of schema *(unless we inherit in the schema?)*
struct CPACSTransformation
{
    double scale;
    Vec3 translation;
    Vec3 rotation;
};

// custom class 
struct CCPACSTransformation : public CPACSTransformation
{
    // implicit constructor from CPACSTransformationSE2
    CCPACSTransformation(CPACSTransformationSE3 se3)
     : CPACSTransformation{1., se3.translation, se3.rotation}
    {}
};

struct CTiglRelativelyPositionedComponent
{
    CCPACSTransformation trafo;
};

• What is your opinion on inheriting transformationType from transformationRTType in the schema itself?
• What do you think about the names transformationRigidBody or transformationSE3 instead of transformationRT?

Note, that this discussion is related to #911. Here, the proposition is to remove CCPACSTransformation from RelativelyPositionedComponent. Instead, it shall have only a TiGL-specific CoordinateSystem, which in turn contains a CoordinateTransform, the latter being an abstraction over a homogeneous 4x4 transform matrix. Then we would have conversion operators / constructors to switch from the two CPACS transformation types to TiGL's CoordinateTransform.

[MarAlder]

Renaming transformationRT to transformationSE3 or others is fine for me. Inheritance in XSD itself might be problematic, as this enforces xsd:sequence and therefore a strict order of scaling, rotation and translation.

Experimental material is available in my systems branch with tiglSystems.cpp serving as entry point for debugging ;-)

[joergbrech]

Ah I had a closer look and of course my code example is too simple, because of the following issues in the TiGL source code:

1. IMHO, there is a dependency issue in the generated code, where a class associated with an xml node gets a constructor for every possible parent node. As a consequence, CPACSTransformation explicitly depends on all classes that use it. We can't construct a base class instance CPACSTransformation of CCPACSTransformation if the parent uses the other transformation type CCPACSTransformationSE3. The dependency should be inversed. Maybe something to fix at a later point 😁
2. A CTiglRelativelyPositionedComponent doesn't store a CCPACSTransformation by value but by reference via the strange MaybeOptionalPtr<CCPACSTransformation> member.
3. It is a bit odd that a CTiglRelativelyPositionedComponent stores a reference to a CCPACSTransformation at all: This implies that a CTiglRelativelyPositionedComponent is always associated with a CPACSXXX class that has a CCPACSTransformation. In practice this has been the case up until now, but it isn't enforced via the code design at all. In practice, we derive a CCPACSXXX class from both a CPACSXXX class and CTiglRelativelyPositionedComponent and notify the CTiglRelativelyPositionedComponent about the CCPACSTransformation that exists in the CPACSXXX class, once we implement the CCPACSXXX class. 🫤

The easiest short-term solution is probably to have CTiglRelativelypositionedComponent work with both transformation types and adapt the interface accordingly as you suggested. But I have the hunch that this just ignores the underlying design issues mentioned above and builds on top of the technical dept we have in the code already.

My current guess is that it might be a good idea to begin with refactoring CTiglRelativelyPositionedComponent in such a way, that we remove CCPACSTransformation from its interface.

// a CPACS element using a traditional CPACS transformation including scale
struct CPACSFoo
{
    CCPACSTransformation trafo;
};

// a CPACS element using the new CPACSTransformationSE3 type
struct CPACSSystem
{
    CCPACSTransformationSE3 trafo;
};

// TiGL source code

struct CTiglTransformation
{
    explicit CTiglTransformation(CCPACSTransformation trafo) {}
    explicit CTiglTransformation(CCPACSTransformationSE3 trafo) {}
};

struct CTiglRelativelyPositionedComponent
{
    virtual ~CTiglRelativelyPositionedComponent() {}
    virtual CTiglTransformation GetTransformationMatrix() const = 0;
};


struct CCPACSFoo : public CPACSFoo, public CTiglRelativelyPositionedComponent
{
    CTiglTransformation GetTransformationMatrix() const override
    {
        return CTiglTransformation(trafo);
    }
};

struct CCPACSSystem : public CPACSSystem, public CTiglRelativelyPositionedComponent
{
    CTiglTransformation GetTransformationMatrix() const override
    {
        return CTiglTransformation(trafo);
    }
};

I do realize, that this is a somewhat painful refactor, because many classes access the CCPACSTransformation via the CTiglRelativelyPositionedComponent. This would also be a good step towards #911.

[MarAlder]

Ok, thanks @joergbrech for your thoughts. I'm modifying CTiglRelativelypositionedComponent to handle both transformations. Now I now at least that I haven't missed a more elegant (easy) solution. 🙏

Refactoring according to #911 would improve this implementation, but I leave that to the experts ;-)