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

SDL: really try lower GL profiles & add GLES context support #10423

Merged
merged 1 commit into from
Dec 20, 2017

Conversation

psyke83
Copy link
Contributor

@psyke83 psyke83 commented Dec 20, 2017

  • When compiled with USING_GLES2, attempt to use only valid ES context
    versions.
  • Ensure that lower profiles are attempted correctly rather than
    prematurely returning from the function after the first failure.

Needed for Raspberry Pi to successfully launch.

@psyke83
Copy link
Contributor Author

psyke83 commented Dec 20, 2017

I tried leaving the attemptVersions array untouched, or adding {2, 0}, to the end - ppsspp would launch successfully, but the emulated content would not actually boot, staying on a black screen, but the menu works when pressing ESC.

The Pi should only support ES 2.0 at maximum, and I suppose it makes sense not to try higher versions when building with -DUSING_GLES2...

@unknownbrackets
Copy link
Collaborator

Hmm, USING_GLES2 actually means GLES 2.0+ for us. Does this happen even if it has {3, 2}, {3, 1}, {3, 0}, {2, 0} in it?

Perhaps we should just let it autodetect as before for ES contexts, and put the entire core section in an #ifndef USING_GLES2...

-[Unknown]

@psyke83
Copy link
Contributor Author

psyke83 commented Dec 20, 2017

Ok, I tried changing the array to {3, 2}, {3, 1}, {3, 0}, {2, 0}, and the content still does play correctly. I'll try to ifndef the whole part next, will let you know in just a few minutes.

@psyke83
Copy link
Contributor Author

psyke83 commented Dec 20, 2017

OK, I tried the following tests separately:
1: Isolating the whole for (size_t i = 0; i < ARRAY_SIZE(attemptVersions); ++i) { loop with an #ifndef USING_GLES2 so that it would only attempt the fallback context creation.
2: From this PR, instead of this:

#ifdef USING_GLES2
                SDL_GL_SetAttribute(SDL_GL_CONTEXT_PROFILE_MASK, SDL_GL_CONTEXT_PROFILE_ES);
                SetGLCoreContext(false);
#else
                SDL_GL_SetAttribute(SDL_GL_CONTEXT_PROFILE_MASK, SDL_GL_CONTEXT_PROFILE_CORE);
                SetGLCoreContext(true);
#endif

Change to this:

#ifndef USING_GLES2
                SDL_GL_SetAttribute(SDL_GL_CONTEXT_PROFILE_MASK, SDL_GL_CONTEXT_PROFILE_CORE);
                SetGLCoreContext(true);
#endif

3: Isolating this whole part:

#ifndef USING_GLES2
                // Make sure to request a somewhat modern GL context at least - the
                // latest supported by MacOS X (really, really sad...)
                SDL_GL_SetAttribute(SDL_GL_CONTEXT_MAJOR_VERSION, ver.major);
                SDL_GL_SetAttribute(SDL_GL_CONTEXT_MINOR_VERSION, ver.minor);
                SDL_GL_SetAttribute(SDL_GL_CONTEXT_PROFILE_MASK, SDL_GL_CONTEXT_PROFILE_CORE);
                SetGLCoreContext(true);
#endif

1 & 2 failed, but 3 works. Would you like me to update the PR to match no. 3 (with the fix to iterate correctly for lower versions for non-ES contexts), or should I just update the array to check the latest ES versions?

@psyke83
Copy link
Contributor Author

psyke83 commented Dec 20, 2017

I won't be available to test for a few hours, so I've updated the PR to include the higher ES profile levels and amended the commit message.

I wasn't completely clear on what you meant re the "entire core section", but this change (no. 3 that I was alluding to) also works:

diff --git a/ext/native/base/PCMain.cpp b/ext/native/base/PCMain.cpp
index 128e045d4..f6713ed96 100644
--- a/ext/native/base/PCMain.cpp
+++ b/ext/native/base/PCMain.cpp
@@ -596,19 +596,19 @@ int main(int argc, char *argv[]) {
 	SDL_GLContext glContext = nullptr;
 	for (size_t i = 0; i < ARRAY_SIZE(attemptVersions); ++i) {
 		const auto &ver = attemptVersions[i];
+#ifndef USING_GLES2
 		// Make sure to request a somewhat modern GL context at least - the
 		// latest supported by MacOS X (really, really sad...)
 		SDL_GL_SetAttribute(SDL_GL_CONTEXT_PROFILE_MASK, SDL_GL_CONTEXT_PROFILE_CORE);
 		SDL_GL_SetAttribute(SDL_GL_CONTEXT_MAJOR_VERSION, ver.major);
 		SDL_GL_SetAttribute(SDL_GL_CONTEXT_MINOR_VERSION, ver.minor);
 		SetGLCoreContext(true);
+#endif
 
 		g_Screen = SDL_CreateWindow(app_name_nice.c_str(), x,y, pixel_xres, pixel_yres, mode);
 		if (g_Screen == nullptr) {
-			NativeShutdown();
 			fprintf(stderr, "SDL_CreateWindow failed: %s\n", SDL_GetError());
-			SDL_Quit();
-			return 2;
+			continue;
 		}
 
 		glContext = SDL_GL_CreateContext(g_Screen);

It's simpler than adding the GLES context support, but if context creation fails for any reason, it will retry the exact same process 11 times unnecessarily.

@hrydgard
Copy link
Owner

Oops, you're gonna need to rebase this - I moved PCMain.cpp to the SDL subdirectory and renamed it SDLMain.cpp.

git should take care of the rename and reapplying your changes automatically though.

Copy link
Collaborator

@unknownbrackets unknownbrackets left a comment

Choose a reason for hiding this comment

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

This approach looks good to me.

-[Unknown]

* When compiled with USING_GLES2, attempt to use only valid ES context
  versions.
* Ensure that lower profiles are attempted correctly rather than
  prematurely returning from the function after the first failure.

Needed for Raspberry Pi to successfully launch.
@psyke83 psyke83 changed the title SDL: try GLES2 context & fix core fallback SDL: really try lower GL profiles & add GLES context support Dec 20, 2017
@psyke83
Copy link
Contributor Author

psyke83 commented Dec 20, 2017

Rebased the commit & retested a build against latest master - still working OK. Thanks.

@hrydgard
Copy link
Owner

Seems fine for now. Ideally at some point later we'll transition to not using #ifdef for the GL/GLES distinction.

@hrydgard hrydgard merged commit c27d64f into hrydgard:master Dec 20, 2017
@psyke83 psyke83 deleted the gles2_ctx_fix branch December 21, 2017 17:12
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