Skip to content

Remove bgfx/bx/bimg and the rendering/scene stack; add Matrix4 + Quaternion to Core#51

Open
jayrulez wants to merge 1 commit into
Redot-Engine:masterfrom
jayrulez:rhi-drop-bgfx
Open

Remove bgfx/bx/bimg and the rendering/scene stack; add Matrix4 + Quaternion to Core#51
jayrulez wants to merge 1 commit into
Redot-Engine:masterfrom
jayrulez:rhi-drop-bgfx

Conversation

@jayrulez

@jayrulez jayrulez commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Drop the bgfx rendering stack entirely: delete the Rendering and Scene modules, the bx/bimg/bgfx submodules and their ThirdParty build tooling, the shader compilation (cmake/Shaders.cmake), and rewire Runtime/Engine/Draconic so the engine builds on Core + Shell + SDL3 only.

RenderSample becomes a shell-only placeholder that opens a window and pumps events until closed.

Base Transform on our new Matrix type.

Summary by CodeRabbit

  • New Features

    • Added matrix4, quaternion, and value-type Transform with interpolation (lerp) and toMatrix() support.
    • Expanded math utilities with trigonometric/sqrt wrappers and epsilon-based nearlyEqual helpers (scalar + vector overloads).
    • Added identity/unit constants for common math types.
  • Changes

    • Simplified the sample to a window + event loop only (no rendering).
    • Removed rendering/scene interfaces and related modules from the public API surface.
  • Tests

    • Added/expanded unit tests for matrix, quaternion, and transform behavior (including inversion/invertibility and interpolation).

@coderabbitai

coderabbitai Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5003a22b-8bb3-4d22-9cf3-e77c55ce7b28

📥 Commits

Reviewing files that changed from the base of the PR and between 2df67e4 and 315133d.

⛔ Files ignored due to path filters (3)
  • Engine/cpp/ThirdParty/cmake/bgfx/generated/bounds.cpp.in is excluded by !**/generated/**
  • Engine/cpp/ThirdParty/cmake/bgfx/generated/shader.cpp.in is excluded by !**/generated/**
  • Engine/cpp/ThirdParty/cmake/bgfx/generated/vertexlayout.cpp.in is excluded by !**/generated/**
📒 Files selected for processing (98)
  • .gitmodules
  • CMakeLists.txt
  • Engine/cpp/CMakeLists.txt
  • Engine/cpp/Runtime/CMakeLists.txt
  • Engine/cpp/Runtime/Core/CMakeLists.txt
  • Engine/cpp/Runtime/Core/Math/Functions.cppm
  • Engine/cpp/Runtime/Core/Math/Math.cppm
  • Engine/cpp/Runtime/Core/Math/Math.test.cpp
  • Engine/cpp/Runtime/Core/Math/Matrix4.cpp
  • Engine/cpp/Runtime/Core/Math/Matrix4.cppm
  • Engine/cpp/Runtime/Core/Math/Quaternion.cppm
  • Engine/cpp/Runtime/Core/Math/Transform.cpp
  • Engine/cpp/Runtime/Core/Math/Transform.cppm
  • Engine/cpp/Runtime/Core/Math/TypesCommon.cppm
  • Engine/cpp/Runtime/Core/Math/Vector2.cppm
  • Engine/cpp/Runtime/Core/Math/Vector3.cppm
  • Engine/cpp/Runtime/Core/Math/Vector4.cppm
  • Engine/cpp/Runtime/Draconic.cppm
  • Engine/cpp/Runtime/Rendering/CMakeLists.txt
  • Engine/cpp/Runtime/Rendering/Material/Material.cppm
  • Engine/cpp/Runtime/Rendering/Mesh/Mesh.cpp
  • Engine/cpp/Runtime/Rendering/Mesh/Mesh.cppm
  • Engine/cpp/Runtime/Rendering/QuadRenderer/QuadRenderer.cpp
  • Engine/cpp/Runtime/Rendering/QuadRenderer/QuadRenderer.cppm
  • Engine/cpp/Runtime/Rendering/RHI/Buffers.cpp
  • Engine/cpp/Runtime/Rendering/RHI/Commands.cpp
  • Engine/cpp/Runtime/Rendering/RHI/Core.cpp
  • Engine/cpp/Runtime/Rendering/RHI/Pipelines.cpp
  • Engine/cpp/Runtime/Rendering/RHI/RHI.cppm
  • Engine/cpp/Runtime/Rendering/RHI/Texture.cpp
  • Engine/cpp/Runtime/Rendering/RHI/UniformRegistry.cppm
  • Engine/cpp/Runtime/Rendering/RHI/Vertex.cppm
  • Engine/cpp/Runtime/Rendering/RHI/macros.h
  • Engine/cpp/Runtime/Rendering/RenderGraph/RenderGraph.cpp
  • Engine/cpp/Runtime/Rendering/RenderGraph/RenderGraph.cppm
  • Engine/cpp/Runtime/Rendering/Renderer/Renderer.cpp
  • Engine/cpp/Runtime/Rendering/Renderer/Renderer.cppm
  • Engine/cpp/Runtime/Rendering/Rendering.cppm
  • Engine/cpp/Runtime/Rendering/Shaders/fs.sc
  • Engine/cpp/Runtime/Rendering/Shaders/fs_quad.sc
  • Engine/cpp/Runtime/Rendering/Shaders/varying.def.sc
  • Engine/cpp/Runtime/Rendering/Shaders/varying_quad.def.sc
  • Engine/cpp/Runtime/Rendering/Shaders/vs.sc
  • Engine/cpp/Runtime/Rendering/Shaders/vs_quad.sc
  • Engine/cpp/Runtime/Scene/CMakeLists.txt
  • Engine/cpp/Runtime/Scene/Camera/CameraController.cpp
  • Engine/cpp/Runtime/Scene/Camera/CameraController.cppm
  • Engine/cpp/Runtime/Scene/Renderable/Renderable.cppm
  • Engine/cpp/Runtime/Scene/Scene.cppm
  • Engine/cpp/Runtime/Scene/TransformComponent/TransformComponent.cpp
  • Engine/cpp/Runtime/Scene/TransformComponent/TransformComponent.cppm
  • Engine/cpp/ThirdParty/CMakeLists.txt
  • Engine/cpp/ThirdParty/bgfx
  • Engine/cpp/ThirdParty/bimg
  • Engine/cpp/ThirdParty/bx
  • Engine/cpp/ThirdParty/cmake/Config.cmake.in
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/dear-imgui.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/fcpp.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/glsl-optimizer.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/glslang.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/meshoptimizer.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/spirv-cross.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/spirv-opt.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/tint.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/webgpu.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/CMakeLists.txt
  • Engine/cpp/ThirdParty/cmake/bgfx/bgfx.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/examples.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/geometryc.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/geometryv.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/shaderc.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/shared.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/texturev.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/util/ConfigureDebugging.cmake
  • Engine/cpp/ThirdParty/cmake/bgfxToolUtils.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/astc_encoder.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/edtaa3.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/etc1.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/etc2.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/iqa.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/libsquish.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/loadpng.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/miniz.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/nvtt.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/pvrtc.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/tinyexr.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/CMakeLists.txt
  • Engine/cpp/ThirdParty/cmake/bimg/bimg.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/bimg_decode.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/bimg_encode.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/texturec.cmake
  • Engine/cpp/ThirdParty/cmake/bx/CMakeLists.txt
  • Engine/cpp/ThirdParty/cmake/bx/bin2c.cmake
  • Engine/cpp/ThirdParty/cmake/bx/bx.cmake
  • Engine/cpp/ThirdParty/cmake/version.cmake
  • Samples/cpp/Render/CMakeLists.txt
  • Samples/cpp/Render/RenderSample.cpp
  • cmake/Shaders.cmake
💤 Files with no reviewable changes (81)
  • Engine/cpp/Runtime/Scene/Renderable/Renderable.cppm
  • Engine/cpp/Runtime/Scene/Scene.cppm
  • Engine/cpp/Runtime/Scene/TransformComponent/TransformComponent.cpp
  • Engine/cpp/Runtime/Rendering/RHI/RHI.cppm
  • Engine/cpp/ThirdParty/bgfx
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/fcpp.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/spirv-cross.cmake
  • Engine/cpp/Runtime/Draconic.cppm
  • Engine/cpp/Runtime/CMakeLists.txt
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/webgpu.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/glslang.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/texturec.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/miniz.cmake
  • Engine/cpp/ThirdParty/cmake/version.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/CMakeLists.txt
  • Engine/cpp/Runtime/Rendering/Rendering.cppm
  • Engine/cpp/Runtime/Scene/TransformComponent/TransformComponent.cppm
  • Engine/cpp/Runtime/Scene/CMakeLists.txt
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/tint.cmake
  • Engine/cpp/Runtime/Rendering/RenderGraph/RenderGraph.cppm
  • Engine/cpp/Runtime/Rendering/Shaders/vs.sc
  • Engine/cpp/Runtime/Rendering/RHI/Vertex.cppm
  • Engine/cpp/Runtime/Scene/Camera/CameraController.cppm
  • Engine/cpp/ThirdParty/cmake/bgfx/geometryv.cmake
  • Engine/cpp/Runtime/Core/Math/Transform.cpp
  • Engine/cpp/Runtime/Rendering/RHI/macros.h
  • Engine/cpp/Runtime/Rendering/Shaders/fs.sc
  • Engine/cpp/ThirdParty/cmake/bgfx/texturev.cmake
  • Engine/cpp/ThirdParty/bimg
  • Engine/cpp/ThirdParty/cmake/bimg/bimg_encode.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/util/ConfigureDebugging.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/edtaa3.cmake
  • Engine/cpp/Runtime/Rendering/QuadRenderer/QuadRenderer.cppm
  • Engine/cpp/Runtime/Rendering/Shaders/varying.def.sc
  • Engine/cpp/Runtime/Rendering/Shaders/varying_quad.def.sc
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/iqa.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/libsquish.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/loadpng.cmake
  • Engine/cpp/Runtime/Rendering/RHI/Buffers.cpp
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/astc_encoder.cmake
  • Engine/cpp/Runtime/Scene/Camera/CameraController.cpp
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/etc1.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/geometryc.cmake
  • cmake/Shaders.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/tinyexr.cmake
  • Engine/cpp/Runtime/Rendering/Shaders/vs_quad.sc
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/glsl-optimizer.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/dear-imgui.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/nvtt.cmake
  • Engine/cpp/Runtime/Rendering/Shaders/fs_quad.sc
  • Engine/cpp/Runtime/Rendering/RHI/Commands.cpp
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/meshoptimizer.cmake
  • Engine/cpp/Runtime/Rendering/QuadRenderer/QuadRenderer.cpp
  • Engine/cpp/Runtime/Rendering/CMakeLists.txt
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/spirv-opt.cmake
  • Engine/cpp/ThirdParty/cmake/bx/CMakeLists.txt
  • Engine/cpp/ThirdParty/cmake/bimg/CMakeLists.txt
  • Engine/cpp/Runtime/Rendering/Renderer/Renderer.cpp
  • Engine/cpp/Runtime/Rendering/Mesh/Mesh.cppm
  • Engine/cpp/Runtime/Rendering/Renderer/Renderer.cppm
  • Engine/cpp/ThirdParty/bx
  • CMakeLists.txt
  • Engine/cpp/ThirdParty/cmake/bgfxToolUtils.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/etc2.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/shared.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/bimg_decode.cmake
  • Engine/cpp/Runtime/Rendering/RHI/UniformRegistry.cppm
  • Engine/cpp/Runtime/Rendering/RHI/Core.cpp
  • Engine/cpp/ThirdParty/cmake/bgfx/examples.cmake
  • Engine/cpp/ThirdParty/cmake/bx/bin2c.cmake
  • Engine/cpp/ThirdParty/cmake/bx/bx.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/shaderc.cmake
  • Engine/cpp/Runtime/Rendering/RHI/Texture.cpp
  • Engine/cpp/ThirdParty/cmake/bimg/bimg.cmake
  • Engine/cpp/Runtime/Rendering/Mesh/Mesh.cpp
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/pvrtc.cmake
  • Engine/cpp/Runtime/Rendering/Material/Material.cppm
  • Engine/cpp/ThirdParty/cmake/Config.cmake.in
  • Engine/cpp/Runtime/Rendering/RenderGraph/RenderGraph.cpp
  • Engine/cpp/Runtime/Rendering/RHI/Pipelines.cpp
  • Engine/cpp/ThirdParty/cmake/bgfx/bgfx.cmake
✅ Files skipped from review due to trivial changes (1)
  • Engine/cpp/Runtime/Core/Math/Math.cppm
🚧 Files skipped from review as they are similar to previous changes (13)
  • Engine/cpp/Runtime/Core/Math/Functions.cppm
  • Engine/cpp/Runtime/Core/Math/TypesCommon.cppm
  • Engine/cpp/Runtime/Core/Math/Vector4.cppm
  • Engine/cpp/Runtime/Core/Math/Vector3.cppm
  • Samples/cpp/Render/RenderSample.cpp
  • Engine/cpp/Runtime/Core/Math/Vector2.cppm
  • Engine/cpp/ThirdParty/CMakeLists.txt
  • Engine/cpp/CMakeLists.txt
  • Engine/cpp/Runtime/Core/Math/Math.test.cpp
  • Engine/cpp/Runtime/Core/Math/Quaternion.cppm
  • Samples/cpp/Render/CMakeLists.txt
  • .gitmodules
  • Engine/cpp/Runtime/Core/Math/Transform.cppm

📝 Walkthrough

Walkthrough

The change adds Matrix4, Quaternion, transform interpolation, and approximate-comparison math APIs. It removes rendering, scene, shader, and bgfx-related build surfaces, narrows runtime dependencies, and rewrites the render sample as a shell event-loop application.

Changes

Engine math foundation

Layer / File(s) Summary
Matrix, quaternion, and comparison utilities
Engine/cpp/Runtime/Core/Math/*
Adds Matrix4 and Quaternion modules, transform helpers, interpolation, vector/scalar approximate comparisons, vector constants, and public math re-exports.
Transform integration and validation
Engine/cpp/Runtime/Core/Math/Transform.cppm, Engine/cpp/Runtime/Core/Math/Matrix4.cpp, Engine/cpp/Runtime/Core/Math/Math.test.cpp
Refactors Transform to use Vector3 and Quaternion values, returns composed matrices, adds interpolation, implements matrix inversion, and expands math tests.

Runtime and sample restructuring

Layer / File(s) Summary
Runtime and third-party build surface
CMakeLists.txt, Engine/cpp/CMakeLists.txt, Engine/cpp/Runtime/..., Engine/cpp/ThirdParty/...
Removes rendering and scene target wiring, narrows Runtime linkage, removes shader inclusion and bgfx/bimg/bx build configuration, and leaves SDL/stb setup.
Shell-only render sample
Samples/cpp/Render/*
Changes the sample to link against Shell_Desktop and process shell events in a timed loop without rendering initialization or shader build steps.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related PRs

Suggested reviewers: mcdubhghlas, Arctis-Fireblight, OldDev78, JoltedJon

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures the main scope: removing the rendering stack and third-party deps, while adding Matrix4 and Quaternion to Core.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (2)
Samples/cpp/Render/RenderSample.cpp (1)

12-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Wrap the createShell result in std::unique_ptr to avoid a leak.

createShell returns a raw owning IShell* (new SDL3Shell(settings)) that is never freed. IShell has a virtual destructor, so unique_ptr will correctly clean up. Ensure #include <memory> is present.

♻️ Proposed fix
-    auto shell = createShell(WindowSettings{
+    auto shell = std::unique_ptr<IShell>(createShell(WindowSettings{
         .title = u8"Draconic Engine Sample",
         .width = 1280,
         .height = 720,
-    });
+    }));
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Samples/cpp/Render/RenderSample.cpp` around lines 12 - 16, Update the shell
initialization around createShell to store its owning raw IShell* result in a
std::unique_ptr, ensuring the virtual destructor performs cleanup at scope exit.
Add the <memory> include if it is not already present, and preserve the existing
WindowSettings arguments.
Engine/cpp/Runtime/Core/Math/Math.test.cpp (1)

1500-1505: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Exercise projection and view behavior, not only coefficients.

Add near/far homogeneous-depth checks and coverage for orthographicRH() and lookAtRH(). The current assertions would miss common sign or transposition regressions.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Engine/cpp/Runtime/Core/Math/Math.test.cpp` around lines 1500 - 1505, Add
behavioral checks to the “perspective has the expected projective structure”
test by projecting representative near- and far-plane points and validating
their homogeneous depth results. Add coverage for orthographicRH() and
lookAtRH(), including transformed points and expected view/depth behavior, so
sign and matrix-transposition regressions are detected rather than only checking
raw coefficients.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Engine/cpp/Runtime/Core/Math/Matrix4.cppm`:
- Around line 251-255: Update Matrix4 inversion to expose singular failures
through tryInverse or an optional result instead of returning
Matrix4::identity(), and replace the absolute determinant check with a
scale-aware singularity criterion. In
Engine/cpp/Runtime/Core/Math/Matrix4.cppm:251-255, modify the inverse
implementation; in Engine/cpp/Runtime/Core/Math/Math.test.cpp:1531-1532, assert
explicit failure for singular matrices and add regression coverage proving
small-but-invertible matrices still invert successfully.

In `@Engine/cpp/Runtime/Core/Math/Quaternion.cppm`:
- Around line 77-80: Update Quaternion::nearlyEqual to consider both the
component-wise comparison of a and b and the comparison of a against the negated
components of b, so antipodal quaternions are treated as equal rotations.
Preserve the epsilon parameter and noexcept contract, and ensure the result
remains true for both identical and sign-inverted quaternion pairs.
- Around line 28-33: Update Quaternion::fromAxisAngle to detect a zero-length
axis before normalization and return the identity quaternion for that input,
preventing rotateVector() from scaling vectors. Preserve the existing
normalized-axis conversion for nonzero axes.

---

Nitpick comments:
In `@Engine/cpp/Runtime/Core/Math/Math.test.cpp`:
- Around line 1500-1505: Add behavioral checks to the “perspective has the
expected projective structure” test by projecting representative near- and
far-plane points and validating their homogeneous depth results. Add coverage
for orthographicRH() and lookAtRH(), including transformed points and expected
view/depth behavior, so sign and matrix-transposition regressions are detected
rather than only checking raw coefficients.

In `@Samples/cpp/Render/RenderSample.cpp`:
- Around line 12-16: Update the shell initialization around createShell to store
its owning raw IShell* result in a std::unique_ptr, ensuring the virtual
destructor performs cleanup at scope exit. Add the <memory> include if it is not
already present, and preserve the existing WindowSettings arguments.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 45bd4269-17eb-4bbc-a280-89ebbcde98e9

📥 Commits

Reviewing files that changed from the base of the PR and between 2f1ec41 and 2df67e4.

⛔ Files ignored due to path filters (3)
  • Engine/cpp/ThirdParty/cmake/bgfx/generated/bounds.cpp.in is excluded by !**/generated/**
  • Engine/cpp/ThirdParty/cmake/bgfx/generated/shader.cpp.in is excluded by !**/generated/**
  • Engine/cpp/ThirdParty/cmake/bgfx/generated/vertexlayout.cpp.in is excluded by !**/generated/**
📒 Files selected for processing (97)
  • .gitmodules
  • CMakeLists.txt
  • Engine/cpp/CMakeLists.txt
  • Engine/cpp/Runtime/CMakeLists.txt
  • Engine/cpp/Runtime/Core/CMakeLists.txt
  • Engine/cpp/Runtime/Core/Math/Functions.cppm
  • Engine/cpp/Runtime/Core/Math/Math.cppm
  • Engine/cpp/Runtime/Core/Math/Math.test.cpp
  • Engine/cpp/Runtime/Core/Math/Matrix4.cppm
  • Engine/cpp/Runtime/Core/Math/Quaternion.cppm
  • Engine/cpp/Runtime/Core/Math/Transform.cpp
  • Engine/cpp/Runtime/Core/Math/Transform.cppm
  • Engine/cpp/Runtime/Core/Math/TypesCommon.cppm
  • Engine/cpp/Runtime/Core/Math/Vector2.cppm
  • Engine/cpp/Runtime/Core/Math/Vector3.cppm
  • Engine/cpp/Runtime/Core/Math/Vector4.cppm
  • Engine/cpp/Runtime/Draconic.cppm
  • Engine/cpp/Runtime/Rendering/CMakeLists.txt
  • Engine/cpp/Runtime/Rendering/Material/Material.cppm
  • Engine/cpp/Runtime/Rendering/Mesh/Mesh.cpp
  • Engine/cpp/Runtime/Rendering/Mesh/Mesh.cppm
  • Engine/cpp/Runtime/Rendering/QuadRenderer/QuadRenderer.cpp
  • Engine/cpp/Runtime/Rendering/QuadRenderer/QuadRenderer.cppm
  • Engine/cpp/Runtime/Rendering/RHI/Buffers.cpp
  • Engine/cpp/Runtime/Rendering/RHI/Commands.cpp
  • Engine/cpp/Runtime/Rendering/RHI/Core.cpp
  • Engine/cpp/Runtime/Rendering/RHI/Pipelines.cpp
  • Engine/cpp/Runtime/Rendering/RHI/RHI.cppm
  • Engine/cpp/Runtime/Rendering/RHI/Texture.cpp
  • Engine/cpp/Runtime/Rendering/RHI/UniformRegistry.cppm
  • Engine/cpp/Runtime/Rendering/RHI/Vertex.cppm
  • Engine/cpp/Runtime/Rendering/RHI/macros.h
  • Engine/cpp/Runtime/Rendering/RenderGraph/RenderGraph.cpp
  • Engine/cpp/Runtime/Rendering/RenderGraph/RenderGraph.cppm
  • Engine/cpp/Runtime/Rendering/Renderer/Renderer.cpp
  • Engine/cpp/Runtime/Rendering/Renderer/Renderer.cppm
  • Engine/cpp/Runtime/Rendering/Rendering.cppm
  • Engine/cpp/Runtime/Rendering/Shaders/fs.sc
  • Engine/cpp/Runtime/Rendering/Shaders/fs_quad.sc
  • Engine/cpp/Runtime/Rendering/Shaders/varying.def.sc
  • Engine/cpp/Runtime/Rendering/Shaders/varying_quad.def.sc
  • Engine/cpp/Runtime/Rendering/Shaders/vs.sc
  • Engine/cpp/Runtime/Rendering/Shaders/vs_quad.sc
  • Engine/cpp/Runtime/Scene/CMakeLists.txt
  • Engine/cpp/Runtime/Scene/Camera/CameraController.cpp
  • Engine/cpp/Runtime/Scene/Camera/CameraController.cppm
  • Engine/cpp/Runtime/Scene/Renderable/Renderable.cppm
  • Engine/cpp/Runtime/Scene/Scene.cppm
  • Engine/cpp/Runtime/Scene/TransformComponent/TransformComponent.cpp
  • Engine/cpp/Runtime/Scene/TransformComponent/TransformComponent.cppm
  • Engine/cpp/ThirdParty/CMakeLists.txt
  • Engine/cpp/ThirdParty/bgfx
  • Engine/cpp/ThirdParty/bimg
  • Engine/cpp/ThirdParty/bx
  • Engine/cpp/ThirdParty/cmake/Config.cmake.in
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/dear-imgui.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/fcpp.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/glsl-optimizer.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/glslang.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/meshoptimizer.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/spirv-cross.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/spirv-opt.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/tint.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/webgpu.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/CMakeLists.txt
  • Engine/cpp/ThirdParty/cmake/bgfx/bgfx.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/examples.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/geometryc.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/geometryv.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/shaderc.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/shared.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/texturev.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/util/ConfigureDebugging.cmake
  • Engine/cpp/ThirdParty/cmake/bgfxToolUtils.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/astc_encoder.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/edtaa3.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/etc1.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/etc2.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/iqa.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/libsquish.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/loadpng.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/miniz.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/nvtt.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/pvrtc.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/tinyexr.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/CMakeLists.txt
  • Engine/cpp/ThirdParty/cmake/bimg/bimg.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/bimg_decode.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/bimg_encode.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/texturec.cmake
  • Engine/cpp/ThirdParty/cmake/bx/CMakeLists.txt
  • Engine/cpp/ThirdParty/cmake/bx/bin2c.cmake
  • Engine/cpp/ThirdParty/cmake/bx/bx.cmake
  • Engine/cpp/ThirdParty/cmake/version.cmake
  • Samples/cpp/Render/CMakeLists.txt
  • Samples/cpp/Render/RenderSample.cpp
  • cmake/Shaders.cmake
💤 Files with no reviewable changes (81)
  • Engine/cpp/Runtime/Rendering/RHI/RHI.cppm
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/tinyexr.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/glslang.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/util/ConfigureDebugging.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/spirv-cross.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/pvrtc.cmake
  • Engine/cpp/Runtime/Rendering/Material/Material.cppm
  • CMakeLists.txt
  • Engine/cpp/Runtime/Scene/TransformComponent/TransformComponent.cpp
  • Engine/cpp/Runtime/Rendering/Shaders/fs_quad.sc
  • Engine/cpp/ThirdParty/bx
  • Engine/cpp/Runtime/Core/Math/Transform.cpp
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/iqa.cmake
  • Engine/cpp/ThirdParty/cmake/Config.cmake.in
  • Engine/cpp/Runtime/Rendering/QuadRenderer/QuadRenderer.cpp
  • Engine/cpp/Runtime/CMakeLists.txt
  • Engine/cpp/ThirdParty/cmake/bimg/texturec.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/shared.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/dear-imgui.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/etc1.cmake
  • Engine/cpp/Runtime/Rendering/Shaders/varying_quad.def.sc
  • Engine/cpp/Runtime/Scene/Scene.cppm
  • Engine/cpp/ThirdParty/cmake/bimg/bimg.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/CMakeLists.txt
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/meshoptimizer.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/etc2.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/texturev.cmake
  • Engine/cpp/Runtime/Rendering/RHI/macros.h
  • Engine/cpp/Runtime/Scene/TransformComponent/TransformComponent.cppm
  • Engine/cpp/Runtime/Rendering/Shaders/vs_quad.sc
  • Engine/cpp/Runtime/Rendering/RHI/Pipelines.cpp
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/spirv-opt.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/bimg_decode.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/miniz.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/nvtt.cmake
  • Engine/cpp/ThirdParty/cmake/bimg/CMakeLists.txt
  • Engine/cpp/ThirdParty/cmake/bimg/bimg_encode.cmake
  • Engine/cpp/Runtime/Rendering/Mesh/Mesh.cppm
  • Engine/cpp/Runtime/Rendering/Shaders/fs.sc
  • Engine/cpp/Runtime/Rendering/RHI/Texture.cpp
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/glsl-optimizer.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/geometryv.cmake
  • Engine/cpp/ThirdParty/cmake/bgfxToolUtils.cmake
  • Engine/cpp/ThirdParty/bgfx
  • Engine/cpp/Runtime/Scene/Renderable/Renderable.cppm
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/edtaa3.cmake
  • Engine/cpp/ThirdParty/cmake/bx/CMakeLists.txt
  • Engine/cpp/Runtime/Rendering/RHI/Buffers.cpp
  • Engine/cpp/ThirdParty/cmake/bgfx/shaderc.cmake
  • Engine/cpp/Runtime/Rendering/RenderGraph/RenderGraph.cppm
  • Engine/cpp/Runtime/Scene/Camera/CameraController.cppm
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/webgpu.cmake
  • Engine/cpp/Runtime/Rendering/Shaders/vs.sc
  • Engine/cpp/ThirdParty/cmake/bgfx/geometryc.cmake
  • Engine/cpp/Runtime/Rendering/Rendering.cppm
  • Engine/cpp/ThirdParty/cmake/bgfx/bgfx.cmake
  • Engine/cpp/Runtime/Rendering/Renderer/Renderer.cpp
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/libsquish.cmake
  • Engine/cpp/Runtime/Rendering/Shaders/varying.def.sc
  • Engine/cpp/ThirdParty/cmake/bx/bin2c.cmake
  • Engine/cpp/Runtime/Rendering/Mesh/Mesh.cpp
  • Engine/cpp/ThirdParty/cmake/bx/bx.cmake
  • Engine/cpp/Runtime/Draconic.cppm
  • Engine/cpp/Runtime/Rendering/RHI/UniformRegistry.cppm
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/astc_encoder.cmake
  • Engine/cpp/Runtime/Rendering/QuadRenderer/QuadRenderer.cppm
  • Engine/cpp/ThirdParty/cmake/bgfx/examples.cmake
  • Engine/cpp/Runtime/Rendering/RHI/Vertex.cppm
  • Engine/cpp/Runtime/Rendering/Renderer/Renderer.cppm
  • Engine/cpp/ThirdParty/cmake/bimg/3rdparty/loadpng.cmake
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/fcpp.cmake
  • Engine/cpp/Runtime/Scene/Camera/CameraController.cpp
  • Engine/cpp/ThirdParty/cmake/version.cmake
  • Engine/cpp/Runtime/Rendering/RenderGraph/RenderGraph.cpp
  • Engine/cpp/ThirdParty/cmake/bgfx/3rdparty/tint.cmake
  • Engine/cpp/Runtime/Scene/CMakeLists.txt
  • Engine/cpp/Runtime/Rendering/RHI/Commands.cpp
  • Engine/cpp/Runtime/Rendering/CMakeLists.txt
  • cmake/Shaders.cmake
  • Engine/cpp/ThirdParty/bimg
  • Engine/cpp/Runtime/Rendering/RHI/Core.cpp

Comment thread Engine/cpp/Runtime/Core/Math/Matrix4.cppm Outdated
Comment thread Engine/cpp/Runtime/Core/Math/Quaternion.cppm
Comment thread Engine/cpp/Runtime/Core/Math/Quaternion.cppm
…ernion to Core

Drop the bgfx rendering stack entirely: delete the Rendering and Scene modules, the bx/bimg/bgfx submodules and their ThirdParty build tooling, the shader compilation (cmake/Shaders.cmake), and rewire Runtime/Engine/Draconic so the engine builds on Core + Shell + SDL3 only. RenderSample becomes a shell-only placeholder that opens a window and pumps events until closed.

Core/Math kept its one bx dependency (Transform's TRS matrix build), so replace it with native math: add core.math.matrix4 (4x4 row-major, row-vector convention: identity/translation/scale/rotation/perspective/ortho/lookAt, multiply, transpose, transformPoint/Direction/Point2D, determinant, inverse) and core.math.quaternion (fromAxisAngle, Hamilton product, conjugate/dot/inverse/normalize/rotateVector/slerp, rotationMatrix). Rewrite Transform to position/rotation(Quaternion)/scale with an inline toMatrix() and a lerp helper, removing bx from Core entirely.

Added missing math helpers used by these types (sin/cos/tan/acos/sqrt, scalar and Vector2/3/4 nearlyEqual, Vector3::zero/one) and added Matrix4/Quaternion/Transform unit tests.

@Shakai-Dev Shakai-Dev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@OldDev78 OldDev78 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A few small things I'd rather see changed.

Comment on lines +1 to +3
# PIC: these are linked into the shared Runtime library (via Core), so their objects
# must be position-independent - a non-PIC static (e.g. Quaternion::identity) otherwise
# fails to relocate when making the shared object.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If this is needed for a specific call in Math, then flag only Math as PIC, and propagate it through Core.
The shared library is Runtime.
Do not flag everything as PIC, it costs optimizations at a level where most code is linked statically.

}

// Component-wise approximate equality against an epsilon.
[[nodiscard]] inline bool nearlyEqual(const Vector2& a, const Vector2& b, f32 epsilon = CMP_EPSILON) noexcept {

@OldDev78 OldDev78 Jul 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don´t think we need two variations of the same check, component-wise near equal check should suffice, so it could effectively replace approx_eq - and make this constexpr. I would shorten the name though, near or nearEq.

Comment on lines +67 to +68
static const Vector3 zero;
static const Vector3 one;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is wrong, because they are declared inline constexpr in Vector3.cppm. Since it's exported as a module fragment, don't bother forward declaring them here.

Comment thread .gitmodules

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

Comment on lines +5 to +6
import shell;
import shell.desktop;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don´t repeat yourself, just import shell. shell should also be exporting shell.desktop.

I assume this is only being done temporarily? In fact, just importing draconic should still be fine.

Keep in mind, modules are not headers, "importing everything" does not mean the same as including everything.

Comment on lines -32 to +13
.title = u8"Draconic Engine Rendering Sample",
.title = u8"Draconic Engine Sample",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is going to be restored for rendering, right?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, most likely

Comment on lines +116 to +121
[[nodiscard]] inline bool nearlyEqual(f32 a, f32 b, f32 epsilon = CMP_EPSILON) noexcept {
return abs(a - b) <= epsilon;
}
[[nodiscard]] inline bool nearlyZero(f32 v, f32 epsilon = CMP_EPSILON) noexcept {
return abs(v) <= epsilon;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should be constexpr (along with the Vector* ones)

Comment on lines +32 to +39
[[nodiscard]] constexpr f32 operator()(usize row, usize col) const noexcept
{
return m[row][col];
}
[[nodiscard]] constexpr f32& operator()(usize row, usize col) noexcept
{
return m[row][col];
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Replace by multidimensional [] operator.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Might turn this into a template, but for now it will do.

Comment on lines +39 to +41
// Identity transform (position 0, rotation identity, scale 1) - the
// default-constructed value.
inline constexpr Transform identityTransform{};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We don't need a special constant which is the same as the default constructed value.

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