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
90 changes: 90 additions & 0 deletions .github/workflows/cmake-linux.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
# This starter workflow is for a CMake project running on multiple platforms. There is a different starter workflow if you just want a single platform.
# See: https://github.com/actions/starter-workflows/blob/main/ci/cmake-single-platform.yml
name: CMake on Linux

on:
push:
branches: [ "**" ]
pull_request:
branches: [ "**" ]

jobs:
build:
runs-on: ubuntu-latest
container:
image: archlinux:latest
options: --user root

strategy:
# Set fail-fast to false to ensure that feedback is delivered for all matrix combinations. Consider changing this to true when your workflow is stable.
fail-fast: false

# Set up a matrix to run the following 3 configurations:
# 1. <Windows, Release, latest MSVC compiler toolchain on the default runner image, default generator>
# 2. <Linux, Release, latest GCC compiler toolchain on the default runner image, default generator>
# 3. <Linux, Release, latest Clang compiler toolchain on the default runner image, default generator>
Comment on lines +22 to +25
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Update outdated matrix comments.

The comments describe a Windows/Linux multi-platform matrix, but this workflow only builds on Linux (Arch Linux container) with gcc and clang.

Apply this diff to fix the comments:

-      # Set up a matrix to run the following 3 configurations:
-      # 1. <Windows, Release, latest MSVC compiler toolchain on the default runner image, default generator>
-      # 2. <Linux, Release, latest GCC compiler toolchain on the default runner image, default generator>
-      # 3. <Linux, Release, latest Clang compiler toolchain on the default runner image, default generator>
+      # Set up a matrix to run the following configurations on Arch Linux:
+      # 1. <Linux, Debug/Release, latest GCC compiler toolchain>
+      # 2. <Linux, Debug/Release, latest Clang compiler toolchain>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Set up a matrix to run the following 3 configurations:
# 1. <Windows, Release, latest MSVC compiler toolchain on the default runner image, default generator>
# 2. <Linux, Release, latest GCC compiler toolchain on the default runner image, default generator>
# 3. <Linux, Release, latest Clang compiler toolchain on the default runner image, default generator>
# Set up a matrix to run the following configurations on Arch Linux:
# 1. <Linux, Debug/Release, latest GCC compiler toolchain>
# 2. <Linux, Debug/Release, latest Clang compiler toolchain>
🤖 Prompt for AI Agents
In .github/workflows/cmake-linux.yml around lines 22 to 25, the matrix comments
are outdated (they reference Windows and a multi-platform matrix) but the
workflow only runs on Linux with GCC and Clang; update the comment text to
accurately describe the current matrix: state that the job runs on an Arch Linux
container and lists two Linux configurations using GCC and Clang in Release
mode, and remove any references to Windows, MSVC, or default generator.

#
# To add more build types (Release, Debug, RelWithDebInfo, etc.) customize the build_type list.
matrix:
build_type: [Debug, Release]
c_compiler: [gcc, clang]
include:
- c_compiler: gcc
cpp_compiler: g++
- c_compiler: clang
cpp_compiler: clang++

steps:
- uses: actions/checkout@v4

- name: Set reusable strings
# Turn repeated input strings (such as the build output directory) into step outputs. These step outputs can be used throughout the workflow file.
id: strings
shell: bash
run: |
echo "build-output-dir=$GITHUB_WORKSPACE/build" >> "$GITHUB_OUTPUT"
echo "install-dir=$GITHUB_WORKSPACE/install/archlinux-latest-${{ matrix.c_compiler }}-${{ matrix.build_type }}" >> "$GITHUB_OUTPUT"
- name: Install SDL windowing deps (Linux)
run: |
pacman -Syu --noconfirm
pacman -S --noconfirm --needed \
base-devel cmake ninja pkgconf gcc clang git python3\
alsa-lib jack libpulse \
xorgproto libx11 libxext libxrandr libxcursor libxfixes libxi libxss libxtst \
libxkbcommon wayland wayland-protocols \
libdrm mesa mesa-utils
- name: Configure CMake
# Configure CMake in a 'build' subdirectory. `CMAKE_BUILD_TYPE` is only required if you are using a single-configuration generator such as make.
# See https://cmake.org/cmake/help/latest/variable/CMAKE_BUILD_TYPE.html?highlight=cmake_build_type
run: >
cmake -B ${{ steps.strings.outputs.build-output-dir }}
-DCMAKE_CXX_COMPILER=${{ matrix.cpp_compiler }}
-DCMAKE_C_COMPILER=${{ matrix.c_compiler }}
-DCMAKE_BUILD_TYPE=${{ matrix.build_type }}
-S $GITHUB_WORKSPACE
-G "Ninja Multi-Config"
- name: Build
# Build your program with the given configuration. Note that --config is needed because the default Windows generator is a multi-config generator (Visual Studio generator).
run: cmake --build ${{ steps.strings.outputs.build-output-dir }} --config ${{ matrix.build_type }}

- name: Test
working-directory: ${{ steps.strings.outputs.build-output-dir }}
# Execute tests defined by the CMake configuration. Note that --build-config is needed because the default Windows generator is a multi-config generator (Visual Studio generator).
# See https://cmake.org/cmake/help/latest/manual/ctest.1.html for more detail
run: ctest --build-config ${{ matrix.build_type }}

- name: Install
run: >
cmake --install ${{ steps.strings.outputs.build-output-dir }}
--config ${{ matrix.build_type }}
--prefix "${{ steps.strings.outputs.install-dir }}"
- name: Upload install folder
uses: actions/upload-artifact@v4
with:
name: archlinux-latest-${{ matrix.c_compiler }}-${{ matrix.build_type }}-install-${{ github.sha }}
path: ${{ steps.strings.outputs.install-dir }}/
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# This starter workflow is for a CMake project running on multiple platforms. There is a different starter workflow if you just want a single platform.
# See: https://github.com/actions/starter-workflows/blob/main/ci/cmake-single-platform.yml
name: CMake on multiple platforms
name: CMake on Windows

on:
push:
Expand All @@ -23,28 +23,19 @@ jobs:
#
# To add more build types (Release, Debug, RelWithDebInfo, etc.) customize the build_type list.
matrix:
os: [ubuntu-latest, windows-latest]
os: [windows-latest]
build_type: [Debug, Release]
c_compiler: [gcc, clang, cl]
include:
- os: windows-latest
c_compiler: cl
cpp_compiler: cl
- os: ubuntu-latest
c_compiler: gcc
cpp_compiler: g++
- os: ubuntu-latest
c_compiler: clang
cpp_compiler: clang++
- os: windows-latest
c_compiler: gcc
cpp_compiler: g++
- os: windows-latest
c_compiler: clang
cpp_compiler: clang++
exclude:
- os: ubuntu-latest
c_compiler: cl

steps:
- uses: actions/checkout@v4
Expand All @@ -69,6 +60,7 @@ jobs:
libxkbcommon-dev libwayland-dev wayland-protocols \
libdrm-dev libgbm-dev libgl1-mesa-dev libgles2-mesa-dev libegl1-mesa-dev \
libdbus-1-dev libudev-dev


- name: Configure CMake
# Configure CMake in a 'build' subdirectory. `CMAKE_BUILD_TYPE` is only required if you are using a single-configuration generator such as make.
Expand Down
8 changes: 6 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
cmake_minimum_required(VERSION 3.20)
cmake_minimum_required(VERSION 3.30)
project(SDL_TEST LANGUAGES C CXX)
set(CMAKE_CXX_STANDARD 23)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
include(FetchContent)

set(FETCHCONTENT_QUIET OFF CACHE BOOL "" FORCE)
Expand Down Expand Up @@ -91,8 +92,11 @@ target_include_directories(imgui PUBLIC
)
target_link_libraries(imgui PUBLIC SDL3::SDL3)

add_subdirectory(sdl_wrapper)

add_executable(SDL_TEST main.cpp)
target_link_libraries(SDL_TEST PRIVATE shaderc imgui)
target_link_libraries(SDL_TEST PRIVATE shaderc imgui sdl_wrapper)


include(GNUInstallDirs)
install(TARGETS SDL_TEST
Expand Down
4 changes: 4 additions & 0 deletions main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
#include <SDL3/SDL.h>
#include <SDL3/SDL_main.h>

import sdl_wrapper;

// the vertex input layout
struct Vertex {
float x, y, z; //vec3 position
Expand Down Expand Up @@ -48,6 +50,8 @@ void main()
FragColor = v_color;
})WSQ";

sopho::BufferWrapper BufferWrapper{};
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove unused BufferWrapper instance.

The global BufferWrapper instance is declared but never used. The existing code continues to use the separate SDL_GPUBuffer *vertexBuffer (line 57) directly throughout the application.

Additionally, the variable name BufferWrapper shadows the class name sopho::BufferWrapper, which can cause confusion.

Consider one of the following:

  • If this is a skeleton for future refactoring, add a TODO comment explaining the migration plan.
  • If you'd like to properly integrate the wrapper, I can help refactor the code to use the BufferWrapper class instead of the raw pointer.
  • Otherwise, remove both the import (line 12) and this declaration to keep the code clean.
🤖 Prompt for AI Agents
In main.cpp around line 53, the global sopho::BufferWrapper instance named
BufferWrapper is unused and shadows the type name; remove this declaration and
any corresponding unused include at line 12, or if you intend to migrate to the
wrapper add a one-line TODO above the declaration describing the refactor plan
and rename the variable to avoid shadowing (e.g., bufferWrapperInstance) and
replace uses of SDL_GPUBuffer* with sopho::BufferWrapper methods; if you choose
removal, also delete the unused include to keep the file clean.


SDL_Window *window;
SDL_GPUDevice *device;
SDL_GPUBuffer *vertexBuffer;
Expand Down
14 changes: 14 additions & 0 deletions sdl_wrapper/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
cmake_minimum_required(VERSION 3.30)
project(sdl_wrapper LANGUAGES CXX)

set(CMAKE_CXX_STANDARD 23)
add_library(sdl_wrapper STATIC)

target_sources(sdl_wrapper
PUBLIC
FILE_SET cxx_modules TYPE CXX_MODULES FILES
${CMAKE_CURRENT_SOURCE_DIR}/sdl_wrapper.ixx
${CMAKE_CURRENT_SOURCE_DIR}/sdl_wrapper.buffer.ixx
)

target_link_libraries(sdl_wrapper PUBLIC SDL3::SDL3)
13 changes: 13 additions & 0 deletions sdl_wrapper/sdl_wrapper.buffer.ixx
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
//
// Created by sophomore on 11/8/25.
//

export module sdl_wrapper:buffer;
#include "SDL3/SDL_gpu.h"

namespace sopho {
export class BufferWrapper {
SDL_GPUBuffer *vertexBuffer{};

};
}
Comment on lines +8 to +13
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

BufferWrapper is unused and provides no functionality.

The BufferWrapper class is declared with a private vertexBuffer member but has no public interface, constructor, or destructor. The global instance declared in main.cpp (line 53) is never used, and the existing code continues to use the separate global SDL_GPUBuffer *vertexBuffer (main.cpp line 57) directly.

Consider one of the following:

  • If this is a skeleton for future work, add a TODO comment explaining the intended functionality.
  • If you want to proceed with the wrapper pattern, would you like me to help design a proper RAII wrapper that manages the buffer lifecycle?
  • If this code serves no purpose yet, consider removing it to keep the codebase clean.
🤖 Prompt for AI Agents
In sdl_wrapper/sdl_wrapper.buffer.ixx around lines 8 to 13, the BufferWrapper
class declares a private SDL_GPUBuffer* vertexBuffer but has no public API or
lifecycle management and the global instance is unused; either remove the class
to clean up dead code, or implement it as a proper RAII wrapper: add
constructor(s) that create/accept a buffer, a destructor that frees/releases the
SDL_GPUBuffer, and public accessors/mutators (or move/copy semantics) so
existing code can use the wrapper instead of the raw global pointer; if you
intend to leave it as a placeholder, add a clear TODO comment explaining
intended functionality and link to a follow-up task.

6 changes: 6 additions & 0 deletions sdl_wrapper/sdl_wrapper.ixx
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
//
// Created by sophomore on 11/7/25.
//

export module sdl_wrapper;
export import :buffer;