aardvark-platform / aardvark.base

Aardvark.Base is the foundation of the open-source Aardvark Platform for visual computing, real-time graphics, and visualization.
https://aardvarkians.com/
Apache License 2.0
154 stars 9 forks source link

Trafo transformations #73

Closed luithefirst closed 2 years ago

luithefirst commented 2 years ago

Trafo does not directly allow to perform a transformation, instead the user needs to explicitly take Forward or Backward and perform the transform using the matrix. This also requires considering that direction vectors need to be transformed with the inverse transposed matrix. I find myself often ignoring this when I know that the Trafo does not have a scale but actually this is a bad practice, especially since it would not cost anything to do it properly.

My suggestion is to add transform methods directly to Trafo that where the versions for Dir internally make an inverse transposed transform:

I'm pretty sure that this has been discussed at some point, but I can't remember any details. Do you see any issues?

Those direct transform methods would make Trafo equivalent to Euclidean and other transforms. A lot of code would get much more robust and easier to read. It would make the awesomeness of Trafo work for you behind the scene without needing to worry about and not always when you want to use a Trafo reminding you that there are two matrices internally.

I would be happy to see this extension in a future Aardvark version.

hyazinthh commented 2 years ago

Added in https://github.com/aardvark-platform/aardvark.base/commit/5f25d838eb9145670fc8e20266f1e1b16671d38b

luithefirst commented 2 years ago

Thanks for the addition, already in use, everything fine!

luithefirst commented 2 years ago

(Inv)TransformPosProj is missing