Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat: sdl2 #203

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from
Draft

feat: sdl2 #203

wants to merge 13 commits into from

Conversation

francisdb
Copy link
Contributor

@francisdb francisdb commented Jan 25, 2024

I ported the SDL code to SDL2.

X11 is ancient tech that is going away.
xpinmame on linux now builds against SDL2 for graphics and sound.

Some light crackling on the sound, probably something to do with pinmame not coming up with audio data in time?

SDL_AUDIODRIVER=pipewire SDL_VIDEODRIVER=wayland ./build/xpinmame -rompath ~/.pinmame/roms -nvram_directory ~/.pinmame/nvram  t2_l8

image

No luck with hidpi for now

@francisdb francisdb force-pushed the feat/sdl2 branch 2 times, most recently from 15507ba to c4599cb Compare January 25, 2024 20:13
@francisdb
Copy link
Contributor Author

Questions:

  • is it ok to enable sdl2 by default on Linux?
  • do we also want to do this for macOS?

@francisdb francisdb marked this pull request as ready for review January 25, 2024 21:38
@francisdb francisdb force-pushed the feat/sdl2 branch 3 times, most recently from b1b9406 to d32cc6c Compare January 29, 2024 09:10
@francisdb
Copy link
Contributor Author

Nobody interested?
Is this a bad idea?
Some feedback would be nice.

@jsm174
Copy link
Contributor

jsm174 commented Feb 8, 2024

The more builds and the more options the better in my opinion.

It would be nice to get a true MacOS build going without needing to install XQuartz.

Is this PR ready to be merged? If not could it be switched to draft?

@francisdb francisdb marked this pull request as draft February 16, 2024 10:41
@francisdb
Copy link
Contributor Author

francisdb commented Feb 16, 2024

No idea how to fix this inline stuff, no issues here on my machine. Must be some clang 14 vs 15 or arm vs x64 issue?

@jsm174
Copy link
Contributor

jsm174 commented Feb 16, 2024

No idea how to fix this inline stuff, no issues here on my machine. Must be some clang 14 vs 15 or arm vs x64 issue?

If you look at the other cmakes, we had to do this:

add_definitions( "-DINLINE=static inline __attribute__((always_inline))" )

So you remove the entry you have in add_compile_definitions ie, INLINE=static __inline and use above?

@francisdb
Copy link
Contributor Author

No idea how to fix this inline stuff, no issues here on my machine. Must be some clang 14 vs 15 or arm vs x64 issue?

If you look at the other cmakes, we had to do this:

add_definitions( "-DINLINE=static inline __attribute__((always_inline))" )

So you remove the entry you have in add_compile_definitions ie, INLINE=static __inline and use above?

Thanks, tried a different version I found on vpinball that did not work. What a mess.

@francisdb
Copy link
Contributor Author

Few remaining questions:

  • Do I remove the commented parts in the cmake files (x11)?
  • How/where do we document that sdl2 is required for xpinmame on mac / linux?
  • Do I duplicate CMakeLists_osx-x64.txt to CMakeLists_osx-arm64.txt or remove the platform prefix as the current x64 works for both?

@jsm174
Copy link
Contributor

jsm174 commented Feb 20, 2024

Few remaining questions:

  • Do I remove the commented parts in the cmake files (x11)?

I'd defer on this question. Maybe if X is no longer needed, it gets converted to just pinmame?

  • How/where do we document that sdl2 is required for xpinmame on mac / linux?

If you compile with the static libSDL2.a would you even need SDL2 as a requirement? It would be directly in the binary.

  • Do I duplicate CMakeLists_osx-x64.txt to CMakeLists_osx-arm64.txt or remove the platform prefix as the current x64 works for both?

For now I would keep multiple CMakeLists.txt. I understand there is one line different, but I think we need an overhaul of CMake, and I'd prefer to do that in one shot.

@gnulnulf
Copy link
Contributor

gnulnulf commented Apr 7, 2024

I tried your version but I get :
Linking xpinmame.sdl ...
/usr/bin/ld: xpinmame.obj/unix.sdl/osdepend.a(sdl.o): in function sysdep_create_display': sdl.c:(.text+0x1f6): undefined reference to SDL_GetCurrentDisplayMode'

I seem to use a different include of sdl in my sdl2 version.
Do you know why you inlcude SDL2/SDL.h and I include just SDL.h

@francisdb
Copy link
Contributor Author

I seem to use a different include of sdl in my sdl2 version. Do you know why you inlcude SDL2/SDL.h and I include just SDL.h

No idea, but does not seem to be an issue on the github builds.

@gnulnulf
Copy link
Contributor

gnulnulf commented Apr 8, 2024

I used makefile.unixsdl which contained sdl-config, after changing that to sdl2-config and SOUND_SDL=1 it worked on my side.
The clicking sound could be a signed/unsigned issue. Sound in mpu35 is good.

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.

3 participants