Skip to content

Conversation

@WSQS
Copy link
Owner

@WSQS WSQS commented Dec 2, 2025

Summary by CodeRabbit

  • New Features

    • Implemented lighting calculations in shaders with diffuse and specular components.
    • Enhanced vertex data to include normal information for improved lighting rendering.
    • Added support for rendering multiple objects with independent transformation matrices.
  • Documentation

    • Updated project status: Basic Lighting feature completed.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 2, 2025

Walkthrough

The changes extend shader lighting support by adding normal vector data to vertex layouts, updating shader interfaces with lighting calculations, and restructuring the rendering context to support multiple distinct camera and position matrices. Fragment reflection now tracks uniform counts, and rendering logic is updated to handle multiple renderables with separate transformation matrices.

Changes

Cohort / File(s) Summary
Documentation
README.md
Marked "Basic Lighting" task as completed in the Track section.
Data Structure Enhancements
data_type/modules/data_type.ixx
Added uniform_count field (std::uint32_t) to FragmentReflection to track shader uniform resource counts.
Shader Reflection Logic
glsl_reflector/modules/glsl_reflector.ixx
Modified reflect_vertex to collect vertex inputs into a local map keyed by layoutLocation before appending to results, changing insertion order from scan order to ascending layout location.
Rendering Context Structure
sdl_wrapper/modules/sdl_wrapper.renderable.ixx
Expanded RenderContext struct: camera_mat changed from single matrix to array of 3 matrices; added new pos field as array of 2 matrices. Includes std::array header.
Application Main Logic & Shaders
main.cpp
Expanded VertexType to include normal components (nx, ny, nz) alongside position and UVs; vertex data expanded from 8 to 36 entries with named-field initialization; vertex shaders now consume/output normals, UVs, and vertex positions; fragment shader adds lighting calculations (diffuse, specular) using lightPos and viewPos; rendering loop restructured to handle two distinct renderables with separate model/view/projection matrices; depth texture handling retained with dynamic resizing.
Shader Reflection & Uniform Pipeline
sdl_wrapper/src/sdl_wrapper.render_procedural.cpp
Fragment shader creation now passes dynamic reflect_result.uniform_count as third argument instead of hardcoded 0; sampler count remains unchanged.
Uniform Data Uploads
sdl_wrapper/src/sdl_wrapper.renderable.cpp
Updated vertex uniform buffer size to accommodate 3 camera matrices (sizeof(Mat<float, 4, 4>) * 3); added new SDL_PushGPUFragmentUniformData call for position matrices; includes std::array header.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • main.cpp requires careful attention: verify that the 36-vertex expansion is correct, shader attribute bindings match the new vertex layout, lighting calculations are mathematically sound, and the dual-renderable rendering loop properly distinguishes model/view/projection matrices for each object.
  • Ensure uniform_count propagation from reflection through render_procedural.cpp aligns with actual shader uniform declarations.
  • Verify array sizing and memory layout changes in RenderContext don't cause buffer alignment or indexing issues during uniform uploads in renderable.cpp.

Possibly related PRs

  • Gl reflection #36: Introduced shader reflection functionality (glsl_reflector); this PR extends it by reordering vertex input collection and propagating uniform counts.
  • Transformations #43: Modified camera and model matrix handling in rendering; this PR expands that concept with multiple matrices per renderable.
  • Coordinate systems #44: Altered sdl_wrapper rendering structures and pipeline; this PR continues that work with RenderContext expansion and uniform data reorganization.

Poem

🐰 A rabbit hops through shader code so bright,
With normals dancing, bringing models light!
From vertex normals to the frag's delight,
Two renderables blend in specular night—
The glow hops on! ✨🔆

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'Basic lighting' directly reflects the main objective of implementing basic lighting calculations in the fragment shader and supporting infrastructure changes.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch Basic-Lighting

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

This commit fixes the style issues introduced in 6d98e3b according to the output
from ClangFormat.

Details: #46
@deepsource-io
Copy link
Contributor

deepsource-io bot commented Dec 2, 2025

Here's the code health analysis summary for commits 859558d..946a0fc. View details on DeepSource ↗.

Analysis Summary

AnalyzerStatusSummaryLink
DeepSource C & C++ LogoC & C++❌ Failure
❗ 32 occurences introduced
🎯 94 occurences resolved
View Check ↗

💡 If you’re a repository administrator, you can configure the quality gates from the settings.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (5)
glsl_reflector/modules/glsl_reflector.ixx (1)

70-90: Good: Sorting vertex inputs by location ensures correct attribute binding order.

The use of std::map guarantees vertex inputs are ordered by layoutLocation, which is essential for proper shader attribute matching. However, <map> is not explicitly included in the module's global fragment.

Add the missing include:

 module;
 #include <iostream>
+#include <map>
 #include <string>
 #include <vector>
sdl_wrapper/modules/sdl_wrapper.renderable.ixx (1)

20-21: Consider more descriptive naming for pos.

The field name pos is ambiguous. Based on usage (lightPos and viewPos), consider renaming to something more descriptive like light_and_view_pos or splitting into separate fields:

std::array<Mat<float, 1, 4>, 2> pos{};  // Current
// Alternative:
Mat<float, 1, 4> light_pos{};
Mat<float, 1, 4> view_pos{};

This would improve code readability and make the intent clearer.

main.cpp (2)

122-124: Performance: Precompute normal matrix on CPU instead of per-vertex inverse().

Computing transpose(inverse(uModel)) per vertex is expensive. The inverse and transpose can be precomputed on the CPU and passed as a separate uniform:

 layout(std140, set = 1, binding = 0) uniform Camera
 {
     mat4 uModel;
     mat4 uView;
     mat4 uProjection;
+    mat3 uNormalMatrix;  // precomputed transpose(inverse(mat3(uModel)))
 };

 void main()
 {
     gl_Position = uProjection * uView * uModel * vec4(a_position, 1.0f);
-    v_normal = normalize(mat3(transpose(inverse(uModel))) * a_normal);
+    v_normal = normalize(uNormalMatrix * a_normal);
     v_pos = vec3(uModel * vec4(a_position, 1.0));
     v_uv = a_uv;
 }

For uniform scaling, mat3(uModel) alone suffices.


264-269: Consider using non-indexed draw since indices are trivial.

The index buffer contains sequential values 0..35 with no vertex sharing (due to per-face normals). This wastes GPU memory. Consider using SDL_DrawGPUPrimitives instead of SDL_DrawGPUIndexedPrimitives:

SDL_DrawGPUPrimitives(render_pass, vertex_count, 1, 0, 0);

Alternatively, if indexed drawing is needed for future flexibility, this is acceptable but unnecessary overhead for now.

README.md (1)

64-64: Mark "Basic Lighting" as completed—and standardize nested list indentation.

The change correctly marks "Basic Lighting" as complete, reflecting the PR's scope. However, there's a minor formatting inconsistency: the nested list items under "Lighting" (lines 63–68) use 4-space indentation, but the Markdown linting standard expects 2-space indentation for one level of nesting.

To align with Markdown best practices and resolve the linting issue, adjust the indentation of sub-items under "Lighting" from 4 spaces to 2:

 - [ ] Lighting
-    - [x] Colors
-    - [x] Basic Lighting
-    - [ ] Materials
-    - [ ] Lighting maps
-    - [ ] Light casters
-    - [ ] Multiple lights
+  - [x] Colors
+  - [x] Basic Lighting
+  - [ ] Materials
+  - [ ] Lighting maps
+  - [ ] Light casters
+  - [ ] Multiple lights
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 859558d and 946a0fc.

📒 Files selected for processing (7)
  • README.md (1 hunks)
  • data_type/modules/data_type.ixx (1 hunks)
  • glsl_reflector/modules/glsl_reflector.ixx (3 hunks)
  • main.cpp (4 hunks)
  • sdl_wrapper/modules/sdl_wrapper.renderable.ixx (2 hunks)
  • sdl_wrapper/src/sdl_wrapper.render_procedural.cpp (1 hunks)
  • sdl_wrapper/src/sdl_wrapper.renderable.cpp (2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-11-13T11:35:48.289Z
Learnt from: WSQS
Repo: WSQS/sdl_test PR: 7
File: sdl_wrapper/sdl_wrapper.gpu.ixx:37-43
Timestamp: 2025-11-13T11:35:48.289Z
Learning: In sdl_wrapper/sdl_wrapper.gpu.ixx, null pointer validation for the GpuWrapper::create_buffer method (checking m_device, SDL_CreateGPUBuffer return value, and shared_from_this() validity) is deferred to a future issue and should not be flagged in reviews.

Applied to files:

  • sdl_wrapper/modules/sdl_wrapper.renderable.ixx
  • sdl_wrapper/src/sdl_wrapper.renderable.cpp
  • sdl_wrapper/src/sdl_wrapper.render_procedural.cpp
📚 Learning: 2025-11-14T00:45:09.403Z
Learnt from: WSQS
Repo: WSQS/sdl_test PR: 7
File: sdl_wrapper/sdl_wrapper.gpu.ixx:73-76
Timestamp: 2025-11-14T00:45:09.403Z
Learning: In sdl_wrapper/sdl_wrapper.gpu.ixx, validation for m_window in the GpuWrapper::get_texture_formate() method (checking if m_window is nullptr before calling SDL_GetGPUSwapchainTextureFormat) is deferred to a future window wrapper implementation and should not be flagged in reviews.

Applied to files:

  • sdl_wrapper/modules/sdl_wrapper.renderable.ixx
  • sdl_wrapper/src/sdl_wrapper.renderable.cpp
📚 Learning: 2025-11-13T11:34:32.732Z
Learnt from: WSQS
Repo: WSQS/sdl_test PR: 7
File: sdl_wrapper/sdl_wrapper.buffer.ixx:0-0
Timestamp: 2025-11-13T11:34:32.732Z
Learning: In sdl_wrapper/sdl_wrapper.buffer.ixx, null pointer validation for the BufferWrapper constructor (checking p_gpu, p_gpu->data(), and SDL_CreateGPUBuffer return value) is deferred to a future issue and should not be flagged in reviews.

Applied to files:

  • sdl_wrapper/modules/sdl_wrapper.renderable.ixx
  • sdl_wrapper/src/sdl_wrapper.renderable.cpp
📚 Learning: 2025-11-13T02:58:26.892Z
Learnt from: WSQS
Repo: WSQS/sdl_test PR: 7
File: sdl_wrapper/sdl_wrapper.gpu.ixx:0-0
Timestamp: 2025-11-13T02:58:26.892Z
Learning: The sdl_wrapper library (in the repository WSQS/sdl_test) does not allow throwing exceptions. Error handling must use return codes, validity checks, or other non-exception mechanisms.

Applied to files:

  • sdl_wrapper/src/sdl_wrapper.renderable.cpp
🪛 markdownlint-cli2 (0.18.1)
README.md

64-64: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)

🔇 Additional comments (8)
data_type/modules/data_type.ixx (1)

66-70: LGTM!

The addition of uniform_count to FragmentReflection properly complements the existing sampler_count field and aligns with the reflection logic in glsl_reflector.ixx that now differentiates between samplers and other uniforms.

sdl_wrapper/src/sdl_wrapper.renderable.cpp (1)

24-27: Verify std140 layout alignment between C++ and GLSL.

The fragment shader Params uniform expects:

layout(std140) uniform Params {
    vec3 lightPos;  // 16 bytes (vec3 + 4 padding)
    vec3 viewPos;   // 16 bytes (vec3 + 4 padding)
};  // Total: 32 bytes

You're passing std::array<Mat<float, 1, 4>, 2> which is 32 bytes (2 × 4 floats × 4 bytes). This works due to std140's 16-byte alignment for vec3, but the semantic intent is unclear—consider using a dedicated struct with explicit padding or vec4 in the shader for clarity.

glsl_reflector/modules/glsl_reflector.ixx (1)

142-145: LGTM!

The fragment reflection now correctly counts non-sampler uniforms separately, enabling proper shader resource binding configuration.

main.cpp (4)

214-262: LGTM!

The cube vertex data is correctly defined with per-face normals, which is appropriate for flat-shaded lighting on cube faces. The UV coordinates are properly set for each face.


128-155: LGTM for basic lighting implementation.

The Phong lighting model is correctly implemented with ambient, diffuse, and specular components. For future flexibility, consider exposing hardcoded values (ambient strength 0.1, specular strength 0.5, shininess 32) as uniforms.


659-683: LGTM!

The rendering of two renderables with distinct shaders (textured+lit vs. plain white) is correctly set up. The second renderable appropriately omits texture and lighting uniforms since fragment_source2 doesn't use them.


157-168: Unused shader inputs are acceptable here.

The fragment shader declares v_normal, v_pos, and v_uv inputs but doesn't use them. This is necessary to match the vertex shader's output interface. No action needed.

sdl_wrapper/src/sdl_wrapper.render_procedural.cpp (1)

238-240: Dynamic reflection correctly applied for shader creation.

The shader creation now properly uses reflect_result.uniform_count and reflect_result.sampler_count instead of hardcoded values, and the parameters match the create_shader signature order: (code, stage, num_uniform_buffers, num_samplers).

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 25 out of 219. Check the log or trigger a new build to see more.

{0.5, 0.5, 0.5, 0., 0.}, {-0.5, 0.5, 0.5, 1., 0.}, {0.5, -0.5, 0.5, 0., 1.}, {-0.5, -0.5, 0.5, 1., 1.},
{0.5, 0.5, -0.5, 1., 1.}, {-0.5, 0.5, -0.5, 0., 1.}, {0.5, -0.5, -0.5, 1., 0.}, {-0.5, -0.5, -0.5, 0., 0.},
// +Z (front) 2 triangles
{.x = 0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 0, .v = 0},
Copy link

Choose a reason for hiding this comment

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

warning: 0.5f is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers]

            {.x = 0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 0, .v = 0},
                                        ^

{0.5, 0.5, 0.5, 0., 0.}, {-0.5, 0.5, 0.5, 1., 0.}, {0.5, -0.5, 0.5, 0., 1.}, {-0.5, -0.5, 0.5, 1., 1.},
{0.5, 0.5, -0.5, 1., 1.}, {-0.5, 0.5, -0.5, 0., 1.}, {0.5, -0.5, -0.5, 1., 0.}, {-0.5, -0.5, -0.5, 0., 0.},
// +Z (front) 2 triangles
{.x = 0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 0, .v = 0},
Copy link

Choose a reason for hiding this comment

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

warning: 0.5f is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers]

            {.x = 0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 0, .v = 0},
                             ^

{0.5, 0.5, 0.5, 0., 0.}, {-0.5, 0.5, 0.5, 1., 0.}, {0.5, -0.5, 0.5, 0., 1.}, {-0.5, -0.5, 0.5, 1., 1.},
{0.5, 0.5, -0.5, 1., 1.}, {-0.5, 0.5, -0.5, 0., 1.}, {0.5, -0.5, -0.5, 1., 0.}, {-0.5, -0.5, -0.5, 0., 0.},
// +Z (front) 2 triangles
{.x = 0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 0, .v = 0},
Copy link

Choose a reason for hiding this comment

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

warning: 0.5f is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers]

            {.x = 0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 0, .v = 0},
                  ^

{0.5, 0.5, 0.5, 0., 0.}, {-0.5, 0.5, 0.5, 1., 0.}, {0.5, -0.5, 0.5, 0., 1.}, {-0.5, -0.5, 0.5, 1., 1.},
{0.5, 0.5, -0.5, 1., 1.}, {-0.5, 0.5, -0.5, 0., 1.}, {0.5, -0.5, -0.5, 1., 0.}, {-0.5, -0.5, -0.5, 0., 0.},
// +Z (front) 2 triangles
{.x = 0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 0, .v = 0},
Copy link

Choose a reason for hiding this comment

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

warning: floating point literal has suffix 'f', which is not uppercase [readability-uppercase-literal-suffix]

Suggested change
{.x = 0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 0, .v = 0},
{.x = 0.5F, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 0, .v = 0},

{0.5, 0.5, 0.5, 0., 0.}, {-0.5, 0.5, 0.5, 1., 0.}, {0.5, -0.5, 0.5, 0., 1.}, {-0.5, -0.5, 0.5, 1., 1.},
{0.5, 0.5, -0.5, 1., 1.}, {-0.5, 0.5, -0.5, 0., 1.}, {0.5, -0.5, -0.5, 1., 0.}, {-0.5, -0.5, -0.5, 0., 0.},
// +Z (front) 2 triangles
{.x = 0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 0, .v = 0},
Copy link

Choose a reason for hiding this comment

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

warning: floating point literal has suffix 'f', which is not uppercase [readability-uppercase-literal-suffix]

Suggested change
{.x = 0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 0, .v = 0},
{.x = 0.5f, .y = 0.5F, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 0, .v = 0},

{.x = 0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 0, .v = 0},
{.x = -0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 0},
{.x = 0.5f, .y = -0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 0, .v = 1},
{.x = -0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 0},
Copy link

Choose a reason for hiding this comment

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

warning: 0.5f is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers]

            {.x = -0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 0},
                   ^

{.x = 0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 0, .v = 0},
{.x = -0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 0},
{.x = 0.5f, .y = -0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 0, .v = 1},
{.x = -0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 0},
Copy link

Choose a reason for hiding this comment

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

warning: floating point literal has suffix 'f', which is not uppercase [readability-uppercase-literal-suffix]

Suggested change
{.x = -0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 0},
{.x = -0.5F, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 0},

{.x = 0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 0, .v = 0},
{.x = -0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 0},
{.x = 0.5f, .y = -0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 0, .v = 1},
{.x = -0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 0},
Copy link

Choose a reason for hiding this comment

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

warning: floating point literal has suffix 'f', which is not uppercase [readability-uppercase-literal-suffix]

Suggested change
{.x = -0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 0},
{.x = -0.5f, .y = 0.5F, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 0},

{.x = 0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 0, .v = 0},
{.x = -0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 0},
{.x = 0.5f, .y = -0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 0, .v = 1},
{.x = -0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 0},
Copy link

Choose a reason for hiding this comment

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

warning: floating point literal has suffix 'f', which is not uppercase [readability-uppercase-literal-suffix]

Suggested change
{.x = -0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 0},
{.x = -0.5f, .y = 0.5f, .z = 0.5F, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 0},

{.x = -0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 0},
{.x = 0.5f, .y = -0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 0, .v = 1},
{.x = -0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 0},
{.x = -0.5f, .y = -0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 1},
Copy link

Choose a reason for hiding this comment

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

warning: 0.5f is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers]

            {.x = -0.5f, .y = -0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 1},
                                          ^

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 25 out of 194. Check the log or trigger a new build to see more.

{.x = -0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 0},
{.x = 0.5f, .y = -0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 0, .v = 1},
{.x = -0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 0},
{.x = -0.5f, .y = -0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 1},
Copy link

Choose a reason for hiding this comment

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

warning: 0.5f is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers]

            {.x = -0.5f, .y = -0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 1},
                               ^

{.x = -0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 0},
{.x = 0.5f, .y = -0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 0, .v = 1},
{.x = -0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 0},
{.x = -0.5f, .y = -0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 1},
Copy link

Choose a reason for hiding this comment

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

warning: 0.5f is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers]

            {.x = -0.5f, .y = -0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 1},
                   ^

{.x = -0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 0},
{.x = 0.5f, .y = -0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 0, .v = 1},
{.x = -0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 0},
{.x = -0.5f, .y = -0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 1},
Copy link

Choose a reason for hiding this comment

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

warning: floating point literal has suffix 'f', which is not uppercase [readability-uppercase-literal-suffix]

Suggested change
{.x = -0.5f, .y = -0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 1},
{.x = -0.5F, .y = -0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 1},

{.x = -0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 0},
{.x = 0.5f, .y = -0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 0, .v = 1},
{.x = -0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 0},
{.x = -0.5f, .y = -0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 1},
Copy link

Choose a reason for hiding this comment

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

warning: floating point literal has suffix 'f', which is not uppercase [readability-uppercase-literal-suffix]

Suggested change
{.x = -0.5f, .y = -0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 1},
{.x = -0.5f, .y = -0.5F, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 1},

{.x = -0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 0},
{.x = 0.5f, .y = -0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 0, .v = 1},
{.x = -0.5f, .y = 0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 0},
{.x = -0.5f, .y = -0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 1},
Copy link

Choose a reason for hiding this comment

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

warning: floating point literal has suffix 'f', which is not uppercase [readability-uppercase-literal-suffix]

Suggested change
{.x = -0.5f, .y = -0.5f, .z = 0.5f, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 1},
{.x = -0.5f, .y = -0.5f, .z = 0.5F, .nx = 0, .ny = 0, .nz = 1, .u = 1, .v = 1},


// -Z (back)
{.x = -0.5f, .y = 0.5f, .z = -0.5f, .nx = 0, .ny = 0, .nz = -1, .u = 0, .v = 0},
{.x = 0.5f, .y = 0.5f, .z = -0.5f, .nx = 0, .ny = 0, .nz = -1, .u = 1, .v = 0},
Copy link

Choose a reason for hiding this comment

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

warning: floating point literal has suffix 'f', which is not uppercase [readability-uppercase-literal-suffix]

Suggested change
{.x = 0.5f, .y = 0.5f, .z = -0.5f, .nx = 0, .ny = 0, .nz = -1, .u = 1, .v = 0},
{.x = 0.5F, .y = 0.5f, .z = -0.5f, .nx = 0, .ny = 0, .nz = -1, .u = 1, .v = 0},


// -Z (back)
{.x = -0.5f, .y = 0.5f, .z = -0.5f, .nx = 0, .ny = 0, .nz = -1, .u = 0, .v = 0},
{.x = 0.5f, .y = 0.5f, .z = -0.5f, .nx = 0, .ny = 0, .nz = -1, .u = 1, .v = 0},
Copy link

Choose a reason for hiding this comment

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

warning: floating point literal has suffix 'f', which is not uppercase [readability-uppercase-literal-suffix]

Suggested change
{.x = 0.5f, .y = 0.5f, .z = -0.5f, .nx = 0, .ny = 0, .nz = -1, .u = 1, .v = 0},
{.x = 0.5f, .y = 0.5F, .z = -0.5f, .nx = 0, .ny = 0, .nz = -1, .u = 1, .v = 0},


// -Z (back)
{.x = -0.5f, .y = 0.5f, .z = -0.5f, .nx = 0, .ny = 0, .nz = -1, .u = 0, .v = 0},
{.x = 0.5f, .y = 0.5f, .z = -0.5f, .nx = 0, .ny = 0, .nz = -1, .u = 1, .v = 0},
Copy link

Choose a reason for hiding this comment

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

warning: floating point literal has suffix 'f', which is not uppercase [readability-uppercase-literal-suffix]

Suggested change
{.x = 0.5f, .y = 0.5f, .z = -0.5f, .nx = 0, .ny = 0, .nz = -1, .u = 1, .v = 0},
{.x = 0.5f, .y = 0.5f, .z = -0.5F, .nx = 0, .ny = 0, .nz = -1, .u = 1, .v = 0},

// -Z (back)
{.x = -0.5f, .y = 0.5f, .z = -0.5f, .nx = 0, .ny = 0, .nz = -1, .u = 0, .v = 0},
{.x = 0.5f, .y = 0.5f, .z = -0.5f, .nx = 0, .ny = 0, .nz = -1, .u = 1, .v = 0},
{.x = -0.5f, .y = -0.5f, .z = -0.5f, .nx = 0, .ny = 0, .nz = -1, .u = 0, .v = 1},
Copy link

Choose a reason for hiding this comment

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

warning: 0.5f is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers]

            {.x = -0.5f, .y = -0.5f, .z = -0.5f, .nx = 0, .ny = 0, .nz = -1, .u = 0, .v = 1},
                                           ^

// -Z (back)
{.x = -0.5f, .y = 0.5f, .z = -0.5f, .nx = 0, .ny = 0, .nz = -1, .u = 0, .v = 0},
{.x = 0.5f, .y = 0.5f, .z = -0.5f, .nx = 0, .ny = 0, .nz = -1, .u = 1, .v = 0},
{.x = -0.5f, .y = -0.5f, .z = -0.5f, .nx = 0, .ny = 0, .nz = -1, .u = 0, .v = 1},
Copy link

Choose a reason for hiding this comment

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

warning: 0.5f is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers]

            {.x = -0.5f, .y = -0.5f, .z = -0.5f, .nx = 0, .ny = 0, .nz = -1, .u = 0, .v = 1},
                               ^

@WSQS WSQS merged commit 9cdcd33 into dev Dec 2, 2025
26 of 27 checks passed
@WSQS WSQS deleted the Basic-Lighting branch December 2, 2025 12:56
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.

3 participants