search

DescentDevelopers / Descent3

Descent 3 by Outrage Entertainment
GNU General Public License v3.0
2.73k stars 231 forks source link

Use SDL2 on Windows #457

Closed Lgt2x closed 5 days ago

Lgt2x commented 1 week ago

This PR makes Windows use SDL2 instead of DirectX, to be on par with the other supported platforms. As a consequence, now the Windows executable:

Windows 32bit has been dropped because of compatibility issues. Note that Linux and OSX 32bit are not built either.

The internal editor gave me a some trouble: this MFC app relies a lot on DirectX tools and interacts with the game through it, and is not a solved problem as of this PR. Making it use the SDL2 game properly is going to require additional work. One solution I tried was to keep using directX for the game on the editor. This way, we would build both DirectInput and SDL2 Input for the ddio module in the form of 2 separate static libraries, but as many modules links to it, that would mean many modules have both a SDL2 and a DirectX version, which would be cumbersome. The solution to get a successful build here is a hybrid one, building the game used in the editor with SDL2, and the rest is still using DirectX. It compiles, but has runtime problems (the editor was broken before this PR anyway)

Future work:

Depends on #453 and #456 / #449

JeodC commented 1 week ago

You probably know, but in windows x64 there is no audio in the main menu. I didn't test further into levels. Mve audio played fine.

Lgt2x commented 1 week ago

yep, I did not look into it yet

Lgt2x commented 1 week ago

rebased on master, nothing obvious comes up to fix the sound, any help welcome, I'll try again later

pzychotic commented 1 week ago

rebased on master, nothing obvious comes up to fix the sound, any help welcome, I'll try again later

I tested it on Windows and had no audio at all, except for movies.

I tracked it down to a change in how the SDL audio device is getting created for movies and the rest of the game.

The callstack for movies is:

Descent3.exe!open_audio_device(const char * devname, int iscapture, const SDL_AudioSpec * desired, SDL_AudioSpec * obtained, int allowed_changes, int min_id) Line 1314
Descent3.exe!SDL_OpenAudio_REAL(SDL_AudioSpec * desired, SDL_AudioSpec * obtained) Line 1593
Descent3.exe!SDL_OpenAudio(SDL_AudioSpec * a, SDL_AudioSpec * b) Line 112
Descent3.exe!StartupSoundSystem(LnxSoundDevice * dev) Line 516
Descent3.exe!LnxSound_CreateSoundBuffer(LnxSoundDevice * dev, SysSoundBufferDesc * lbdesc, LnxSoundBuffer * * lsndb) Line 168
Descent3.exe!LnxSound_CreateSoundBuffer(LnxSoundDevice * dev, SysSoundBufferDesc * lbdesc, LnxSoundBuffer * * lsndb) Line 101
Descent3.exe!'anonymous namespace'::MovieSoundDevice::CreateSoundBuffer(SysSoundBufferDesc * lbdesc, ISysSoundBuffer * * lsndb) Line 225
Descent3.exe!sndConfigure(unsigned int rate, unsigned int buflen, unsigned int stereo, unsigned int frequency, unsigned int bits16, unsigned int comp16) Line 362
Descent3.exe!MVE_rmStepMovie() Line 1215
Descent3.exe!mve_PlayMovie(const char * pMovieName, oeApplication * pApp) Line 355
Descent3.exe!PlayMovie(const char * moviename) Line 113

going through sndConfigure() and calling open_audio_device() with allowed_changes = 0

While the callstack for the menu and other things looks like this:

Descent3.exe!open_audio_device(const char * devname, int iscapture, const SDL_AudioSpec * desired, SDL_AudioSpec * obtained, int allowed_changes, int min_id) Line 1314
Descent3.exe!SDL_OpenAudioDevice_REAL(const char * device, int iscapture, const SDL_AudioSpec * desired, SDL_AudioSpec * obtained, int allowed_changes) Line 1611
Descent3.exe!SDL_OpenAudioDevice(const char * a, int b, const SDL_AudioSpec * c, SDL_AudioSpec * d, int e) Line 115
Descent3.exe!lnxsound::InitSoundLib(char mixer_type, oeApplication * sos, unsigned char max_sounds_played) Line 103
Descent3.exe!hlsSystem::InitSoundLib(oeApplication * sos, char mixer_type, char quality, bool f_kill_sound_list) Line 536
Descent3.exe!InitD3Systems1(bool editor) Line 1901
Descent3.exe!Descent3() Line 464

going through lnxsound::InitSoundLib() and calling open_audio_device() with allowed_changes = SDL_AUDIO_ALLOW_ANY_CHANGE

If I change the SDL_AUDIO_ALLOW_ANY_CHANGE to 0 in https://github.com/DescentDevelopers/Descent3/blob/95f3efe11a9db5baf68d1f11ad1a0e00220c331f/sndlib/sdlsound.cpp#L102 it works for me.

This change seems to boil down to SDL_audio.c Line 1478 where build_stream=SDL_TRUE now and SDL_NewAudioStream() gets called.

I have no clue about any consequences this might have for other platforms or in general...

Lgt2x commented 1 week ago

@pzychotic you rock, thanks for investigating! you've saved me at least a couple hours of sleep tonight :) I'll give your fix a try later and report back

JeodC commented 1 week ago

@pzychotic you rock, thanks for investigating! you've saved me at least a couple hours of sleep tonight :) I'll give your fix a try later and report back

This should, in theory, work on all platforms, but definitely test it and verify. According to documentation for SDL_OpenAudioDevice, setting the flags to 0 effectively restricts SDL from changing the audio behavior when the hardware device is unable to offer a specific feature.

I'm not at all certain what this change would mean for embedded systems like android that don't have similar audio hardware and drivers like the three core platforms. Maybe add a fallback to where if SDL fails to init audio, go back to the SDL_AUDIO_ALLOW_ANY_CHANGE flag and try again. That sort of conditional should be easy enough to add.

JeodC commented 1 week ago

So at sdlsound.cpp do this, maybe.


  sound_device = SDL_OpenAudioDevice(nullptr, 0, &spec, nullptr, 0);
  if (sound_device == 0) {
    strcpy(m_error_text, SDL_GetError());
    // Retry
    sound_device = SDL_OpenAudioDevice(nullptr, 0, &spec, nullptr, SDL_AUDIO_ALLOW_ANY_CHANGE);
    if (sound_device == 0) {
      strcpy(m_error_text, SDL_GetError());
    }
    return false;
  }
Lgt2x commented 1 week ago

Sound fix tested and approved on Windows! This PR is now open for review (@winterheart @Arcnor ?)

This should, in theory, work on all platforms, but definitely test it and verify. According to documentation for SDL_OpenAudioDevice, setting the flags to 0 effectively restricts SDL from changing the audio behavior when the hardware device is unable to offer a specific feature.

Let's not preemptively fix a bug we don't have yet and we can't test. First, find a case where this is proven to be problematic

Jayman2000 commented 1 week ago

I just noticed something. This PR’s description says:

This PR makes Windows use SDL2 instead of DirectX, to be on par with the other supported platforms. As a consequence, now the Windows executable:

  • Is fully 64-bit compatible

[…]

Windows 32bit has been dropped because of compatibility issues.

At the same time, this PR’s README says:

Building - Windows

[…]

  1. Open a “x86 Native Tools Command Prompt” and run:

    git clone https://github.com/DescentDevelopers/Descent3
cd Descent3
cmake --preset win32 -D ENABLE_LOGGER=[ON|OFF]

I think that the “x86 Native Tools Command Prompt” and the --preset win32 parts need to be updated.

Lgt2x commented 6 days ago

I think that the “x86 Native Tools Command Prompt” and the --preset win32 parts need to be updated.

Readme updated

Lgt2x commented 6 days ago

huh is updated builtin-baseline too recent for Github-actions?

Lgt2x commented 6 days ago

rebased (correctly) on master

winterheart commented 5 days ago

Project compiles, but cannot start due failing to find required OpenGL symbols on dlopen. See https://github.com/Lgt2x/Descent3/pull/7 for proposed fix. Didn't test deeper, but after applying fix critical parts of game runs fine. Tested video playback, main screen UI, starting training mission and Retribution.

There only one non-critical regression found: all sound and briefings goes in German language. I don't know why, seems that happens because I'm using non-English locale on Windows and game incorrectly detects it as German.

Lgt2x commented 5 days ago

Project compiles, but cannot start due failing to find required OpenGL symbols on dlopen. See Lgt2x#7 for proposed fix. Didn't test deeper, but after applying fix critical parts of game runs fine. Tested video playback, main screen UI, starting training mission and Retribution.

There only one non-critical regression found: all sound and briefings goes in German language. I don't know why, seems that happens because I'm using non-English locale on Windows and game incorrectly detects it as German.

Du sprichst nicht Deutsch? :D More seriously locale detection looks broken, I suspect this is also the case on Linux. I want to tackle this in a future PR. I created #477 to report it.

Your fix looks correct, I tested & cherry-picked it. It makes more sense to use the SDL loading functions for OpenGL just like Linux does. I don't know why it worked for me and others, maybe system version differences.

winterheart commented 5 days ago

Well, that was worked earlier for me too, I don't know why is was broken for this time...