Skip to content

feat: add dual SDL2/SDL3 support with CMake option - #129

Open
doncollins1985 wants to merge 10 commits into
projectM-visualizer:masterfrom
doncollins1985:sdl3-upgrade
Open

feat: add dual SDL2/SDL3 support with CMake option#129
doncollins1985 wants to merge 10 commits into
projectM-visualizer:masterfrom
doncollins1985:sdl3-upgrade

Conversation

@doncollins1985

Copy link
Copy Markdown
  • Add USE_SDL CMake option (Auto/SDL2/SDL3) — prefers SDL3 when available
  • All source files use #ifdef USE_SDL3 guards for API differences: Event types, key struct members, modifier keys, timer/window functions, ImGui backend (sdl2/sdl3), GLSL version (150/300 es), audio capture API, OpenGL proc loader, drop file handling
  • Conditional SDL linkage: SDL3::SDL3 vs SDL2::SDL2 + SDL2::SDL2main
  • New cmake/SDL3Target.cmake for SDL3 target verification
  • Updated ImGui.cmake for conditional backend and kept ImGuiDemo target
  • Updated dependencies_check.cmake and packaging-linux.cmake for SDL version
  • Added audio device hotplug support for SDL3
  • Added CurrentAudioLevel() and RefreshDeviceList() to AudioCapture
  • Refactored drop file handler into HandleDropFile() method
  • SDL3 audio capture uses SDL_AudioStream API
  • SDL key constants remapped via #undef/#define for SDL3 naming
  • Guarded projectm_create_with_opengl_load_proc for older libprojectM
  • CI builds both SDL2 and SDL3 on all four platforms: Ubuntu Linux, Arch Linux, Windows (vcpkg), macOS (Homebrew)
  • vcpkg.json includes both sdl2 and sdl3
  • Windows runner pinned to windows-2022 for VS 2022 compatibility

- Add USE_SDL CMake option (Auto/SDL2/SDL3) — prefers SDL3 when available
- All source files use #ifdef USE_SDL3 guards for API differences:
  Event types, key struct members, modifier keys, timer/window functions,
  ImGui backend (sdl2/sdl3), GLSL version (150/300 es), audio capture API,
  OpenGL proc loader, drop file handling
- Conditional SDL linkage: SDL3::SDL3 vs SDL2::SDL2 + SDL2::SDL2main
- New cmake/SDL3Target.cmake for SDL3 target verification
- Updated ImGui.cmake for conditional backend and kept ImGuiDemo target
- Updated dependencies_check.cmake and packaging-linux.cmake for SDL version
- Added audio device hotplug support for SDL3
- Added CurrentAudioLevel() and RefreshDeviceList() to AudioCapture
- Refactored drop file handler into HandleDropFile() method
- SDL3 audio capture uses SDL_AudioStream API
- SDL key constants remapped via #undef/#define for SDL3 naming
- Guarded projectm_create_with_opengl_load_proc for older libprojectM
- CI builds both SDL2 and SDL3 on all four platforms:
  Ubuntu Linux, Arch Linux, Windows (vcpkg), macOS (Homebrew)
- vcpkg.json includes both sdl2 and sdl3
- Windows runner pinned to windows-2022 for VS 2022 compatibility
@doncollins1985

Copy link
Copy Markdown
Author

?

@kblaschke kblaschke 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.

Overall, a solid upgrade.

I'm not a huge fan of using #ifdefs everywhere, though given how SDL calls are spread over the code base, I don't see any other simple solution than wrapping SDL again, which doesn't make lots of sense.

I've added some comments, mostly related to the build files, packaging and check workflow. The most important one is adding features to the vcpkg manifest to prevent pulling in both libraries when only one of them is ever needed.

Comment thread CMakeLists.txt
Comment thread vcpkg.json Outdated
Comment thread packaging-linux.cmake Outdated
Comment thread .github/workflows/buildcheck.yaml Outdated
Comment thread .github/workflows/buildcheck.yaml Outdated
Comment thread .github/workflows/buildcheck.yaml Outdated
Comment thread .github/workflows/buildcheck.yaml Outdated
Comment thread .github/workflows/buildcheck.yaml Outdated
Comment thread .github/workflows/buildcheck.yaml Outdated
@kblaschke

Copy link
Copy Markdown
Member

Sorry for the late reply, I was swamped with other work in the past weeks/months, and the PRs just kept piling up.
You've not been forgotten!

doncollins1985 and others added 9 commits September 5, 2026 10:12
Co-authored-by: Kai Blaschke <kai.blaschke@kb-dev.net>
Co-authored-by: Kai Blaschke <kai.blaschke@kb-dev.net>
Co-authored-by: Kai Blaschke <kai.blaschke@kb-dev.net>
Co-authored-by: Kai Blaschke <kai.blaschke@kb-dev.net>
Co-authored-by: Kai Blaschke <kai.blaschke@kb-dev.net>
Co-authored-by: Kai Blaschke <kai.blaschke@kb-dev.net>
Co-authored-by: Kai Blaschke <kai.blaschke@kb-dev.net>
- vcpkg.json: wrap sdl2/sdl3 into 'sdl2'/'sdl3' manifest features instead of hard dependencies
- CMakeLists.txt: set VCPKG_MANIFEST_FEATURES from USE_SDL (prefer SDL3 for Auto)
- buildcheck.yaml: build SDL2 on ubuntu-24.04 and SDL3 on ubuntu-26.04 using OS packages

@kblaschke kblaschke 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.

Looks good except for the unused CurrentAudioLevel() functions, which should be removed as they seemingly originate from another feature not part of this PR.

The other comments are just minor things, I leave it to you to change them or leave it as-is.

Comment on lines +280 to +282
unsigned int samples = bytesRead / sizeof(float) / _channels;
projectm_pcm_add_float(_projectMHandle, buffer.data(), samples,
static_cast<projectm_channels>(_channels));

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.

You can call the AudioInputCallback() function here to avoid duplicate code:

Suggested change
unsigned int samples = bytesRead / sizeof(float) / _channels;
projectm_pcm_add_float(_projectMHandle, buffer.data(), samples,
static_cast<projectm_channels>(_channels));
AudioInputCallback(this, buffer.data(), bytesRead);

Or use the same synchronous technique with both SDL2 and SDL3 via SDL_AudioStreamAvailable()/SDL_AudioStreamGet() for SDL2 here.

Comment thread src/RenderLoop.cpp

switch (keyCode)
{
#ifdef USE_SDL3

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.

Just as a thought:

Instead of all these redefines, it might be easier to declare our own enum with the key codes in two separate headers, one with SDL2 and one with SDL3 constants, and then include the correct one at the top, then use the enum values instead of the SDL2 constants.

Later, we'll support custom key bindings, so mapping all key constants would be necessary anyway.

We can keep this for now, I'll refactor it when I start the key binding stuff.

Comment on lines +23 to +27
#ifdef USE_SDL3
return "SDL3 Rendering Window";
#else
return "SDL2 Rendering Window";
#endif

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.

Suggested change
#ifdef USE_SDL3
return "SDL3 Rendering Window";
#else
return "SDL2 Rendering Window";
#endif
return "SDL Rendering Window";

That's probably sufficient. Devs can still easily check which SDL library the executable is linked against if needed.

Comment thread src/AudioCapture.h
Comment on lines +62 to +66
/**
* @brief Returns the current audio peak level (0.0-1.0) for display.
* @return The current audio level, or -1.0 if not available.
*/
float CurrentAudioLevel() const;

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.

I don't see this function being used anywhere, and it's not doing anything useful.
Please remove this function and the other copies in the *Impl classes.

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.

2 participants