Irrlicht refactoring checklist

[appgurueu]

I'm wading through Irrlicht code whenever I need to touch something intimately intertwined with rendering like skeletal animations (which need a lot of touching e.g. to fix issues like #14818, #9218, #13043) and expect to do more of that in the future (e.g. for modernizing our pipeline so we can get to enjoy order-independent transparency) so I want to refactor it. I'm glad for anyone who joins me and the work that has already been done e.g. by sfan5 :)

My rough checklist so far:

• Get rid of unnecessary interfaces. For example there is an ISkinnedMesh interface, the only implementor of which is CSkinnedMesh. We should just use CSkinnedMesh directly. There are quite a few instances of this.
• Replace core::string with std::string
• Replace core::array with std::vector
  • Slightly annoying: Irrlicht (ab)uses sorted arrays as on-demand sorted sets. Proper refactoring requires either using an actual sorted set (or sometimes unordered set / map) or figuring out the right time to sort the array so that std::lower_bound / std::upper_bound can be used later on (regrettably std::binary_search just returns a bool).
• Refactor matrices (see some discussion in Refactor matrix4.h #15216). We just need to "draw the rest of the fucking owl": We want column-major 4x4 matrices with mostly functional-style methods. (Let's minimize unnecessary mutable variants of everything more generally.)
• Related to matrices: Introduce transforms - structs which store translation, rotation, and scale. Often matrices are used (and a costly and possible buggy decomposition is done on them) when these values should never have been thrown away to begin with.
• Refactor skeletal animation. The current design couples it with mutable mesh state in a bad way. The data structures should look more like the ones in glTF to lend themselves better to refactoring this to be functional-style so we can eventually let the GPU do at the very least the vertex transformations; the meshes should just expose a function which lets you get a bunch of bone transforms for a given animation frame.
  • The current SWeight structure sucks. We should move to a more glTF-like representation.
• Move the irrlicht changes files into Irrlicht itself, in particular the EnrichedStrings font drawing needs to be refactored.
• Refactor color byte order vs endianness order, see e.g. Fix rendering regression with TGA type 1 files with BGRA8 color #15402
• Consolidate animated & static meshes: There should be just animated (animatable) meshes. Static meshes are just the special case where the animation isn't there, which animated meshes have to support anyways. Similarly there should be just an animated mesh scene node.
• Refactor rotations:
  • Unit test rotation orders and directions of rotations (clockwise or counterclockwise? I would hope everything uses the latter, but I don't know yet)
  • Create a new struct EulerAngles or similar. This should be used consistently. It should document rotation order, direction, and radians vs degrees.
  • Get rid of radians vs degrees internally by only using radians, convert to degrees only when necessary when presenting values to modders as degrees for backwards compat.
• Eventually maybe get rid of the silly I / C / S naming conventions?

Feel free to chime in.

[HybridDog]

Instead of refactoring and extending matrix code I think it should be replaced by an external library, which can simplify the code, add more complex features such as eigenvalue computation, and add platform-specific performance optimisations.

[appgurueu]

Instead of refactoring and extending matrix code I think it should be replaced by an external library

I had considered this, however

which can simplify the code

We wouldn't need to maintain the matrix code, that much is true. However, we'd also need to make all matrix usages use the APIs of whatever external library we decide to use, which would make refactoring this gradually harder.

add more complex features such as eigenvalue computation

Frankly I don't think we need such complex features (yet), there are much more basic issues to resolve first.

We really only need pretty much just 4x4 matrices and pretty basic operations them, not a whole linear algebra library.

platform-specific performance optimisations

Fair point, however I think there are two reasons why this doesn't matter very much: One, the compiler should largely be able to optimize the simple calculations here to use e.g. appropriate SIMD instructions itself. Two, if we're doing that much matrix math on the CPU that it's becoming a performance problem, then it should probably be done on the GPU (for example skinning should be done on the GPU) to begin with.

[nauta-turbidus]

What's bad about the I C S naming conventions?

[appgurueu]

What's bad about the I C S naming conventions?

I find them annoying and not very useful (I could see keeping I, but the other two are basically useless; I don't need a constant reminder that I'm working with classes in C++, no thanks). They also don't match our conventions.

And even inside Irrlicht they don't seem to be used consistently. How come vectors, quaternions, matrices etc. don't suffer from them, for example?

It doesn't matter much (hence why it's an "eventually maybe"), but it also isn't hard to change since renames are cheap with a capable editor. Though the diff noise might be considered a problem.