Mathematical Camera class with types of Eigen library

Scope

I cannot fully comment on use of the class or the linear algebra involved, because:

1. It's not clear how it will be used
2. There are no real operations shown, just setup/config of the class plus "camera move".

General

• Code compiles fine on a recent compiler (I am using clang-9 ). Only 2 warnings with -Wall -Wextra:

camera.cpp:110:9: warning: private field 'm_near' is not used [-Wunused-private-field] float m_near; ^ camera.cpp:111:9: warning: private field 'm_far' is not used [-Wunused-private-field] float m_far; ^ 

• Given you are using gcc7.4 and that I assume you have -std=C++17 enabled, then I do not believe you need EIGEN_MAKE_ALIGNED_OPERATOR_NEW. As documented here and explained in more detail in this bugzilla entry. All of Eigen's aligment challenges are solved in recent C++17 compilers apparently. Your question mentions C++11, but if you can you should be compiling with -std=c++17 to get best behaviour from Eigen and have access many other useful features. The only reason to not do this would be if you have some other 3rd party dependency which won't compile in C++17 mode, or if you also need to support platforms with older , more limited compilers, where C++17 is not available.
• You are using double for all your vector/matrix coefficients, yet you are using float for m_width and m_height and therefore AspectRatio(). Is there a specific reason? If not I would just standardise on double, which can often be faster on modern hardware with FP co-processors.
• Instead of your typedefs you could consider writing this, which is considered "more modern practice":

using Vector9d = Eigen::Matrix<double, 9, 1>; 

• Your constructors and operator methods look fine to me
• You could get "extra points" for C++ style by making your "getter" methods const and [[nodiscard]] like this:

 [[nodiscard]] const Vector3d& Forward() const { /* ... */ }; [[nodiscard]] const Vector3d& a() const { /* ... */ }; [[nodiscard]] const Vector3d& up() const { /* ... */ }; [[nodiscard]] Matrix32d A() const { /* ... */ }; [[nodiscard]] Matrix32d R11() const { /* ... */ }; [[nodiscard]] Matrix32d R12() const { /* ... */ }; 

• When you compile for performance (ie "Release mode") ensure you pass -DNDEBUG=1 -O3 similar to below. The NDEBUG bypasses runtime checks on matrix dimensions and -O3 will bring huge benefits, especially with Eigen's expression templates.

g++ -DNDEBUG=1 -O3 -Wall -Wextra -std=c++17 -I include/eigen/ -o build/camera apps/camera.cpp 

Memory structure: layout, allocation and operations

You seemed concerned about speed / copying / memory usage and therefore whether you should be using pointers.

Firstly the vectors/matrices you are using are fixed size and very very small. The biggest one is 9 elements of doubles which are 8 bytes each so 72 bytes (you don't seem to use the Matrix92d). You don't appear to have std::vectors or other collections of these vectors/matrices or of the camera object itself.

Your concern seems to be related to "moving and therefore changing the camera object often". You have decided to recompute the internal m_r1 and m_r2 during m_setCamVec which is called from eg operator-=.

I have read up on the Eigen operations and studied how Eigen stores its structures. They are very simple: essentially, under the hood they are C-style arrays. On my machine, sizeof (Camera) reports that your objects will be 208 bytes, which is very manageable on the stack. This includes all the vectors/matrices which are all packed into the object (no heap allocations here).

If you needed to pass one or a few Camera objects around between functions, you should pass them by reference. Not least because Eigen recommends that for its structures.

If you were to make thousands of Camera objects then these should probably go on the heap, and that would happen naturally if you made a std::vector<Camera> of them. But even then, each Camera object on the heap would contain all its 208 bytes of vectors/matrices within it.

When you call operator+= or other methods which reposition the Camera it looks like there are just a few assignment statements for m_position etc and then recalculateRS() which does 2 tiny cross-products and assignments.

Therefore, there is no need to worry about copying during the recalculation and re-assigning to the member fields. Apart from any possible temporaries, (which Eigen tries to minimise), Eigen and C++ will use the same memory which is already part of your 208-byte Camera object. Very little (if any!) extra memory will be used and everything (!) will be cleaned up each time you reposition the camera object.

Expected costs

This should all be super fast because:

• You have chosen a good quality linear algebra library
• You are using fixed sized matrices
• Your matrices are very small
• If you compile with -O3 -DNDEBUG=1 then Eigen & the compiler should be able to optimise away any temporaries.
• There are no heap allocations here at all (unless you have thousands of camera objects). Also, if Eigen cannot avoid a temporary, it will likely use the stack for it:

EIGEN_STACK_ALLOCATION_LIMIT - defines the maximum bytes for a buffer to be allocated on the stack. For internal temporary buffers, dynamic memory allocation is employed as a fall back. For fixed-size matrices or arrays, exceeding this threshold raises a compile time assertion. Use 0 to set no limit. Default is 128 KB.

you can further force this with:

EIGEN_NO_MALLOC - if defined, any request from inside the Eigen to allocate memory from the heap results in an assertion failure. This is useful to check that some routine does not allocate memory dynamically. Not defined by default.

Note that, if Eigen did allocate memory (very unlikely from what we have seen of your application), then Eigen will free that memory as soon as it's not needed. This is the standard behaviour in C++.

Better to store or construct?

Impossible to tell without knowing the usage pattern. Need to know the actual number of times camera is repositioned vs the number of times these quantities would have to be computed if they were not stored. Neither option seems expensive. Since there is no malloc, this is just two SIMD optimised cross products of tiny matrices. I have no idea how long that takes but if it's longer than a couple of hundred cycles I would be surprised.

Performance mockup

I hacked this basic loop to get an idea. The escape() is to avoid the compiler just removing our program altogether.

static void escape(void *p) { asm volatile("" : : "g"(p) : "memory"); } int main() { Vector9d init = Vector9d::Random(); auto cam = Camera{init, 2, 1, 3, 4}; std::cout << cam.Position() << "\n"; for (std::size_t i = 0; i < 1'000'000; ++i) { cam += init; Matrix32d r12 = cam.R12(); escape(&r12); } std::cout << cam.Position() << "\n"; return 0; } 

On my (quite old i7 2600) machine with -O3 -DNDEBUG that runs in 65ms. So a single camera move takes around (very roughly!) 65ns. Here is the, slightly modified to eliminate the std::cout printing, version in machine code. A few hundred lines of assembly, packed full of SIMD instructions. Scary fast?

The bottlenecks for your application are likely to lie elsewhere in your calling code. In general you are slightly in danger of "premature optimisation" concerns. It's not at all clear that there is anything expensive here. Need to measure first when application is up and running.

I hope that helps. I had to make some assumptions. Come back with comments if they were incorrect.