Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/clang-tidy-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ jobs:
-DCMAKE_EXPORT_COMPILE_COMMANDS=ON

- name: Run clang-tidy-review
uses: ZedThree/clang-tidy-review@v0.21.0
uses: ZedThree/clang-tidy-review@v0.22.1
id: review
with:
token: ${{ secrets.GITHUB_TOKEN }}
Expand Down
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ FetchContent_MakeAvailable(googletest)
include(CTest)
include(GoogleTest)

add_subdirectory(data_type)
add_subdirectory(sdl_wrapper)
add_subdirectory(glsl_reflector)

Expand Down
11 changes: 11 additions & 0 deletions data_type/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
cmake_minimum_required(VERSION 3.30)
project(data_type LANGUAGES CXX)

set(CMAKE_CXX_STANDARD 23)
add_library(data_type STATIC)

target_sources(data_type
PUBLIC
FILE_SET cxx_modules TYPE CXX_MODULES FILES
modules/data_type.ixx
)
31 changes: 31 additions & 0 deletions data_type/modules/data_type.ixx
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// data_type.ixx
// Created by sophomore on 11/23/25.
//
module;
#include<expected>
export module data_type;

export namespace sopho {
enum class GpuError {
UNINITIALIZED,
CREATE_DEVICE_FAILED,
CREATE_WINDOW_FAILED,
CLAIM_WINDOW_FAILED,
CREATE_GPU_BUFFER_FAILED,
CREATE_TRANSFER_BUFFER_FAILED,
CREATE_SHADER_FAILED,
CREATE_PIPELINE_FAILED,
GET_TEXTUREFORMAT_FAILED,
BUFFER_OVERFLOW,
MAP_TRANSFER_BUFFER_FAILED,
ACQUIRE_COMMAND_BUFFER_FAILED,
SUBMIT_COMMAND_FAILED,
BEGIN_COPY_PASS_FAILED,
COMPILE_VERTEX_SHADER_FAILED,
COMPILE_FRAGMENT_SHADER_FAILED,
};

using TError = GpuError;
template<typename T>
using checkable = std::expected<T, TError>;
}
34 changes: 22 additions & 12 deletions main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
#include "SDL3/SDL.h"
#include "SDL3/SDL_gpu.h"
#include "SDL3/SDL_keycode.h"

import data_type;
import glsl_reflector;
import sdl_wrapper;

Expand Down Expand Up @@ -57,7 +57,7 @@ layout(std140, set = 1, binding = 0) uniform Camera
void main()
{
gl_Position = uView * vec4(a_position, 1.0f);
v_color = vec4(1);
v_color = a_color;
})WSQ";

std::string fragment_source =
Expand Down Expand Up @@ -116,7 +116,7 @@ void main()
}

// 3. Create vertex buffer.
auto render_data = std::move(m_gpu->create_data(pw_result.value(), 3));
auto render_data = std::move(m_gpu->create_data(pw_result.value(), 4));
if (!render_data)
{
SDL_LogError(SDL_LOG_CATEGORY_ERROR, "Failed to create vertex buffer, error = %d",
Expand Down Expand Up @@ -229,7 +229,7 @@ void main()
{
bool changed = false;
auto editor_data = m_renderable->data()->vertex_view();
auto ptr = editor_data.raw;
auto raw_ptr = editor_data.raw;
for (int vertex_index = 0; vertex_index < editor_data.vertex_count; ++vertex_index)
{
for (const auto& format : editor_data.layout.get_vertex_reflection().inputs)
Expand All @@ -242,22 +242,29 @@ void main()
{
case 3:
changed |= ImGui::DragFloat3(std::format("{}{}", format.name, vertex_index).data(),
reinterpret_cast<float*>(ptr), 0.01f, -1.f, 1.f);
reinterpret_cast<float*>(raw_ptr), 0.01f, -1.f, 1.f);
break;
case 4:
changed |= ImGui::DragFloat4(std::format("{}{}", format.name, vertex_index).data(),
reinterpret_cast<float*>(ptr), 0.01f, -1.f, 1.f);
reinterpret_cast<float*>(raw_ptr), 0.01f, -1.f, 1.f);
}
}
break;
default:
break;
}
auto size = sopho::get_size(sopho::to_sdl_format(format.basic_type, format.vector_size));
ptr += size;
raw_ptr += size;
}
}

auto index_view = m_renderable->data()->index_view();
auto index_ptr = index_view.raw;
for (int index_index = 0; index_index < 2; ++index_index)
{
changed |= ImGui::InputInt3(std::format("index_{}", index_index).data(),
reinterpret_cast<int*>(index_ptr));

Choose a reason for hiding this comment

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

warning: do not use reinterpret_cast [cppcoreguidelines-pro-type-reinterpret-cast]

                                                reinterpret_cast<int*>(index_ptr));
                                                ^

index_ptr += 3 * sizeof(int);
}
if (changed)
{
auto upload_result = m_renderable->data()->upload();
Expand Down Expand Up @@ -431,10 +438,13 @@ void main()
SDL_PushGPUVertexUniformData(commandBuffer, 0, cam.m.data(), static_cast<std::uint32_t>(sizeof(cam.m)));
}

SDL_BindGPUVertexBuffers(renderPass, 0, m_renderable->data()->get_buffer_binding().data(),
m_renderable->data()->get_buffer_binding().size());
SDL_BindGPUVertexBuffers(renderPass, 0, m_renderable->data()->get_vertex_buffer_binding().data(),
m_renderable->data()->get_vertex_buffer_binding().size());

SDL_BindGPUIndexBuffer(renderPass, &m_renderable->data()->get_index_buffer_binding(),
SDL_GPU_INDEXELEMENTSIZE_32BIT);

SDL_DrawGPUPrimitives(renderPass, 3, 1, 0, 0);
SDL_DrawGPUIndexedPrimitives(renderPass, 6, 1, 0, 0, 0);

Choose a reason for hiding this comment

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

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

        SDL_DrawGPUIndexedPrimitives(renderPass, 6, 1, 0, 0, 0);
                                                 ^


ImGui_ImplSDLGPU3_RenderDrawData(draw_data, commandBuffer, renderPass);

Expand Down Expand Up @@ -505,4 +515,4 @@ void main()
* @return sopho::App* Pointer to a heap-allocated application object; the caller takes ownership and is responsible for
* deleting it.
*/
sopho::App* create_app(int argc, char** argv) { return new UserApp(); }
sopho::checkable<sopho::App*> create_app(int argc, char** argv) { return new UserApp(); }

Choose a reason for hiding this comment

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

warning: function 'create_app' can be made static or moved into an anonymous namespace to enforce internal linkage [misc-use-internal-linkage]

Suggested change
sopho::checkable<sopho::App*> create_app(int argc, char** argv) { return new UserApp(); }
static sopho::checkable<sopho::App*> create_app(int argc, char** argv) { return new UserApp(); }

Choose a reason for hiding this comment

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

warning: parameter 'argc' is unused [misc-unused-parameters]

Suggested change
sopho::checkable<sopho::App*> create_app(int argc, char** argv) { return new UserApp(); }
sopho::checkable<sopho::App*> create_app(int /*argc*/, char** argv) { return new UserApp(); }

Choose a reason for hiding this comment

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

warning: parameter 'argv' is unused [misc-unused-parameters]

Suggested change
sopho::checkable<sopho::App*> create_app(int argc, char** argv) { return new UserApp(); }
sopho::checkable<sopho::App*> create_app(int argc, char** /*argv*/) { return new UserApp(); }

2 changes: 1 addition & 1 deletion sdl_wrapper/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,4 @@ target_sources(sdl_wrapper
sdl_callback_implement.cpp
)

target_link_libraries(sdl_wrapper PUBLIC SDL3::SDL3 shaderc glsl_reflector)
target_link_libraries(sdl_wrapper PUBLIC SDL3::SDL3 shaderc glsl_reflector data_type)
15 changes: 15 additions & 0 deletions sdl_wrapper/arch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# SDL Wrapper Arch

```mermaid
graph TD

subgraph RenderData
BufferWrapper
end

Renderable --> RenderProcedural
Renderable --> RenderData
RenderProcedural --> GpuWrapper
RenderData --> GpuWrapper

```
11 changes: 6 additions & 5 deletions sdl_wrapper/sdl_callback_implement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@
// Created by sophomore on 11/9/25.
//
#define SDL_MAIN_USE_CALLBACKS
#include <SDL3/SDL.h>
#include <SDL3/SDL_main.h>
#include "SDL3/SDL.h"
#include "SDL3/SDL_main.h"

Choose a reason for hiding this comment

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

warning: included header SDL.h is not used directly [misc-include-cleaner]

Suggested change
#include "SDL3/SDL_main.h"
#include "SDL3/SDL_main.h"

import sdl_wrapper;

Choose a reason for hiding this comment

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

warning: module 'sdl_wrapper' not found [clang-diagnostic-error]

import sdl_wrapper;
       ^

import data_type;

extern sopho::App* create_app(int argc, char** argv);
extern sopho::checkable<sopho::App*> create_app(int argc, char** argv);

/**
* @brief Initializes the SDL video subsystem, constructs the application, and invokes its initialization.
Expand All @@ -26,8 +27,8 @@ SDL_AppResult SDL_AppInit(void** appstate, int argc, char** argv)
auto app = create_app(argc, argv);
if (!app)
return SDL_APP_FAILURE;
*appstate = app;
return app->init(argc, argv);
*appstate = app.value();
return app.value()->init(argc, argv);
}

/**
Expand Down
5 changes: 3 additions & 2 deletions sdl_wrapper/sdl_wrapper.buffer.ixx
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ module;
#include "SDL3/SDL_gpu.h"

export module sdl_wrapper:buffer;
import data_type;
import :decl;

export namespace sopho
namespace sopho
{
class BufferWrapper
{
Expand Down Expand Up @@ -42,7 +43,7 @@ export namespace sopho

/// Returns the underlying SDL_GPUBuffer pointer.
[[nodiscard]] SDL_GPUBuffer* gpu_buffer() const noexcept { return m_gpu_buffer; }
[[nodiscard]] auto cpu_buffer() noexcept { return m_cpu_buffer.data(); }
[[nodiscard]] std::byte* cpu_buffer() noexcept { return m_cpu_buffer.data(); }

~BufferWrapper() noexcept;

Expand Down
21 changes: 0 additions & 21 deletions sdl_wrapper/sdl_wrapper.decl.ixx
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,6 @@ export module sdl_wrapper:decl;

export namespace sopho
{
enum class GpuError
{
UNINITIALIZED,
CREATE_DEVICE_FAILED,
CREATE_WINDOW_FAILED,
CLAIM_WINDOW_FAILED,
CREATE_GPU_BUFFER_FAILED,
CREATE_TRANSFER_BUFFER_FAILED,
CREATE_SHADER_FAILED,
CREATE_PIPELINE_FAILED,
GET_TEXTUREFORMAT_FAILED,
BUFFER_OVERFLOW,
MAP_TRANSFER_BUFFER_FAILED,
ACQUIRE_COMMAND_BUFFER_FAILED,
SUBMIT_COMMAND_FAILED,
BEGIN_COPY_PASS_FAILED,
COMPILE_VERTEX_SHADER_FAILED,
COMPILE_FRAGMENT_SHADER_FAILED,

};

class App;
class GpuWrapper;
class BufferWrapper;
Expand Down
14 changes: 10 additions & 4 deletions sdl_wrapper/sdl_wrapper.gpu.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,18 @@ namespace sopho
uint32_t vertex_count)
{
auto size = render_procedural.vertex_layout().get_stride() * vertex_count;
auto buffer = create_buffer(SDL_GPU_BUFFERUSAGE_VERTEX, size);
if (!buffer)
auto vertex_buffer = create_buffer(SDL_GPU_BUFFERUSAGE_VERTEX, size);
auto index_buffer = create_buffer(SDL_GPU_BUFFERUSAGE_INDEX, 6 * sizeof(int));
if (!vertex_buffer)
{
return std::unexpected(buffer.error());
return std::unexpected(vertex_buffer.error());
}
return RenderData{std::move(buffer.value()), render_procedural.vertex_layout(), vertex_count};
if (!index_buffer)
{
return std::unexpected(index_buffer.error());
}
return RenderData{std::move(vertex_buffer.value()), std::move(index_buffer.value()),
render_procedural.vertex_layout(), vertex_count};
}
/**
* @brief Create a RenderProcedural configured for the device's texture format.
Expand Down
2 changes: 1 addition & 1 deletion sdl_wrapper/sdl_wrapper.gpu.ixx
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ export namespace sopho
GpuWrapper& operator=(GpuWrapper&&) = delete;

[[nodiscard]] auto device() const { return m_ctx.device.raw; }
[[nodiscard]] auto window() const { return m_ctx.window.raw; }
[[nodiscard]] SDL_Window *window() const { return m_ctx.window.raw; }

[[nodiscard]] std::expected<BufferWrapper, GpuError> create_buffer(SDL_GPUBufferUsageFlags flag, uint32_t size);
[[nodiscard]] std::expected<RenderData, GpuError>
Expand Down
35 changes: 27 additions & 8 deletions sdl_wrapper/sdl_wrapper.render_data.ixx
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,22 @@ namespace sopho
{
export class RenderData
{
BufferWrapper m_buffer;
BufferWrapper m_vertex_buffer;
BufferWrapper m_index_buffer;
VertexLayout m_layouts{};
size_t m_vertex_count{};
std::vector<SDL_GPUBufferBinding> m_bindings{};
SDL_GPUBufferBinding m_index_binding{};

public:
explicit RenderData(BufferWrapper&& buffer_wrapper, const VertexLayout& layouts, size_t vertex_count) :
m_buffer(std::move(buffer_wrapper)), m_layouts(layouts), m_vertex_count(vertex_count)
explicit RenderData(BufferWrapper&& vertex_buffer_wrapper, BufferWrapper&& index_buffer_wrapper,
const VertexLayout& layouts, size_t vertex_count) :
m_vertex_buffer(std::move(vertex_buffer_wrapper)), m_index_buffer(std::move(index_buffer_wrapper)),
m_layouts(layouts), m_vertex_count(vertex_count)
{
m_bindings.emplace_back(m_buffer.gpu_buffer(), 0);
m_bindings.emplace_back(m_vertex_buffer.gpu_buffer(), 0);
m_index_binding.buffer = m_index_buffer.gpu_buffer();
m_index_binding.offset = 0;
}

public:
Expand All @@ -31,19 +37,32 @@ namespace sopho
size_t vertex_count{};
std::byte* raw{};
};
struct IndexView
{
size_t index_count{};
std::byte* raw{};
};
Comment on lines +40 to +44
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

IndexView uses vertex_count, which can miscount indices

index_view() currently sets index_count = m_vertex_count. If the number of indices ever differs from the number of vertices (which is common), this will report the wrong count and can cause incorrect draw calls.

Consider tracking index count separately and wiring it through the constructor, e.g.:

-    VertexLayout m_layouts{};
-    size_t m_vertex_count{};
+    VertexLayout m_layouts{};
+    size_t m_vertex_count{};
+    size_t m_index_count{};

-    explicit RenderData(BufferWrapper&& vertex_buffer_wrapper, BufferWrapper&& index_buffer_wrapper,
-                        const VertexLayout& layouts, size_t vertex_count) :
-            m_vertex_buffer(std::move(vertex_buffer_wrapper)), m_index_buffer(std::move(index_buffer_wrapper)),
-            m_layouts(layouts), m_vertex_count(vertex_count)
+    explicit RenderData(BufferWrapper&& vertex_buffer_wrapper, BufferWrapper&& index_buffer_wrapper,
+                        const VertexLayout& layouts, size_t vertex_count, size_t index_count) :
+            m_vertex_buffer(std::move(vertex_buffer_wrapper)), m_index_buffer(std::move(index_buffer_wrapper)),
+            m_layouts(layouts), m_vertex_count(vertex_count), m_index_count(index_count)
@@
-        auto index_view() { return IndexView{.index_count = m_vertex_count, .raw = m_index_buffer.cpu_buffer()}; }
+        auto index_view() { return IndexView{.index_count = m_index_count, .raw = m_index_buffer.cpu_buffer()}; }

You’d then adjust call sites to pass the true index count.

Also applies to: 56-56

🤖 Prompt for AI Agents
In sdl_wrapper/sdl_wrapper.render_data.ixx around lines 40-44 (and also line
56), IndexView currently uses m_vertex_count to populate index_count which
misreports when index and vertex counts differ; change IndexView to store a
separate index_count member and add a constructor or parameter to set it from
callers, update index_view() to pass the true index count instead of
m_vertex_count, and update all call sites (including the spot around line 56) to
supply the actual index count so draw calls use the correct value.

RenderData(const RenderData&) = delete;
RenderData& operator=(const RenderData&) = delete;
RenderData(RenderData&&) = default;
RenderData& operator=(RenderData&&) = default;
auto& buffer() { return m_buffer; }
auto& get_buffer_binding() { return m_bindings; }
auto& buffer() { return m_vertex_buffer; }
auto& get_vertex_buffer_binding() { return m_bindings; }
auto& get_index_buffer_binding() { return m_index_binding; }
auto vertex_view()
{
return VertexView{.layout = m_layouts, .vertex_count = m_vertex_count, .raw = m_buffer.cpu_buffer()};
return VertexView{.layout = m_layouts, .vertex_count = m_vertex_count, .raw = m_vertex_buffer.cpu_buffer()};
}
auto index_view() { return IndexView{.index_count = m_vertex_count, .raw = m_index_buffer.cpu_buffer()}; }
auto upload()
{
return m_buffer.upload();
auto result = m_vertex_buffer.upload();
if (!result)
{
return result;
}
result = m_index_buffer.upload();
return result;
}
};
} // namespace sopho
1 change: 1 addition & 0 deletions sdl_wrapper/sdl_wrapper.render_procedural.ixx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ module;
#include "SDL3/SDL_gpu.h"
#include "shaderc/shaderc.hpp"
export module sdl_wrapper:render_procedural;
import data_type;
import :decl; // GpuError, forward declarations, etc.
import :vertex_layout;
export namespace sopho
Expand Down
2 changes: 1 addition & 1 deletion sdl_wrapper/sdl_wrapper.renderable.ixx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ namespace sopho
{
return m_render_procedural;
}
auto & data()
std::shared_ptr<RenderData> & data()
{
return m_render_data;
}
Expand Down