Merge lp:~peter.waller/widelands/toggle-fullscreen-rewrite into lp:widelands

[Peter Waller]

Hi. I'm _certain_ there are things wrong with this, but it appears to work and not leak memory, so I thought I would propose merging to have a review.

I was expecting there to be problems due to some relationship between Pictures and the m_sdl_screen, but I haven't seen them.

Tested: switching hundreds of times repeatedly (removing the keydown check so that the 'f' key toggles as fast as possible), no visible memory increase.

[SirVer (sirver)]

Unfortunately, this no longer works. I feel this merge request somehow slipped by and noone did properly look at it. I am very sorry for this Peter!

Setting to rejected for now.

[Peter Waller (peter.waller)]

Hey, no worries sirver.

Regarding "the right way to do it", I think it would be good if whilst in full screen we don't break alt-tab, which is currently the case. It just means if you happy to be in the multiplayer lobby waiting for someone to connect to you (say, you're hosting), you can't do anything else with your computer.. which might be irritating if the person trying to connect has a problem connecting and is trying to communicate it to you.

If it is the case that alt-tab is still broken, I will make a bug and investigate fixing that instead at some point.

[SirVer (sirver)]

I am not sure if widelands does this or if this is an SDL issue.

However, the graphic system should now be able to handle Reintialize() in the fullscreen handler given there are no InMemoryImages constructed (which should be in most cases). So you can try and go for that too and make the implementation proper.

Unmerged revisions

6358. By Peter Waller on 2012-04-25

Experimental: Remove use of SDL_WM_ToggleFullScreen

=== modified file 'src/graphic/graphic.cc'
--- src/graphic/graphic.cc	2012-03-09 16:57:20 +0000
+++ src/graphic/graphic.cc	2012-04-25 20:10:39 +0000
@@ -62,9 +62,6 @@
 uint32_t luminance_table_g[0x100];
 uint32_t luminance_table_b[0x100];
 
-/**
- * Initialize the SDL video mode.
-*/
 Graphic::Graphic
 	(int32_t const w, int32_t const h,
 	 int32_t const bpp,
@@ -76,6 +73,30 @@
 	m_update_fullscreen(false),
 	m_roadtextures     (0)
 {
+    Initialize(w, h, bpp, fullscreen, opengl);
+}
+
+/**
+ * Free the surface
+*/
+Graphic::~Graphic()
+{
+	delete m_rendertarget;
+	delete m_roadtextures;
+
+	// Remove traces of cached pictures
+	UI::g_fh->flush_cache();
+}
+
+/**
+ * Initialize the SDL video mode.
+*/
+void Graphic::Initialize
+    (int32_t const w, int32_t const h,
+	 int32_t const bpp,
+	 bool    const fullscreen,
+	 bool    const opengl)
+{
 	// Initialize the table used to create grayed pictures
 	for
 		(uint32_t i = 0, r = 0, g = 0, b = 0;
@@ -312,18 +333,6 @@
 }
 
 /**
- * Free the surface
-*/
-Graphic::~Graphic()
-{
-	delete m_rendertarget;
-	delete m_roadtextures;
-
-	// Remove traces of cached pictures
-	UI::g_fh->flush_cache();
-}
-
-/**
  * Return the screen x resolution
 */
 int32_t Graphic::get_xres() const
@@ -354,9 +363,11 @@
 */
 void Graphic::toggle_fullscreen()
 {
-	log("Try DL_WM_ToggleFullScreen...\n");
-	//ToDo Make this work again
-	SDL_WM_ToggleFullScreen(m_sdl_screen);
+	log("Graphic::toggle_fullscreen trying to reinitialize graphics");
+	bool fullscreen = !(m_sdl_screen->flags & SDL_FULLSCREEN); // Toggle current state
+	bool opengl = m_sdl_screen->flags & SDL_OPENGL;
+	
+	Initialize(get_xres(), get_yres(), m_sdl_screen->format->BitsPerPixel, fullscreen, opengl);
 }
 
 /**

=== modified file 'src/graphic/graphic.h'
--- src/graphic/graphic.h	2012-02-21 13:42:13 +0000
+++ src/graphic/graphic.h	2012-04-25 20:10:39 +0000
@@ -121,6 +121,10 @@
 		(int32_t w, int32_t h, int32_t bpp,
 		 bool fullscreen, bool opengl);
 	~Graphic();
+	
+    void Initialize
+		(int32_t w, int32_t h, int32_t bpp,
+		 bool fullscreen, bool opengl);
 
 	int32_t get_xres() const;
 	int32_t get_yres() const;