[rcamera] Redesign

[Crydsch]

The current implementation of the camera module has some issues.
It uses raylib-dependant input detection and is overall quite complex.
I also noticed that the CAMERA_FREE mode uses a "standard 3d-content-creation scheme" and does not actually
behave like a free camera one would expect in a space game or flight simulator.

To improve the camera module I talked to @raysan5 about a quaternion-based redesign I would like to work on.
By utilizing quaternions we naturally avoid gimbal-lock, and can simplify the camera code.
It also reduces the memory footprint.

Requirements

• Single-file header-only
• No external dependencies, only libc
• Engine-agnostic, no dependencies on external types or functions
• Minimal global state variables
• Support camera modes: Free, FP, TP, Orbital

Also, UpdateCamera() should not perform it's own input detection, but rather get the current input state passed by the caller.

Redesign

After some prototyping I came up with the following design:

Camera Modes

I found out that the four camera modes are related to each other, which allows great simplification.

CAMERA_FREE is completely unrestricted.
CAMERA_FIRST_PERSON is a CAMERA_FREE without roll and locked on the up-axis (to prevent over rotation when looking up/down).
CAMERA_THIRD_PERSON is a CAMERA_FIRST_PERSON focused on a target. (And potentially further locked to prevent looking up).
CAMERA_ORBITAL is a CAMERA_THIRD_PERSON with a locked target. (And potentially auto-rotation).

Note: The new CAMERA_FREE is different from the old one. If a panning camera mode similar to the old free mode is wanted, it can simply be added as another mode. Even though I think a (non auto-rotating) orbital camera is even better in that respect.

API

To ease the usability we provide functions, such as:
(Naming WIP)

MoveCamera(Camera3D *camera, Vector3 offset)
TurnCameraYaw(Camera3D *camera, float rad)
TurnCameraPitch(Camera3D *camera, float rad)
TurnCameraRoll(Camera3D *camera, float rad)
...

With such functions we are independent from any input system and one can easily create a custom camera mode.

To be fully engine-agnostic the camera may also provide functions to get the camera matrices:
GetCameraViewMatrix()
GetCameraProjectionMatrix()

With the former functions in place, raylib/the user could manipulate the camera based on input and allow custom manipulation.
This would probably make an UpdateCamera() function obsolete.
Alternatively we could pass input information in an engine-agnostic way (enums?) to UpdateCamera()
and handle the necessary calls internally. (This actually seems more complicated to me).

Feedback

Please let me know what you think about this design.
I have some prototyping code ready and can start preparing a PR.

[raysan5]

@Crydsch thank you very much for writting down the issue... it requires some further thinking...

[Crydsch]

No worries, take your time.
If anything is unclear, I will gladly explain further.

[Crydsch]

Hey, it's been two weeks now, i really don't want to pressure you, but have you had time to think about it?
I'd really like to start working on this for my university course.

If you think a bigger API change would be too disruptive now, I can start to implement the internals only and leave the API as is.
I would only extend the existing API and keep the raylib-dependend input handling for now.

Is this acceptable?

[orcmid]

I have a question and suggestion.

Can you make the changes you want in include file(s) separate from raylib that map what you want over raylib/raygui ?

That way you can build with those procedures, confirm operation, and satisfy yourself. Then, depending on how Ramon is satisfied, you can remove the extension once they are part of raylib/raygui for direct use and/or cope with naming differences. And you don't need some sort of lock-step coordination that depends on changes to raylib before you can advance your work.

This should let you also deal with an expanded DLL and bindngs you want from other languages.

[chriscamacho]

I've just looked at my camera code in a long term project I'm doing.

As I wanted a free fly camera that can toggle between fly and using the mouse with a 2d GUI, I had a number of challenges!

I basically had to abstract (and then mostly rewrite) the camera first person mode (camera update isn't called during gui mode). Swapping camera modes (gui mode and back) meant I needed to access private members like camera angle x,y and also had to allow for mouse input rather than directly using GetMousePosition, not to mention the view bobbing that cannot be customised presently...

I'd strongly recommend getters and setters to most if not all the private CAMERA variable struct, to maximise flexibility

This will make "custom" camera modes actually possible.

Not coupling to raylib / raymath is needless (no one is realistically going to use rcamera.h outside raylib) and hardcoding copies of raymath functions its a perfect opportunity to introduce bugs/inconsistencies with the math, that had be puzzled for a short while....

[raysan5]

Hi @Crydsch, excuse my late response, I'm a bit busy lately.

Provided low-level function look nice to me, some review:

// Camera inputs state must be set every frame, it could be called by a higher-level function like UpdateCamera() in raylib core module
void SetCameraInputs(int *currentKeysDown, int keyCount, Vector2 currentMousePos, int *currentGamepadButtonsDown, int gamepadButtonCount, Vector2 currentGamepadAxisValue); 

void CameraMoveForward(Camera3D *camera, float distance);
void CameraYaw(Camera3D *camera, float deg);
void CameraPitch(Camera3D *camera, float deg);
void CameraRoll(Camera3D *camera, float deg);

Matrix GetCameraViewMatrix()
Matrix GetCameraProjectionMatrix()

To decouple inputs from camera, a function to receive input state every frame should be available. Input state could also be managed internally by the camera (current/previous states...).

To start with, we can use raylib provided data types: Camera3D, Vector2, Matrix, it's the easier apporach to start but they can be replaced by custom types later on and map them to raylib when required (i.e. in UpdateCamera() function in core module)

To start with, we can use raymath provided functions but, again, required functions can be moved to internal rcamera module later on, that way the module will be portable and self-contained. Here it is a great reference.

Current camera module abuses of lots of defines, some of them confusing, it will be nice to move some of those configuration values to variables accesible to the user for runtime configuration.

Side note: Current camera module was originally implemented in 2014 by a maths student, I redesigned-reviewed it several times but the core Euler maths used are still there.

[Crydsch]

@orcmid Thank you for the suggestion, but that will not work as I want to change internals and not just add onto them.

@chriscamacho Some valid points!
Maybe I did not understand correctly, but I am not familiar with a 3D camera and Gui in the same context.
Is a Gui not always 2D?

Anyway, getters and setters should always be available in raylib as you can just access the structs yourself.
(Except in other languages than C...)
But something alike is planned: Functions like SetCameraPosition(..) would essentially be a setter.
I do plan to allow for custom modes, especially with generic functions such as MoveCamera(..) or TurnCamera(..).

I imagine you start out with a base mode such as FREE or FP to determining how the camera should handle rotations (ie allow roll or not) and then it is up to you to call move and turn functions.

Regarding the coupling with raymath:
I agree that copying the math functions is not the best solution, but i have the requirement to be engine-agnostic.
Only relying on raymath would be the perfect compromise. If someone wants to use it outside of raylib they
only have to copy rcamera.h and raymath.h and we get have everything were expected.

@raysan5 Thank you for your response. With that i feel confident starting with it :)
I agree with using raymath and raylib datatypes for now and potentially changing them later.
Thank you for the reference, that looks very usefull.

I have only one question regarding the suggested SetCameraInputs(..) function.
Every framework/engine defines their inputs differently. Maybe some enum like this would work:

enum {
    MoveForwardKey,
    MoveBackwardKey,
    MoveRightKey,
    MoveLeftKey,
    ...
}

That way we can pass just an array (int pointer) and the user can define their own key mapping.
(With powers of two we might even get away with a bit per key in a long)

Is this what you had in mind?

[raysan5]

@Crydsch Actually I don't know what's the best way to allow custom inputs mapping, complexity for that option could scalate quickly! For example, in case we consider gamepad/touch inputs for the camera. In any case, it would be a great addition. 3d camera support for touch devices has been on my TODO list forever! :D

Like the other parts, we can start with a simple approach and scalate it later on. Defining an enum with the available camera actions and map them to an array of values looks ok for now. We can start with an array of int values and scalate it to InputAction values later on when required.

[chriscamacho]

@Crydsch the gui is 2d, but you need the mouse for it, which is in use during mouse look mode... accounting for the mouse and swapping "modes" with the current camera update is problematic for a number of reasons, hence the need for my own update camera function...

a number the the variables needed for a custom camera are not accessible as they are inside a static struct inside rcamera.h

Do take care with Quats and don't assume they are the miracle cure for gimbal lock as even with quats if implemented wrong, you can still come unstuck.

Attempting a one solution fits all solution, leads to inflexibility and what we have today with rcamera.h (hence its persistence too)

I think it would be better to leave the user with MatrixLookAt and the camera public struct with reduced members, basically just what BeginMode3D needs, leave UpdateCamera in as deprecated with a warning message to the console just shown the first time UpdateCamera is run (but make it an obvious multi line warning!)

A few simple examples for first person on a plane (Eulers), 3d fly mode (full YPR rotations with quats) and maybe one or two others - will have a minimal amount of code for their camera update, and as the end user is doing it for themselves, a novice from an example, or a more seasoned coder from their own experience, then there will be far more flexibility and a much more robust camera system.

Another benefit of this is that it will help people learn about just what is going on with the 3d maths rather than glossing over it with some magic black box function...

[Crydsch]

I see you meant the static variables that are not accessible. I agree and I would like to get rid of that.

I am aware of the difficulties of quats and they are not a miracle cure. But I still think they allow for a very nice camera and will try to implement it this way. If you are more experienced with quats, I would appreciate a review of my code then.

The one 'solution fits all' came up naturally while prototyping. Maybe you change your mind once you see it. I do not think it leads to inflexibility.

I like the idea of a deprecated message. I will see, if I can leave the original 'UpdateCamera' in and integrate it for a transition period.

[meshula]

Camera math is surprisingly complex. In case it's of use as reference, I have fully worked and mature camera math at the link below, as well as interactive camera controllers that are independent of the input system. It has all the usual "editor" camera modes, "cinema" modes, and the usual modes, like look at. https://github.com/meshula/LabCamera (Not suggesting you use it, but if you're looking for a how do I do X reference, it's all there :) )

[raysan5]

Wow! This is a great reference! Thanks for sharing! :)

[orcmid]

@orcmid Thank you for the suggestion, but that will not work as I want to change internals and not just add onto them.

@Crydsch please understand me. This is a way to stage and confirm API changes by first building atop the existing APIs, then reverting to the updated original API where it has been updated to match. I assure you that I have done this in a software project as part of getting from a 1.0 API to a 2.0 API and it worked just great. It also let developers start using the 1.0-bis API addition early without getting blocked waiting for the stable 2.0 to use.

You are not obligated to try this of course. However, you are relying on an open-source project and you cannot assert schedule dependencies :). Don't break your own heart.

[Crydsch]

I think I understand it now. Still such a staging might not be necessary for raylib as a project. I can implement any changes on my own fork and ray can merge the PR when its acceptable. Nevertheless, I like the idea to have API changes in stages and will try to make it happen. Thanks for the explanation.

[orcmid]

While we're talking about this, you might find that the Windows APIs were often updated without obsoleting the previous version for backwards compatibility reasons. There might be the original mumble function and also mumbleX. for example, Deprecation might happen later or never, depending. When COM and interfaces were introduced there were even more ways to version APIs although there were breaking occasions (not getting the memo, apparently).

[chriscamacho]

I honestly do not see camera maths as somehow "complex" yes the current updateCamera seems complex until you isolate the modes, and replace the manually implemented matrix code with calls to raymath...

I really do not see it too burdensome for the end user to do the updating they want (or should there be a player controller with fixed complex mash up of modes too?)

[Crydsch]

Yes, by "complex" I meant that you have to take a deeper look and "isolate the modes" etc.. to make sense of it - not as in complex to understand code. I plan to write easy to understand code, especially as raylib is meant as an educational tool!
The end user should be able to do the updating they want - I agree, hence I proposed the low-level functions and data accessibility.

[Crydsch]

Camera speed and FrameTime

Any game that should run on a different machine than the on it was developt on,
should account for the (dynamic) FrameTime when handling user inputs.
This way a character moves with the same speed regardless the fps.

This is true for camera systems as well, especially in conjunction with camera movement.
Here are my thoughts on this regarding the camera redesign.
Basically - at which rate should the camera move/rotate on a simple key-down input?
This is rather difficult.

I do not think we should hide some magic value in a define. This should be user configurable.
Then again, we also would need to factor the FrameTime into it as well.
But! Querrying the FrameTime is engine specific and should not be part of the camera code (i think).

My current solution would look like this:
We have low-level functions allowing precise changes.
This gives the user precise manipulation if needed; like CameraYaw(&cam, 90 * DEG2RAD);.
This way they can implement their own custom camera speed * frameTime, and the camera system
is engine agnostic.

For raylib internally, we could then prepare everything in CameraUpdate().
There we could then also call GetFrameTime() and factor this in.

Still I am not entirely sure where/how to save/allow user defined camera speeds..
Maybe we actually have to use some sane, hard coded value and if the user requires a different speed, they can use the low-level functions directly?

What do you think about this problem?

[chriscamacho]

Just have the speed as part of the camera struct that can be directly accessed by user code (ie not an internal struct) similarly frame time (a float as a fraction of a second) should be set by the user, either by GetFramTime() or by any other similar function from which ever engine it is being used with.

[Crydsch]

That's one possibility, but I fear that other functionality might need to store some state too:

• camera move speed
• camera rotation speed
• view bobbing state
• ...

This could inflate the struct quite a bit.
Further, the FrameTime changes from one frame to the next, so the user would have to adjust this value every frame.
Then we do not have to store it at all. The user can just call the appropriate function and pass it as a parameter.

[raysan5]

I personally prefer to keep Camera struct as simple as possible. Even adding the mode parameter is not clear to me, I feel it doesn't belong to Camera data but just a Camera movement processing parameter that can, maybe, be resolved at runtime.

[Crydsch]

Ok, I will try to keep the struct minimal! The mode is necessary, since we need to handle updating the camera differently depending on the current mode. Maybe it can be resolved at runtime, but I think it cannot.

[chriscamacho]

well you have to put it somewhere, better to make it accessible than hide it away, and as its only a one off (or at most 3-4 cameras) it really doesn't matter the size, so what if its 2kb or even 20kb....

[Crydsch]

Absolutely, if we are going to save them, then in the struct, accessible for the user!
What I wanted to note was that, if the user has to adapt this value every frame, then we can also just make it a parameter.
I guess are right about the size of the struct though. Maybe I am just thinking too memory conservative :)

[raysan5]

Personally I prioritize simplicity over a full-featured API exposing lots of options and features. raylib is designed to be customizable to user specific needs, that's also the philosophy behind the camera define values. Users can just tweak those values for their needs, recompile raylib and use it.

Adding a big API or exposing many options just makes it more complex to use and maintain, specially for the binding developers that need to expose all that extra functionality that, probably, only a few users will need (coincidentally, probably the ones that don't care editing the defines directly in raylib and recompiling).

This is my point of view always: Keey it Simple, Avoid Overengineering, Less is More... most raylib users don't want and don't need full control.

[Crydsch]

New Input System ?

From the beginning on of this redesign we talked about a new input system for various reasons.
But now, the more I think of it the harder it gets. Please let me elaborate:

The new system should handle (engine agnostic!) inputs and manipulate the camera accordingly.
We also talked about frame time compensation, which should be added as well.
On top of that we want to enable all kinds of inputs (keyboard, mouse, gamepad, touch), adding to the complexity.

In general we have 2 kinds of inputs:

• Discrete inputs (keyboard, buttons):

  • either pressed or not
  • translated to predefined speed

• Continuous inputs (mouse, gamepad, touch):

  • float value (allows for custom speeds also)
  • needs input sensitivity adjustment (hardware specific!)

Both kinds need frame time compensation.

Take 1: handle input states

We could get all kinds of input states directly and handle which buttons are currently pressed and resolve deltas internally.
This has some major implications though. Now we need to save the previous state!
Either in some static variable again (we want to remove them not add some) or in the camera struct (would blow up).

Also these kinds of input handling is typically already available in every engine.
We should not replicate existing functionality.

=> From now on I assume we get the delta values passed directly.

Take 2: camera actions

We could implement engine agnostic discrete inputs with an enum of camera actions.
void SetCameraInputs(int *actions, int countActions);.

Still we would need to get any continuous inputs as floats directly.
void SetCameraInputs(int *actions, int countActions, Vector2 mouseDelta);.

Then we want to support all other kinds of inputs.
void SetCameraInputs(int *actions, int countActions, Vector2 mouseDelta, Vector2 gamepadMoveAxisDelta, Vector2 gamepadRotateAxisDelta, ...);.

And we want frame time compensation, which is engine specific, so it needs to be passed too.
void SetCameraInputs(float frameTime, int *actions, int countActions, Vector2 mouseDelta, Vector2 gamepadMoveAxisDelta, Vector2 gamepadRotateAxisDelta, ...);.

This is very complex and not usable. You were right when you said it would escalate quickly ;)

Note that we just combined all discrete inputs into one array of actions.
We could combine the float inputs similarly.
void SetCameraInputs(float frameTime, int *actions, int countActions, Vector3 move, Vector3 rotate);.

Now that the user is providing the inputs contiuously anyway, we dont need the actions anymore.
And while they factor in the input sensitivity, it just makes sense to factor the frame time compensation in as well.
This leaves us with:

void SetCameraInputs(Vector3 move, Vector3 rotate);.

=> This looks awfully similar to the low-level API!

Take 3: a step back

We wanted this new input system to be engine agnostic and we want to provide ease of use from within raylib.
I think its valid to guess that any user, making use of this camera system in another engine,
will want to have fully control over it anyway and will use the low-level API directly.

In raylib we only make use of the UpdateCamera(&camera); function.

Solution

At this point I am not sure if we really need new input system on top of the low-level API.
In raylib we can handle all kinds of input in the UpdateCamera(&camera); function by leveraging the existing input handling modules.

bool IsKeyDown(int key);
Vector2 GetMouseDelta(void);
float GetGamepadAxisMovement(int gamepad, int axis);
Vector2 GetGestureDragVector(void);
float GetFrameTime(void);
...

We can also apply frame time compensation here as well.

If a raylib user wants access to the low-level API they just have to #import rcamera.h.
Outside of raylib we provide the low-level API only, which is all they want to use anyway.

Any additional system in between, will just complicate things.

As you said "Keep it Simple, Avoid Overengineering, Less is More".

Maybe I am missing something, or am seeing this wrong somewhere.
Please let me know! :)

[Pirolye]

One thing you didn't mention but probably already have been thinking about it is the heavy use of static variables when some of them could be placed inside the camera struct. I have spent a long time investigating an issue with my multiple camera setup and camera switching only to realize it stems from the CameraData being a static variable instead of being inside the Camera3D.

I will probably modify my local copy of raylib and "fix" this but spending some time on this probably wouldn't hurt overall.

[Crydsch]

That's actually a big reason for the redesign. We want to eliminate all static variables.

[raysan5]

@Crydsch @Pirolye This PR already looks very good and it's ready for merging but it's a big/breaking change in current camera system and I'm just waiting for next release approach and some further review.

[Crydsch]

Sounds good :)
Sadly I will not have much time in the next month as the new semester began..

4 Things are currently still on the TODO list. Here are my thoughts about those:

1. CAMERA_ORBITAL autorotation
   Should be a "one-liner" in UpdateCamera, basically
   if (mode == CAMERA_ORBITAL ) CameraYaw(camera, ORBITAL_ROTATION_SPEED, true);
2. Review stand alone mode
   I did this from time to time while working on it, so this point is basically done.
   I left it unchecked to indicate that a final review is still necessary.
3. Check all examples for correct functionality/info boxes
   Same as with 2. I checked all of them repeatedly and they should compile and work fine.
   Multiple were adjusted and perhaps new ones added or smth. This would require review.
4. Frame time compensation
   The last real issue. Currently there is no frametime compensation, which is already available in the old version.
   This would be a step back. Still, with the redesign it should be rather simple to add.

I set the PR to ready for review so it can be merged in the future.
I`ll be available for questions should anything be unclear, though I won't be able to contribute much code..

Take your time and review it whenever - will be great to have it merged in an upcoming version :D