ColinPitrat / caprice32

An emulator of the Amstrad CPC 8bit home computer range.
GNU General Public License v2.0
146 stars 32 forks source link

Fix segfault on Mac, OpenGL being deprecated in favor of Metal since #201

Closed devnexen closed 3 years ago

devnexen commented 3 years ago
years, it is not possible to dynamically load it.
devnexen commented 3 years ago

not sure if this is still viable plugin but just not to give false impression it has a chance to work on mac.

ColinPitrat commented 3 years ago

Have you built it on mac? I'm interested by feedback on it. Ideally I'd also like to be able to have a continuous build for it, potentially releasing a ready to use binary.

Compiling without HAVEGL should do the trick but I'm OK to add this if it works.

devnexen commented 3 years ago

In fact I build it on a mac indeed.

devnexen commented 3 years ago

Have you built it on mac? I'm interested by feedback on it.

Other than that, the default SDL2 mode works just fine as the audio too.

ColinPitrat commented 3 years ago

What does it give when selecting OpenGL video plugin with your change? I think it'd be better to define HAVE_GL to be false if on mac. Something like ifdef APPLE define HAVE_GL 0, or undef HAVE_GL

devnexen commented 3 years ago

What does it give when selecting OpenGL video plugin with your change?

it segfaults. To explain the reason, since Mojave OpenGL is deprecated (still available tough but not as straight shared library anymore so you can t "dlopen" it like in the "old days"), passing libGL.dylib or null (means default path but for macOs case the said path set by SDL does not exist anymore) to SDL_GL_LoadLibrary does not solve it.

I think it'd be better to define HAVE_GL to be false if on mac. Something like ifdef APPLE define HAVE_GL 0, or undef HAVE_GL

Your advice is valid, I agree.

devnexen commented 3 years ago

What does it give when selecting OpenGL video plugin with your change?

Sorry I misread, the change just gave an error message but I followed your advice anyway.

ColinPitrat commented 3 years ago

I'm trying to setup a macos build using GitHub Actions. This reproduces the issue: https://github.com/ColinPitrat/caprice32/actions

I think make WITHOUT_GL=1 would be the right approach, but deducing it without the option for Mac would be nice too.

ColinPitrat commented 3 years ago

I started a Mac build with GitHub actions and then got caught it and forgot that the purpose was to test this :-) Taking a look at removing WITHOUT_GL now: https://github.com/ColinPitrat/caprice32/commit/5b69719f42a45ca0555c5498f5b1ad1479afb5da

ColinPitrat commented 3 years ago

I pushed a different fix in https://github.com/ColinPitrat/caprice32/commit/79f30e31cc78a7a4875e1574542fdbe8f39aad34

Sorry if this feels a bit NIH, it's just that HAVE_GL is used in many places and I wanted to ensure they are all activated or deactivated together.

devnexen commented 3 years ago

I saw it :) fine by me

ColinPitrat commented 3 years ago

There's now a MacOS bundle attached to latest release (only latest for now). I think it should work fine. If you have a mac, can you give it a try? There are probably improvements that can be done (icon, metadata, etc ...)

devnexen commented 3 years ago

I m guessing it s for Mac Intel (eventually for Rosetta) ?

ColinPitrat commented 3 years ago

Yes, I suppose so :)

Le sam. 10 avr. 2021 à 18:22, David CARLIER @.***> a écrit :

I m guessing it s for Mac Intel (eventually for Rosetta) ?

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/ColinPitrat/caprice32/pull/201#issuecomment-817174288, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALYNZ5LVCSJ33B2534PXGTTICCOPANCNFSM42RPKIAQ .

devnexen commented 3 years ago

Yes it is just checked, you might need to build what is called universal binary (if that is possible) https://developer.apple.com/documentation/apple-silicon/building-a-universal-macos-binary

ColinPitrat commented 3 years ago

That's what I use: https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners

I'll look into universal builds.

devnexen commented 3 years ago

Sure ; if not it work ok under Rosetta ;-)