Merge lp:~widelands-dev/widelands/bug1098263 into lp:widelands

[Jens Beyer]

Ok, I tried to fix this.

The reason I had to split the init function is that the GL_EXTENSIONS seems to be available only late in the game (it looks like glewInit() does provide it).

Problem now is that it looks like we are leaking a sdlsurface instance there (check the TODO), or not? I am not sure.
And I am not sure if I found all possible paths in the code.

Please review thoroughly ;-)

[SirVer (sirver)]

Thanks for looking into this. I spent too much time on Widelands already today, so I'll postpone this review till next week.

And another comment: could you push your branches to ~widelands-dev in the future? that way others can directly edit your branch which makes talking about nits unnecessary.

[Jens Beyer (qcumber-some)]

I moved this branch to ~widelands-dev and will do so in the future :-)

[SirVer (sirver)]

I reviewed this in r6603. I changed some nits (only formatting really) and changed the TODO to an explanation. I also deleted some dead code that could never be reached - can you look at my changes if they look fine to you and if they are go ahead and merge this in?

review: Approve

=== modified file 'src/graphic/graphic.cc'
--- src/graphic/graphic.cc	2013-07-13 14:32:49 +0000
+++ src/graphic/graphic.cc	2013-07-14 11:00:38 +0000
@@ -140,6 +140,13 @@
 	if (0 != (sdlsurface->flags & SDL_FULLSCREEN))
 		log("Graphics: FULLSCREEN ENABLED\n");
 
+<<<<<<< TREE
+=======
+#ifdef USE_OPENGL
+	bool use_arb = true;
+	const char * extensions;
+
+>>>>>>> MERGE-SOURCE
 	if (0 != (sdlsurface->flags & SDL_OPENGL)) {
 		//  We have successful opened an opengl screen. Print some information
 		//  about opengl and set the rendering capabilities.
@@ -151,9 +158,36 @@
 			throw wexception("glewInit returns %i: Broken OpenGL installation.", err);
 		}
 
+		extensions = reinterpret_cast<const char *>(glGetString (GL_EXTENSIONS));
+
+		if (strstr(extensions, "GL_ARB_framebuffer_object") != 0) {
+			use_arb = true;
+		} else if (strstr(extensions, "GL_EXT_framebuffer_object") != 0) {
+			use_arb = false;
+		} else {
+			log
+			("Graphics: Neither GL_ARB_framebuffer_object or GL_EXT_framebuffer_object supported! "
+			"Switching off OpenGL!\n"
+			);
+			flags &= ~SDL_OPENGL;
+			m_fallback_settings_in_effect = true;
+
+			// One must never free the screen surface of SDL (using
+			// SDL_FreeSurface) as it is owned by SDL itself, therefore the next
+			// call does not leak memory.
+			sdlsurface = SDL_SetVideoMode
+				(FALLBACK_GRAPHICS_WIDTH, FALLBACK_GRAPHICS_HEIGHT, FALLBACK_GRAPHICS_DEPTH, flags);
+			m_fallback_settings_in_effect = true;
+			if (!sdlsurface)
+				throw wexception("Graphics: could not set video mode: %s", SDL_GetError());
+		}
+	}
+
+	// Redoing the check, because fallback settings might mean we no longer use OpenGL.
+	if (0 != (sdlsurface->flags & SDL_OPENGL)) {
+		//  We now really have a working opengl screen...
 		g_opengl = true;
 
-
 		GLboolean glBool;
 		glGetBooleanv(GL_DOUBLEBUFFER, &glBool);
 		log
@@ -184,8 +218,7 @@
 			("Graphics: OpenGL: Version %d.%d \"%s\"\n",
 			 m_caps.gl.major_version, m_caps.gl.minor_version, str);
 
-		const char * extensions = reinterpret_cast<const char *>(glGetString (GL_EXTENSIONS));
-
+		// extensions will be valid if we ever succeeded in runnning glewInit.
 		m_caps.gl.tex_power_of_two =
 			(m_caps.gl.major_version < 2) and
 			(strstr(extensions, "GL_ARB_texture_non_power_of_two") == 0);
@@ -276,7 +309,7 @@
 		SDL_GL_SwapBuffers();
 		glEnable(GL_TEXTURE_2D);
 
-		GLSurfaceTexture::Initialize();
+		GLSurfaceTexture::Initialize(use_arb);
 
 	}
 

=== modified file 'src/graphic/render/gl_surface_texture.cc'
--- src/graphic/render/gl_surface_texture.cc	2013-02-10 18:47:18 +0000
+++ src/graphic/render/gl_surface_texture.cc	2013-07-14 11:00:38 +0000
@@ -30,28 +30,17 @@
 /**
  * Initial global resources needed for fast offscreen rendering.
  */
-void GLSurfaceTexture::Initialize() {
-	const char * extensions = reinterpret_cast<const char *>(glGetString (GL_EXTENSIONS));
+void GLSurfaceTexture::Initialize(bool use_arb) {
+	use_arb_ = use_arb;
 
 	// Generate the framebuffer for Offscreen rendering.
-	bool fbs = false;
-	if (strstr(extensions, "GL_ARB_framebuffer_object") != 0) {
-		fbs = true;
-		use_arb_ = true;
+	if (use_arb) {
 		glGenFramebuffers(1, &gl_framebuffer_id_);
 		glBindFramebuffer(GL_FRAMEBUFFER, 0);
-	} else if (strstr(extensions, "GL_EXT_framebuffer_object") != 0) {
-		fbs = true;
-		use_arb_ = false;
+	} else {
 		glGenFramebuffersEXT(1, &gl_framebuffer_id_);
 		glBindFramebufferEXT(GL_FRAMEBUFFER, 0);
 	}
-	if (!fbs) {
-		throw wexception
-			("No support for GL_ARB_framebuffer_object or GL_ARB_framebuffer_object "
-			 "in OpenGL implementation. One of these is needed for Widelands in OpenGL mode. You can "
-			 "try launching Widelands with --opengl=0.");
-	}
 }
 
 /**

=== modified file 'src/graphic/render/gl_surface_texture.h'
--- src/graphic/render/gl_surface_texture.h	2013-02-10 17:39:24 +0000
+++ src/graphic/render/gl_surface_texture.h	2013-07-14 11:00:38 +0000
@@ -27,7 +27,7 @@
 public:
 	// Call this once before using any instance of this class and Cleanup once
 	// before the program exits.
-	static void Initialize();
+	static void Initialize(bool use_arb);
 	static void Cleanup();
 
 	GLSurfaceTexture(SDL_Surface * surface, bool intensity = false);