Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

SDL3: Add port #2069

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

SDL3: Add port #2069

wants to merge 1 commit into from

Conversation

er2off
Copy link

@er2off er2off commented Mar 5, 2025

This PR adds initial workable SDL3 port into upstream.

The only issue I found is problem with switching from windowed mode to fullscreen.
Some more tests may be needed to find other issues (like I didn't checked software rendering yet, didn't checked voice recording or game controllers)

if (clgame.client_dll_uses_sdl)
Con_Printf( S_NOTE "%s uses %s for mouse input\n", name, "SDL2" );
else
{
Copy link
Member

Choose a reason for hiding this comment

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

This is incorrect. We check whether client.dll is linked directly to SDL2.dll because it might use SDL2 API for mouse input, and therefore need relative mouse mode enabled.

With SDL3, it's the same, we can ship sdl2-compat for GoldSrc mods, just in case they want SDL2 API for mouse input.

Checking whether client.dll wants SDL3 or not would be viable only if Valve migrate GoldSrc to SDL3, which for now (?) doesn't seem very likely to me.

@@ -212,7 +217,10 @@ void IN_SetRelativeMouseMode( qboolean set )

if( set && !s_bRawInput )
{
#if XASH_SDL == 2
#if XASH_SDL == 3
SDL_GetRelativeMouseState(NULL, NULL);
Copy link
Member

Choose a reason for hiding this comment

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

Spaces in parentheses, please.

If you're not sure what to do, we have uncrustify config, some editors allow only to reformat by selection.

I can also do this myself later, if you don't want to bother.

@@ -17,7 +17,12 @@ GNU General Public License for more details.
*/

#if XASH_SDL
#include <SDL.h> // SDL_GetBasePath
// SDL_GetBasePath
#if XASH_SDL == 3
Copy link
Member

Choose a reason for hiding this comment

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

As I told you in chat, you can just include <SDL.h>, as we always add include/SDL3 to the include path.

@@ -240,7 +251,11 @@ void IN_SetMouseGrab( qboolean set )
if( set && !s_bMouseGrab )
{
#if XASH_SDL
#if XASH_SDL == 3
Copy link
Member

Choose a reason for hiding this comment

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

You can do

#if XASH_SDL == 3
#elif XASH_SDL
#endif     

here.

@@ -264,25 +277,43 @@ void SDLash_HandleGameControllerEvent( SDL_Event *ev )
switch( ev->type )
{
case SDL_CONTROLLERAXISMOTION:
#if SDL_VERSION_ATLEAST( 3, 2, 0 )
SDLash_SetActiveGameController( ev->gaxis.which );
Copy link
Member

Choose a reason for hiding this comment

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

When I tried to do my own port, I just redefined caxis/cbutton/csensor/etc to it's g**** counterparts to avoid code duplication.

@@ -204,7 +267,9 @@ Makes sure dma.buffer is valid
*/
void SNDDMA_BeginPainting( void )
{
#if !SDL_VERSION_ATLEAST(3, 2, 0)
SDL_LockAudioDevice( sdl_dev );
Copy link
Member

Choose a reason for hiding this comment

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

In theory this should be done anyway, because SNDDMA_BeginPainting/SNDDMA_Submit is basically locks/unlocks the mutex in SDL sound thread.

When engine calls BeginPainting, it means that it will start writing to the dma.buffer and sound thread shouldn't read anything from it. Submit does the opposite.

@@ -343,8 +452,10 @@ qboolean VoiceCapture_Lock( qboolean lock )
if( SDLash_IsAudioError( in_dev ))
return false;

#if !SDL_VERSION_ATLEAST( 3, 2, 0 ) // SDL3 have internal locks for streams
Copy link
Member

Choose a reason for hiding this comment

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

Same here. How does these internal locks know whether it's safe to read or write into engine's buffers?

There is https://wiki.libsdl.org/SDL3/SDL_LockAudioStream, but even there they say about their internal locks. I don't understand when they are supposed to call our audio callback?

@@ -30,7 +30,7 @@ so it can unlock and free the data block after it has been played.
=======================================================================
*/

dma_t dma;
//dma_t dma;
Copy link
Member

Choose a reason for hiding this comment

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

If it's unused, you can just remove it.

@@ -1197,18 +1410,21 @@ qboolean VID_SetMode( void )
if( iScreenWidth < VID_MIN_WIDTH ||
iScreenHeight < VID_MIN_HEIGHT ) // trying to get resolution automatically by default
{
#if SDL_VERSION_ATLEAST( 2, 0, 0 )
#if !defined( DEFAULT_MODE_WIDTH ) || !defined( DEFAULT_MODE_HEIGHT )
#if defined( DEFAULT_MODE_WIDTH ) || defined( DEFAULT_MODE_HEIGHT )
Copy link
Member

Choose a reason for hiding this comment

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

Considering the amount of macros here, how about moving the worst offenders to separate files or even sdl3 folder?

I think vid_ stuff might benefit from it, as almost all macros will be gone.

@a1batross
Copy link
Member

a1batross commented Mar 5, 2025

Also, please rename the commit to follow our contribution guide.

SDL3: Add port is sure useful, but engine: client: initial sdl3 port at least says that all changes are only client-specific, and don't touch server/dedicated logic, renderers or other modules. A full explanation of what has been done in commit message is also in general is good.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants