[Help Request] Transform Refactor

(Joël Lupien) #1

@everyone Hi there!
As I discussed earlier, I made some pretty significant changes to how Transform and GlobalTransform work.

The fork is located here: https://github.com/jojolepro/amethyst/tree/transform

Here’s a quick recap:

  • GlobalTransform has been removed, and the fields were moved into Transform
  • Transform is now generic over the N: Real bound (f32, f64).
  • Transform has been split into Transform2 and Transform3 because of the various differences in required data between 2d and 3d (also its a big optimisation for z ordering in 2d).
  • TransformSystem was updated for Transform3.

As you guessed, modifying the most used struct of all Amethyst will break a lot of things…
That’s why I’m requesting some help to update the remaining of the code.

Here’s what is left to do:

  • Rename TransformSystem to TransformSystem3
  • Create TransformSystem2
  • Fix the broken code
  • Fix the broken docs
  • Review the changes, current and future, to make sure we aren’t shooting ourselves in the foot with this change.

Here’s some numbers:

  • There is 92 mentions of “GlobalTransform” in the whole codebase (includes book).
  • There is 840 mentions of “Transform”.

Considering the amount of code to update, I estimate we would need 2 or 3 developers working on different amethyst_* crates to be able to complete this in two to three weeks.

So, in order:

  1. Is this change wanted?
  2. Who wants to review the current changes, and try to find if there is any missing fields or features missing?
  3. Who wants to help update the code so it compiles?
  4. Who wants to update the documentation?

I opened a pull request here https://github.com/amethyst/amethyst/pull/1334 to give write access to everyone, and to make reviewing easier.

Thanks everyone! :heart:

(Joël Lupien) #2

Related forum post Transform Refactor Discussion

(Joël Lupien) #3

Also, I did a small scale benchmark, and combining GlobalTransform into Transform had a small performance boost.

(Théo Degioanni) #4

Wanted to quickly mention that yes, this change is most wanted for many other reasons than what you are stating. One being for 3D-plane 2D rendering for example.

(Kel) #5

Speaking topically in terms of documentation, and the general learning experience, I think this API would do well. I really like it from a high level

(Teemu) #6

I smell a good GUI system in the air. (The kind which isn’t shackled to anchors :smirk:)

If this is done with “refactor -> rename” in intellij IDE then I’m fairly sure there would be 0 broken code & broken docs.

(OvermindDL1) #7

Not TransformSystem3D?

(Khionu Sybiern) #8

3 refers to the number of axis. 3D is more of a perception/art detail than technical.

(OvermindDL1) #9

Got that about 3, but when seeing just a 3 it usually implies a revision number or so of a function to me, where 3D implies it is about dimensionality instead. ^.^;

(Khionu Sybiern) #10

Revision in type names is not something I’ve seen as standard, and I can definitely say you won’t see it in Amethyst

(OvermindDL1) #11

It’s something I see way too often the past few decades for ‘updated’ calls and thus it’s something I fight against, so seeing a 3 there is making me a touch itchy. ^.^;

What about 3F or 3D to signify floating or double floating point in 3-axis? That’s also a common pattern in transformation libraries.

(Khionu Sybiern) #12

It’s not my call, but given we’re using nalgebra, I think we’ll be following their suit, and just using the number.

(OvermindDL1) #13

That works. I’m curious how it distinguishes between type sizes in the math then? In the C++ world I traditionally use the Eigen3 (3 being the revision) library that is fully templated but has default names for a variety of types, such as Vector2f/Vector2d/Vector2i/Vector2cd/Vector2cf/etc… And of course you can define your own types (have a decimal library, or a bignum library, or whatever…). Vector2i (integers) I use quite heavily in my old engine as I have a tendency to do a lot of voxel-style work (tiles in 2d or blocks in 3d) and being able to transform those is amazingly useful. As well as what if a game wants to use floats engine types for the memory savings (fantastic on mobile and web), or another game that wants to use doubles as the base engine type for its greater range handling, or some visualization tool want to use complex float, how do you distinguish those? Will such a system be able to handle any of such types?

(Kel) #14

The naming scheme AlgebraicType{dim} is standard in our math ecosystem currently so It’s not worth bikeshedding over. https://www.nalgebra.org/quick_reference/

EDIT: Also, these types are now generic over the Real trait, defined in the num_traits crate. Which solves your other question. Real is currently implemented for f64 and f32 however in the future this trait could also meaningfully be implemented for fixed point numbers as well.

(Joël Lupien) #15
  1. I’m following nalgebra’s convention. That is, Transform3 for 3D, Transform2 for 2D
  2. We are generic over the number type for transform. The only constraint is that it needs to be a Real (f32 or f64 currently).

I would be interested to know if it is possible to use Vector and Quaternion directly instead of Isometry. This way we might be able to have i32 transforms too?

(OvermindDL1) #16

Ah that’s actually one of my big specific use-cases, awesome. :slight_smile:

Please! Integral transforms are way crazy useful in some situations! It is a feature of Eigen that I’ve used far more than I ever expected!