Skip to content

Add camera class and update .gitignore#4

Merged
OwlWorksInnovations merged 3 commits into
masterfrom
dev
Mar 18, 2026
Merged

Add camera class and update .gitignore#4
OwlWorksInnovations merged 3 commits into
masterfrom
dev

Conversation

@OwlWorksInnovations

Copy link
Copy Markdown
Owner

No description provided.

@qodo-code-review

Copy link
Copy Markdown

Review Summary by Qodo

Add camera system with 3D cube rendering

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Implement camera class with keyboard and mouse controls
• Refactor mesh from 2D quad to 3D cube with proper vertices
• Add mouse input tracking and delta calculations
• Integrate camera into rendering pipeline with view/projection matrices
Diagram
flowchart LR
  Input["Input System<br/>Keyboard + Mouse"] -->|"ProcessKeyboard<br/>ProcessMouse"| Camera["Camera Class<br/>Position, Yaw, Pitch"]
  Camera -->|"GetViewMatrix<br/>GetProjectionMatrix"| Shader["Shader<br/>View/Projection"]
  Shader -->|"Render"| Mesh["3D Cube Mesh<br/>24 Vertices"]
  Engine["Engine::Run"] -->|"Update"| Input
  Engine -->|"Use"| Shader
Loading

Grey Divider

File Changes

1. src/Render/Camera.hpp ✨ Enhancement +27/-0

Camera class header definition

• New camera class header with position, yaw, pitch, and speed parameters
• Public methods for view/projection matrix generation
• Keyboard and mouse input processing methods
• Private vectors for front, up, right directions with update mechanism

src/Render/Camera.hpp


2. src/Render/Camera.cpp ✨ Enhancement +48/-0

Camera implementation with input handling

• Constructor initializing camera position and updating direction vectors
• GetViewMatrix using glm::lookAt for view transformation
• GetProjectionMatrix with perspective projection and aspect ratio
• ProcessKeyboard handling WASD, Space, and Ctrl for movement
• ProcessMouse updating yaw/pitch with gimbal lock prevention
• UpdateVectors recalculating front, right, up from euler angles

src/Render/Camera.cpp


3. src/Core/Input.hpp ✨ Enhancement +6/-0

Input class mouse tracking interface

• Added GetMouseDeltaX and GetMouseDeltaY public methods
• Added window pointer, mouseDeltaX, and mouseDeltaY static members

src/Core/Input.hpp


View more (3)
4. src/Core/Input.cpp ✨ Enhancement +30/-10

Input system mouse delta tracking

• Store window pointer for mouse position queries
• Track mouse position with firstMouse flag for initialization
• Calculate and store mouse delta values each frame
• Added glfwSetInputMode for cursor capture in Init
• Minor formatting improvements and comment updates

src/Core/Input.cpp


5. src/Core/Engine.cpp ✨ Enhancement +58/-28

Engine integration with camera and 3D cube

• Include Camera.hpp header
• Enable cursor capture mode in InitWindow
• Replace 2D quad vertices with 3D cube (24 vertices, 6 faces)
• Update indices for cube face rendering
• Instantiate Camera with initial position
• Add deltaTime calculation for frame-rate independent movement
• Integrate camera keyboard and mouse processing
• Use camera matrices for view and projection transformations
• Calculate aspect ratio from framebuffer size
• Add ESC key binding to close window

src/Core/Engine.cpp


6. .cache/clangd/index/stb_image.h.A00D440F907B407F.idx Additional files +0/-0

...

.cache/clangd/index/stb_image.h.A00D440F907B407F.idx


Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented Mar 18, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0) 📐 Spec deviations (0)

Grey Divider


Action required

1. Aspect divide-by-zero 🐞 Bug ⛯ Reliability
Description
Engine::Run computes aspect = w / h without guarding against h == 0, which can produce inf/NaN
and feed an invalid projection matrix into the shader. This can break rendering whenever framebuffer
height is reported as 0.
Code

src/Core/Engine.cpp[R117-123]

+    int w, h;
+    glfwGetFramebufferSize(window, &w, &h);
+    float aspect = (float)w / (float)h;
-    shader.SetMat4("u_model", model);
-    shader.SetMat4("u_view", view);
-    shader.SetMat4("u_projection", projection);
+    shader.SetMat4("u_model", glm::mat4(1.0f));
+    shader.SetMat4("u_view", camera.GetViewMatrix());
+    shader.SetMat4("u_projection", camera.GetProjectionMatrix(aspect));
Evidence
The code divides by h unconditionally and then uses the result to build a projection matrix via
glm::perspective, with no validation at either call site.

src/Core/Engine.cpp[117-123]
src/Render/Camera.cpp[13-15]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`Engine::Run()` divides by framebuffer height (`h`) to compute aspect ratio without checking for `h == 0`. If `h` is 0, the aspect becomes inf/NaN and produces an invalid projection matrix.
### Issue Context
`glfwGetFramebufferSize()` can return transient zero sizes (e.g., during resize/minimize). Passing an invalid projection matrix to the shader can corrupt rendering.
### Fix Focus Areas
- src/Core/Engine.cpp[117-123]
### Suggested change
Add a guard before computing aspect/projection, e.g.:
- If `w == 0 || h == 0`, skip rendering this frame (or clamp to a safe default) and continue the loop.
- Only call `GetProjectionMatrix(aspect)` when `h &amp;amp;gt; 0`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. First-frame deltaTime spike 🐞 Bug ✓ Correctness
Description
Engine::Run initializes lastTime to 0, so the first frame computes deltaTime as glfwGetTime()-0 and
can move the camera an unexpectedly large distance on startup. This directly affects
Camera::ProcessKeyboard, which multiplies movement by deltaTime.
Code

src/Core/Engine.cpp[R97-110]

+  float lastTime = 0.0f;
+
while (!glfwWindowShouldClose(window)) {
-    // Shader
-    shader.Use();
+    float currentTime = (float)glfwGetTime();
+    float deltaTime = currentTime - lastTime;
+    lastTime = currentTime;
-    // Input
  Input::Update();
-    // Clear the screen
+    if (Input::IsKeyPressed(GLFW_KEY_ESCAPE))
+      glfwSetWindowShouldClose(window, true);
+
+    camera.ProcessKeyboard(deltaTime);
+    camera.ProcessMouse(Input::GetMouseDeltaX(), Input::GetMouseDeltaY());
Evidence
Engine::Run sets lastTime=0 and immediately uses it to compute deltaTime, then passes that value
into camera movement. Camera movement scales linearly with deltaTime, so an inflated first deltaTime
causes an inflated position change.

src/Core/Engine.cpp[95-110]
src/Render/Camera.cpp[17-32]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`Engine::Run()` sets `lastTime` to `0.0f`, so the first computed `deltaTime` includes all time since GLFW initialization, which can cause a noticeable first-frame camera jump.
### Issue Context
Camera motion is scaled by `deltaTime` (`velocity = speed * deltaTime`). A too-large first `deltaTime` yields a too-large first movement step.
### Fix Focus Areas
- src/Core/Engine.cpp[95-103]
### Suggested change
Initialize `lastTime` from `glfwGetTime()` right before entering the loop, e.g.:
- `float lastTime = (float)glfwGetTime();`
or compute `deltaTime` with a special-case for the first frame.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Tracked clangd cache file 🐞 Bug ⚙ Maintainability
Description
A clangd-generated index file under .cache/clangd/index is committed/modified, creating large
noisy diffs and repo bloat. .gitignore already ignores .cache/, but tracked cache files will
continue to be versioned unless removed from the index.
Code

.cache/clangd/index/stb_image.h.A00D440F907B407F.idx[R1-8]

+RIFF����CdIxmeta��������stri����_d��x��=ks�6��<���ڻXZ�������^
+Ǜ�U�I9���s�X
+�������F3[�߯���
+�3���ly9D��h��~�T����(�*�VR,�"U�,�\�����������z+�m��:��-T#�lU֪Y��g��	!T����^..D�����hJ�GM��q&�:ۊ
+
+]����@��z��
+��y������Չ���f��p��4j�&��V������^���]{���/�<9���&�?�ӱ�M��~���'&@\-��H�n�"�H�x�b�������x���?����?N�����/�㒜�Q����Txl
+�:*
Evidence
The repo’s .gitignore is configured to ignore .cache/, yet the .cache/clangd/index/...idx file
exists in the repository contents and is binary-like (RIFF header), indicating it is an editor/tool
artifact being tracked.

.gitignore[1-4]
.cache/clangd/index/stb_image.h.A00D440F907B407F.idx[1-5]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A clangd index file in `.cache/` is being tracked and modified in the PR. Even though `.cache/` is in `.gitignore`, gitignore does not stop updates to already-tracked files.
### Issue Context
These files are machine-generated and typically large and frequently changing, which makes PR diffs noisy and bloats the repository.
### Fix Focus Areas
- .gitignore[1-4]
- .cache/clangd/index/stb_image.h.A00D440F907B407F.idx[1-5]
### Suggested change
1. Remove tracked cache files from git index (but keep them locally):
- `git rm -r --cached .cache/`
2. Commit the removal.
3. Ensure `.gitignore` continues to include `.cache/` (it already does) so the files don’t reappear.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Comment thread src/Core/Engine.cpp Outdated
Comment on lines +117 to +123
int w, h;
glfwGetFramebufferSize(window, &w, &h);
float aspect = (float)w / (float)h;

shader.SetMat4("u_model", model);
shader.SetMat4("u_view", view);
shader.SetMat4("u_projection", projection);
shader.SetMat4("u_model", glm::mat4(1.0f));
shader.SetMat4("u_view", camera.GetViewMatrix());
shader.SetMat4("u_projection", camera.GetProjectionMatrix(aspect));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Aspect divide-by-zero 🐞 Bug ⛯ Reliability

Engine::Run computes aspect = w / h without guarding against h == 0, which can produce inf/NaN
and feed an invalid projection matrix into the shader. This can break rendering whenever framebuffer
height is reported as 0.
Agent Prompt
### Issue description
`Engine::Run()` divides by framebuffer height (`h`) to compute aspect ratio without checking for `h == 0`. If `h` is 0, the aspect becomes inf/NaN and produces an invalid projection matrix.

### Issue Context
`glfwGetFramebufferSize()` can return transient zero sizes (e.g., during resize/minimize). Passing an invalid projection matrix to the shader can corrupt rendering.

### Fix Focus Areas
- src/Core/Engine.cpp[117-123]

### Suggested change
Add a guard before computing aspect/projection, e.g.:
- If `w == 0 || h == 0`, skip rendering this frame (or clamp to a safe default) and continue the loop.
- Only call `GetProjectionMatrix(aspect)` when `h > 0`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@OwlWorksInnovations OwlWorksInnovations merged commit 07de8e8 into master Mar 18, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant