Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor camera code #474

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Refactor camera code #474

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ab8774b
[camera] Change return type of methods to const reference
IAmNotHanni Jun 19, 2022
e89a9ce
[camera] Update code documentation
IAmNotHanni Jun 19, 2022
54d832e
[application|renderer] Chain calls to camera methods
IAmNotHanni Jun 19, 2022
e444acf
[camera] Make camera code thread-safe
IAmNotHanni Jun 20, 2022
9f3f5c2
[application|renderer] Do not reset camera when resizing window
IAmNotHanni Jun 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
134 changes: 66 additions & 68 deletions include/inexor/vulkan-renderer/camera.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,71 +10,71 @@
namespace inexor::vulkan_renderer {

namespace directions {
/// The default value of the camera's front vector.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we use or not the ending period?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK found it, we use periods at the end of the sentences :D
Already "discussed" on Discord.

/// The default value of the camera's front vector
constexpr glm::vec3 DEFAULT_FRONT{1.0f, 0.0f, 0.0f};
/// The default value of the camera's right vector.
/// The default value of the camera's right vector
constexpr glm::vec3 DEFAULT_RIGHT{0.0f, 1.0f, 0.0f};
/// The default value of the camera's up vector.
/// The default value of the camera's up vector
constexpr glm::vec3 DEFAULT_UP{0.0f, 0.0f, 1.0f};
} // namespace directions

enum class CameraMovement { FORWARD, BACKWARD, LEFT, RIGHT };
Copy link
Member

@IceflowRE IceflowRE Jun 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Iam not a friend of using an enum for indicating a camera movement. I would allow direct manipulations via vectors.
In general i don't really like the current state of this camera class.
This camera class is already highly specified and implemented and thus limited in adding new camera types.
By handling inputs and the resulting camera movement.

I would suggest to make a more general camera class with the most important methods like direction, position, plane, etc.
Then inherit from this base camera and implement a PlayerCamera which has more traits like rotation speed, movement speed etc.

In my eyes is camera movement already part of the game (user) code and not the engine code. We should only provide callbacks or signals to inputs.


// TODO: Implement more camera types.
// TODO: Implement more camera types
enum class CameraType { LOOK_AT };

/// @warning Not thread safe!
/// @warning This class is not thread safe!
class Camera {
private:
/// The type of the camera. Currently only one type is implemented.
/// The type of the camera
CameraType m_type{CameraType::LOOK_AT};
/// The start position of the camera.
/// The start position of the camera
glm::vec3 m_position{0.0f, 0.0f, 0.0f};
/// The vector of direction in which the camera is looking.
/// The vector of direction in which the camera is looking
glm::vec3 m_front{directions::DEFAULT_FRONT};
/// The vector of direction which points to the right.
/// The vector of direction which points to the right
glm::vec3 m_right{directions::DEFAULT_RIGHT};
/// The vector which indicates "upwards".
/// The vector which indicates "upwards"
glm::vec3 m_up{directions::DEFAULT_UP};
/// The world vector which indicates "upwards".
/// The world vector which indicates "upwards"
glm::vec3 m_world_up{directions::DEFAULT_UP};
glm::mat4 m_view_matrix{};
glm::mat4 m_perspective_matrix{};

/// The camera's yaw angle.
/// The camera's yaw angle
float m_yaw{0.0f};
/// The camera's roll angle.
/// The camera's roll angle
float m_roll{0.0f};
/// The camera's pitch angle.
/// The camera's pitch angle
float m_pitch{0.0f};
/// The camera's minimum pitch angle.
/// Looking straight downwards is the maximum pitch angle.
/// The camera's minimum pitch angle
/// Looking straight downwards is the maximum pitch angle
float m_pitch_min{-89.0f};
/// The camera's maximum pitch angle.
/// Looking straight upwards is the maximum pitch angle.
/// The camera's maximum pitch angle
/// Looking straight upwards is the maximum pitch angle
float m_pitch_max{+89.0f};
/// The camera's horizontal field of view.
/// The camera's horizontal field of view
float m_fov{90.0f};
/// The camera's maximum field of view.
/// The camera's maximum field of view
float m_fov_max{90.0f};
/// The camera's minimum field of view.
/// The camera's minimum field of view
float m_fov_min{20.0f};
/// The zoom step when zooming in or out.
/// The zoom step when zooming in or out
float m_zoom_step{10.0f};
/// The camera's rotation speed.
/// The camera's rotation speed
float m_rotation_speed{1.0f};
/// The camera's movement speed.
/// The camera's movement speed
float m_movement_speed{2.0f};
/// The camera's aspect ratio (width divided by height).
/// The camera's aspect ratio (width divided by height)
float m_aspect_ratio{1920.0f / 1080.0f};
/// The sensitivity of the mouse.
/// The sensitivity of the mouse
float m_mouse_sensitivity{0.005f};
/// The camera's near plane.
/// The camera's near plane
float m_near_plane{0.001f};
/// The camera's far plane.
/// The camera's far plane
float m_far_plane{1000.0f};

/// The keys for the movement FORWARD, BACKWARD, LEFT, RIGHT.
/// The keys for the movement FORWARD, BACKWARD, LEFT, RIGHT
std::array<bool, 4> m_keys{false, false, false, false};

float m_vertical_fov{0.0f};
Expand All @@ -90,41 +90,39 @@ class Camera {
[[nodiscard]] bool is_moving() const;

public:
/// @brief Default constructor.
/// @param position The camera's position.
/// @param yaw The camera's yaw angle in degrees.
/// @param pitch The camera's pitch angle in degrees.
/// @param window_width The width of the window.
/// @param window_height The height of the window.
/// Default constructor
/// @param position The camera's position
/// @param yaw The camera's yaw angle in degrees
/// @param pitch The camera's pitch angle in degrees
/// @param window_width The width of the window
/// @param window_height The height of the window
Camera(const glm::vec3 &position, float yaw, float pitch, float window_width, float window_height);

// TODO: Add more overloaded constructors.

/// @brief Set the camera type.
/// @note We will implement more camera types in the future.
/// @param type The camera type.
/// Set the camera type
/// @note We will implement more camera types in the future
/// @param type The camera type
Comment on lines +111 to +112
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// @note We will implement more camera types in the future
/// @param type The camera type
/// @param type The camera type
/// @note We will implement more camera types in the future

Param before notes.

Camera &set_type(CameraType type);

[[nodiscard]] const CameraType &type() const {
return m_type;
}

/// @brief Notify the camera if a certain key is pressed or released.
/// @param key The key which was pressed or released.
/// @param pressed ``true`` if the key is pressed.
/// Notify the camera if a certain key is pressed or released
/// @param key The key which was pressed or released
/// @param pressed ``true`` if the key is pressed
Camera &set_movement_state(CameraMovement key, bool pressed);

/// @brief Set the position of the camera.
/// @param position The position of the camera.
/// Set the position of the camera
/// @param position The position of the camera
Camera &set_position(glm::vec3 position);

[[nodiscard]] const glm::vec3 &position() const {
return m_position;
}

/// @brief Set the aspect ratio (window width divided by window height) of the camera view matrix.
/// @param width The width of the window.
/// @param height The height of the window.
/// Set the aspect ratio (window width divided by window height) of the camera view matrix
/// @param width The width of the window
/// @param height The height of the window
Camera &set_aspect_ratio(float width, float height);

[[nodiscard]] float aspect_ratio() const {
Expand All @@ -135,32 +133,32 @@ class Camera {
return m_fov;
}

/// @brief Set the movement speed of the camera.
/// @param speed The movement speed of the camera.
/// Set the movement speed of the camera
/// @param speed The movement speed of the camera
Camera &set_movement_speed(float speed);

[[nodiscard]] float movement_speed() const {
return m_movement_speed;
}

/// @brief Set the rotation speed of the camera.
/// @param speed The rotation speed of the camera.
/// Set the rotation speed of the camera
/// @param speed The rotation speed of the camera
Camera &set_rotation_speed(float speed);

[[nodiscard]] float rotation_speed() const {
return m_rotation_speed;
}

/// @brief Rotates the camera around x, y, and z axis.
/// @param delta_yaw The yaw angle.
/// @param delta_pitch The pitch angle.
/// @param delta_roll The roll angle.
/// Rotates the camera around x, y, and z axis
/// @param delta_yaw The yaw angle
/// @param delta_pitch The pitch angle
/// @param delta_roll The roll angle
Camera &rotate(float delta_yaw, float delta_pitch, float delta_roll = 0.0f);

/// @brief Set the camera's rotation.
/// @param yaw The yaw angle.
/// @param pitch The pitch angle.
/// @param roll The roll angle.
/// Set the camera's rotation
/// @param yaw The yaw angle
/// @param pitch The pitch angle
/// @param roll The roll angle
Camera &set_rotation(float yaw, float pitch, float roll);

[[nodiscard]] const glm::vec3 &rotation() const {
Expand Down Expand Up @@ -191,28 +189,28 @@ class Camera {
return m_right;
}

/// @brief Set the near plane distance of the camera.
/// @param near_plane The near plane distance.
/// Set the near plane distance of the camera
/// @param near_plane The near plane distance
Camera &set_near_plane(float near_plane);

[[nodiscard]] float near_plane() const {
return m_near_plane;
}

/// @brief Set the far plane distance of the camera.
/// @param far_plane The far plane distance.
/// Set the far plane distance of the camera
/// @param far_plane The far plane distance
Camera &set_far_plane(float far_plane);

[[nodiscard]] float far_plane() const {
return m_far_plane;
}

/// @brief Change the zoom of the camera.
/// @param offset The mouse wheel offset change.
/// Change the zoom of the camera
/// @param offset The mouse wheel offset change
Camera &change_zoom(float offset);

/// @brief Update the camera (recalculate vectors and matrices).
/// @param delta_time The change in time since the last frame.
/// Update the camera (recalculate vectors and matrices)
/// @param delta_time The change in time since the last frame
Camera &update(float delta_time);

[[nodiscard]] const glm::mat4 &view_matrix() {
Expand Down