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

SDL1.2 internal state is not initialized before calling SDL_ functions #15

Open
ghost opened this issue Jan 14, 2015 · 9 comments
Open

Comments

@ghost
Copy link

ghost commented Jan 14, 2015

It seems 87ef789 from pull request #14 broke the SDL1.2 initialization. On parts of the platforms the main function must be replaced and instead the private SDL-main (parachute) function has to be called first. This is the only use of this SDL_main.h include. The programs main function is redefined as SDL_main and the main function is included using a special static library. The normal startup will therefore be main(SDLmain.a) -> SDL_main(previously known called main in the actual program source code). Of course, main() is not always called main() but I just use it here as placeholder for the real name.

This special indirection is required to initialize the internal state of SDL before the actual program calls any SDL function.

A similar problem already caused the windows builds to break (in a very hard to debug way) in the past.

@ghost ghost changed the title SDL internal state is not initialized before calling SDL_ functions SDL1.2 internal state is not initialized before calling SDL_ functions Jan 14, 2015
@ghost
Copy link
Author

ghost commented Jan 14, 2015

One way to deal with it would be to drop SDL 1.2 support

@littleguy77
Copy link
Member

Thank you @conchurnavid for the perfectly clear explanation. Once again I cannot see beyond my own hands :(.

@richard42 @Narann I suggest reverting #14 immediately. That PR was one of several ways to address the windows build issues discussed in #12. I can submit a new PR to address #12 in a less invasive manner.

Three steps forward, three steps back :P

@Narann
Copy link
Member

Narann commented Jan 14, 2015

Thanks a lot for pointing this @conchurnavid. I've create the reverted PR.

@littleguy77 I would suggest a #ifndef SDL2 (with comments of course :D ) over the SDL_main.h.

There is a point where SDL1.2 support will be dropped but we would lost many platforms that still can run mupen64plus. And it's not just a question of dropping SDL1.2 but also cleanup many things. (What do you think @richard42).

@littleguy77
Copy link
Member

I have no opinion on SDL 1.2. I merely posted #14 as one way to solve windows issues from #12, thinking I might as well cleanup what I thought was an obsolete dependency. I see no reason to surround the #include SDL_main.h with #ifndef SDL2 as it doesn't resolve the issues from #12 for windows. Might as well keep the SDL behavior the same regardless of SDL version.

@Narann
Copy link
Member

Narann commented Jan 14, 2015

I see no reason to surround the #include SDL_main.h with #ifndef SDL2 as it doesn't resolve the issues from #12 for windows.

Ok, I was assuming it was a potential fix. :(

@richard42
Copy link
Member

Yeah I mentioned the original reason for the SDL_main linkage in PR #12 . As Narann mentioned, I would like to retain SDL 1.2 compatibility for now in order to support older platforms.

On another note, I've been working with Gonetz to get his new OpenGL 3.3 video plugin working in OSX, and in the course of this work I discovered that our video mode setting code in vidext.c is wrong for SDL2. If I get time this weekend I will rewrite this to use the correct (new) functions. It will be a rather low-level change, so I'm a little concerned about causing regressions.

@Narann
Copy link
Member

Narann commented Jan 16, 2015

Thanks @richard42.

So, we don't drop SDL1.2 for now and will wait a future PR from @littleguy77 adressing this problem.

I've been working with Gonetz to get his new OpenGL 3.3 video plugin working in OSX

Excellent news! I didn't see @gonetz code yet, I hope it's clean enough for future contribution. :)

About possible regression. If it's just few lines to change in every video plugin it wouldn't be a real problem right?

@littleguy77
Copy link
Member

Thanks for the reminder @Narann. Will submit a new PR tonight.

@richard42
Copy link
Member

The SDL2 change that I plan to make this weekend shouldn't require any changes to the video plugins (this is one nice thing about the video extension architecture). Mostly I'm concerned that it either: 1. won't work at all on some platforms, or 2. fullscreen/window mode switching won't work on some platforms. Of course, fullscreen switching already doesn't work on Windows due to a platform limitation.

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

No branches or pull requests

3 participants