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

[SirVer]

- Fix big memory leak and extra work in UI::Table::draw.
- Fix a couple of minor memory leaks around the code I found using the Leaks tool in Apple's Instruments.
- Correctly crop destination and source rectangle while blitting.

Major leak explanation:
On each frame we created a downscaled texture if the image of a table entry was too small and leaked this texture immediately. This extra scaling work we did there was even unnecessary since OpenGL can scale down images while blitting for free.

Deleted that rescaling code, which makes the texture creation unnecessary, gets rid of the memory leak and also buys CPU cycles since a lot of work is now never done.

[bunnybot (widelandsofficial)]

Hi, I am bunnybot (https://github.com/widelands/bunnybot).

I am keeping the source branch lp:~widelands-dev/widelands/memory_leak mirrored to https://github.com/widelands/widelands/tree/_widelands_dev_widelands_memory_leak

You can give me commands by starting a line with @bunnybot <command>. I understand:
 merge: Merges the source branch into the target branch, closing the merge proposal. I will use the proposed commit message if it is set.

[GunChleoc (gunchleoc)]

Have you tested that this doesn't regress https://bugs.launchpad.net/widelands/+bug/1522564?

This bug was the reason I stopped using blitrect_scale.

[bunnybot (widelandsofficial)]

Travis build 343 has changed state to: errored. Details: https://travis-ci.org/widelands/widelands/builds/104189929.

[SirVer (sirver)]

> Have you tested that this doesn't regress https://bugs.launchpad.net/widelands/+bug/1522564?

No, I did not. It very likely regresses that as blitrect_scale does not change the source rect when cropping. I'll look into that, but I'd like to get https://code.launchpad.net/~widelands-dev/widelands/fix_editor_transparency_issue/+merge/282874 in before that, since it touches on the same code and I do not want to deal with the conflicts.

[bunnybot (widelandsofficial)]

Travis build 343 has changed state to: passed. Details: https://travis-ci.org/widelands/widelands/builds/104189929.

[kaputtnik (franku)]

> Have you tested that this doesn't regress https://bugs.launchpad.net/widelands/+bug/1522564?

This branch regresses that bug. It looks likes the images are in an additional row. I attached a screenshot to the bug, look : https://bugs.launchpad.net/widelands/+bug/1522564/+attachment/4555267/+files/overlap.png

I am in hurry, want to test further this afternoon.

[GunChleoc (gunchleoc)]

The problem is that blitrect_scale doesn't crop.

[kaputtnik (franku)]

I get increasing memory for about 30-100k per second. Opening the message window does not increase the memory usage anymore.

All looks good, except the overlapping images in the message menu.

Do not approve, because bug 1522564 is linked here.

[SirVer (sirver)]

> I get increasing memory for about 30-100k per second.

Does this ever stop or grow boundless?

I just pushed a commit that should fix bug 1522564 too. This is ready for code review and another round of testing.

[kaputtnik (franku)]

It increases boundless. I believe this caused by the wares which are produced... but i am not sure.

Today i have no time anymore for testing.

[Tino (tino79)]

I do not experience any more memory leaking on windows.
And no overlapping icons in the message menu.

review: Approve

[bunnybot (widelandsofficial)]

Travis build 354 has changed state to: failed. Details: https://travis-ci.org/widelands/widelands/builds/104294390.

[GunChleoc (gunchleoc)]

Graphics now look OK on Ubuntu. Code LGTM.

Travis fail is because of Codecheck - I fixed all but 1 Codecheck warning in https://code.launchpad.net/~widelands-dev/widelands/codecheck_compiler_fixes/+merge/283722.

review: Approve

[kaputtnik (franku)]

To fiddle out why the memory increases will take some time. So we could merge this and i open a new bug if i found the something.

[SirVer (sirver)]

Widelands is currently not leaking memory during normal gameplay. I checked that yesterday. So the increases you see are probably just due to map objects being created.

I'd still like a code review if possible.

> Am 24.01.2016 um 11:27 schrieb kaputtnik <email address hidden>:
>
> To fiddle out why the memory increases will take some time. So we could merge this and i open a new bug if i found the something.
>
>
> --
> https://code.launchpad.net/~widelands-dev/widelands/memory_leak/+merge/283690
> You proposed lp:~widelands-dev/widelands/memory_leak for merging.

[SirVer (sirver)]

Oh, gun already reviewed the code. did not see that.

@bunnybot merge

[kaputtnik (franku)]

> So the increases you see are probably just due to map objects being created.

Yes that is probably be the reason.

=== modified file 'src/graphic/rendertarget.cc'
--- src/graphic/rendertarget.cc	2016-01-17 19:54:32 +0000
+++ src/graphic/rendertarget.cc	2016-01-24 08:40:10 +0000
@@ -154,17 +154,13 @@
 void RenderTarget::blit(const Point& dst, const Image* image, BlendMode blend_mode, UI::Align align)
 {
 	Point destination_point(dst);
-
 	UI::correct_for_align(align, image->width(), image->height(), &destination_point);
 
-	Rect srcrc(Point(0, 0), image->width(), image->height());
+	Rect source_rect(Point(0, 0), image->width(), image->height());
+	Rect destination_rect(destination_point.x, destination_point.y, source_rect.w, source_rect.h);
 
-	if (to_surface_geometry(&destination_point, &srcrc)) {
-		m_surface->blit(Rect(destination_point.x, destination_point.y, srcrc.w, srcrc.h),
-		                *image,
-		                srcrc,
-		                1.,
-		                blend_mode);
+	if (to_surface_geometry(&destination_rect, &source_rect)) {
+		m_surface->blit(destination_rect, *image, source_rect, 1., blend_mode);
 	}
 }
 
@@ -172,14 +168,13 @@
 									  const Image* image,
 									  const RGBAColor& blend_mode, UI::Align align) {
 	Point destination_point(dst);
-
 	UI::correct_for_align(align, image->width(), image->height(), &destination_point);
 
-	Rect srcrc(Point(0, 0), image->width(), image->height());
+	Rect source_rect(Point(0, 0), image->width(), image->height());
+	Rect destination_rect(destination_point.x, destination_point.y, source_rect.w, source_rect.h);
 
-	if (to_surface_geometry(&destination_point, &srcrc)) {
-		m_surface->blit_monochrome(Rect(destination_point.x, destination_point.y, srcrc.w, srcrc.h),
-		                           *image, srcrc, blend_mode);
+	if (to_surface_geometry(&destination_rect, &source_rect)) {
+		m_surface->blit_monochrome(destination_rect, *image, source_rect, blend_mode);
 	}
 }
 
@@ -194,49 +189,33 @@
 
 	// We want to use the given srcrc, but we must make sure that we are not
 	// blitting outside of the boundaries of 'image'.
-	Rect srcrc(gsrcrc.x,
+	Rect source_rect(gsrcrc.x,
 	           gsrcrc.y,
 	           std::min<int32_t>(image->width() - gsrcrc.x, gsrcrc.w),
 	           std::min<int32_t>(image->height() - gsrcrc.y, gsrcrc.h));
+	Rect destination_rect(dst.x, dst.y, source_rect.w, source_rect.h);
 
-	Point destination_point(dst);
-	if (to_surface_geometry(&destination_point, &srcrc))
-		m_surface->blit(Rect(destination_point.x, destination_point.y, srcrc.w, srcrc.h),
-		         *image,
-		         srcrc,
-		         1.,
-		         blend_mode);
+	if (to_surface_geometry(&destination_rect, &source_rect)) {
+		m_surface->blit(destination_rect, *image, source_rect, 1., blend_mode);
+	}
 }
 
-void RenderTarget::blitrect_scale(const Rect& dst,
+void RenderTarget::blitrect_scale(Rect destination_rect,
                                   const Image* image,
-                                  const Rect& source_rect,
-											 const float opacity,
+                                  Rect source_rect,
+                                  const float opacity,
                                   const BlendMode blend_mode) {
-
-	Point destination_point(dst.x, dst.y);
-	Rect srcrect(source_rect);
-	if (to_surface_geometry(&destination_point, &srcrect)) {
-		m_surface->blit(Rect(destination_point.x, destination_point.y, dst.w, dst.h),
-		                *image,
-		                source_rect,
-		                opacity,
-		                blend_mode);
+	if (to_surface_geometry(&destination_rect, &source_rect)) {
+		m_surface->blit(destination_rect, *image, source_rect, opacity, blend_mode);
 	}
 }
 
-void RenderTarget::blitrect_scale_monochrome(const Rect& destination_rect,
+void RenderTarget::blitrect_scale_monochrome(Rect destination_rect,
                                        const Image* image,
-                                       const Rect& source_rect,
+                                       Rect source_rect,
 													const RGBAColor& blend) {
-	Point destination_point(destination_rect.x, destination_rect.y);
-	Rect srcrect(source_rect);
-	if (to_surface_geometry(&destination_point, &srcrect)) {
-		m_surface->blit_monochrome(
-		   Rect(destination_point.x, destination_point.y, destination_rect.w, destination_rect.h),
-		   *image,
-		   source_rect,
-		   blend);
+	if (to_surface_geometry(&destination_rect, &source_rect)) {
+		m_surface->blit_monochrome(destination_rect, *image, source_rect, blend);
 	}
 }
 
@@ -347,12 +326,13 @@
                                      uint32_t time,
                                      const RGBColor* player_color,
                                      const Rect& source_rect) {
-	Point destination_point = dst - animation.hotspot();
-	destination_point += source_rect.origin();
+	Rect destination_rect(dst.x - animation.hotspot().x + source_rect.x,
+	                      dst.y - animation.hotspot().y + source_rect.y, source_rect.w,
+	                      source_rect.h);
 
 	Rect srcrc(source_rect);
-	if (to_surface_geometry(&destination_point, &srcrc))
-		animation.blit(time, destination_point, srcrc, player_color, m_surface);
+	if (to_surface_geometry(&destination_rect, &srcrc))
+		animation.blit(time, destination_rect.origin(), srcrc, player_color, m_surface);
 
 	// Look if there is a sound effect registered for this frame and trigger the
 	// effect (see SoundHandler::stereo_position).
@@ -423,41 +403,71 @@
  * Clip against window and source bitmap, returns false if blitting is
  * unnecessary because image is not inside the target surface.
  */
-bool RenderTarget::to_surface_geometry(Point* dst, Rect* srcrc) const
+bool RenderTarget::to_surface_geometry(Rect* destination_rect, Rect* source_rect) const
 {
-	assert(0 <= srcrc->x);
-	assert(0 <= srcrc->y);
-	*dst += m_offset;
-
-	// Clipping
-	if (dst->x < 0) {
-		if (srcrc->w <= -dst->x)
-			return false;
-		srcrc->x -= dst->x;
-		srcrc->w += dst->x;
-		dst->x = 0;
-	}
-
-	if (dst->x + srcrc->w > m_rect.w) {
-		if (m_rect.w <= dst->x)
-			return false;
-		srcrc->w = m_rect.w - dst->x;
-	}
-
-	if (dst->y < 0) {
-		if (srcrc->h <= -dst->y)
-			return false;
-		srcrc->y -= dst->y;
-		srcrc->h += dst->y;
-		dst->y = 0;
-	}
-
-	if (dst->y + srcrc->h > m_rect.h) {
-		if (m_rect.h <= dst->y)
-			return false;
-		srcrc->h = m_rect.h - dst->y;
-	}
-
-	*dst += m_rect.origin();
+	assert(0 <= source_rect->x);
+	assert(0 <= source_rect->y);
+	destination_rect->x += m_offset.x;
+	destination_rect->y += m_offset.y;
+
+	// We have to clip the target rect against our own drawing area. If we make
+	// changes to any side of our rectangle, we have to change the source rect
+	// too. But since the source_rectangle might have a different size than the
+	// destination_rect, we do this by making the proportional change.
+
+	// Clipping, from the left.
+	if (destination_rect->x < 0) {
+		if (destination_rect->w <= -destination_rect->x) {
+			return false;
+		}
+		// Adding 0.5 is a cheap way of turning integer truncation into a rounded value.
+		const int source_rect_pixel_change =
+		   0.5 + -static_cast<double>(destination_rect->x) / destination_rect->w * source_rect->w;
+		source_rect->x += source_rect_pixel_change;
+		source_rect->w -= source_rect_pixel_change;
+		destination_rect->w += destination_rect->x;
+		destination_rect->x = 0;
+	}
+
+	// Clipping, from the right.
+	if (destination_rect->x + destination_rect->w > m_rect.w) {
+		if (m_rect.w <= destination_rect->x) {
+			return false;
+		}
+		const int new_destination_w = m_rect.w - destination_rect->x;
+		// Adding 0.5 is a cheap way of turning integer truncation into a rounded value.
+		source_rect->w =
+		   0.5 + static_cast<double>(new_destination_w) / destination_rect->w * source_rect->w;
+		destination_rect->w = new_destination_w;
+	}
+
+	// Clipping, from the top.
+	if (destination_rect->y < 0) {
+		if (destination_rect->h <= -destination_rect->y) {
+			return false;
+		}
+		// Adding 0.5 is a cheap way of turning integer truncation into a rounded value.
+		const int source_rect_pixel_change = 0.5 +
+		   -static_cast<double>(destination_rect->y) / destination_rect->h * source_rect->h;
+		source_rect->y += source_rect_pixel_change;
+		source_rect->h -= source_rect_pixel_change;
+		destination_rect->h += destination_rect->y;
+		destination_rect->y = 0;
+	}
+
+	// Clipping, from the bottom.
+	if (destination_rect->y + destination_rect->h > m_rect.h) {
+		if (m_rect.h <= destination_rect->y) {
+			return false;
+		}
+		const int new_destination_h = m_rect.h - destination_rect->y;
+		// Adding 0.5 is a cheap way of turning integer truncation into a rounded value.
+		source_rect->h =
+		   0.5 + static_cast<double>(new_destination_h) / destination_rect->h * source_rect->h;
+		destination_rect->h = new_destination_h;
+	}
+
+	destination_rect->x += m_rect.x;
+	destination_rect->y += m_rect.y;
 	return true;
 }

=== modified file 'src/graphic/rendertarget.h'
--- src/graphic/rendertarget.h	2016-01-23 10:08:40 +0000
+++ src/graphic/rendertarget.h	2016-01-24 08:40:10 +0000
@@ -83,16 +83,18 @@
 	// multiplied with 'opacity' before blitting. The 'blend_mode'
 	// defines if values are blended with whats already there or just
 	// copied over.
-	void blitrect_scale(const Rect& destination_rect,
+	// Rect's are taken by value on purpose.
+	void blitrect_scale(Rect destination_rect,
 	                    const Image* image,
-	                    const Rect& source_rect,
+	                    Rect source_rect,
 	                    float opacity,
 	                    BlendMode blend_mode);
 
-	// Like blitrect_scale. See MonochromeBlitProgram for details.
-	void blitrect_scale_monochrome(const Rect& destination_rect,
+	// Like blitrect_scale. See MonochromeBlitProgram for details. Rect's are
+	// taken by value on purpose.
+	void blitrect_scale_monochrome(Rect destination_rect,
 	                               const Image* image,
-	                               const Rect& source_rect,
+	                               Rect source_rect,
 	                               const RGBAColor& blend);
 
 	void tile(const Rect&,
@@ -118,7 +120,7 @@
 
 protected:
 	bool clip(Rect & r) const;
-	bool to_surface_geometry(Point* dst, Rect* srcrc) const;
+	bool to_surface_geometry(Rect* destination_rect, Rect* source_rect) const;
 
 	// Does the actual blitting.
 	void do_blit_animation(const Point& dst,

=== modified file 'src/graphic/text/font_io.cc'
--- src/graphic/text/font_io.cc	2014-11-18 17:45:58 +0000
+++ src/graphic/text/font_io.cc	2016-01-24 08:40:10 +0000
@@ -40,7 +40,7 @@
 	// this class is destroyed.
 	std::unique_ptr<std::string> memory;
 	{
-		FileRead* fr = new FileRead();
+		std::unique_ptr<FileRead> fr(new FileRead());
 		fr->open(*g_fs, filename);
 		memory.reset(new std::string(fr->data(0), fr->get_size()));
 	}

=== modified file 'src/logic/map_objects/world/world.cc'
--- src/logic/map_objects/world/world.cc	2016-01-13 07:27:55 +0000
+++ src/logic/map_objects/world/world.cc	2016-01-24 08:40:10 +0000
@@ -50,15 +50,15 @@
 	for (size_t i = 0; i < terrains_->size(); ++i) {
 		TerrainDescription* terrain = terrains_->get_mutable(i);
 		for (size_t j = 0; j < terrain->texture_paths().size(); ++j) {
-			SDL_Surface* sdl_surface = load_image_as_sdl_surface(terrain->texture_paths()[j]);
-
 			// Set the minimap color on the first loaded image.
 			if (j == 0) {
+				SDL_Surface* sdl_surface = load_image_as_sdl_surface(terrain->texture_paths()[j]);
 				uint8_t top_left_pixel = static_cast<uint8_t*>(sdl_surface->pixels)[0];
 				const SDL_Color top_left_pixel_color =
 				   sdl_surface->format->palette->colors[top_left_pixel];
 				terrain->set_minimap_color(
 				   RGBColor(top_left_pixel_color.r, top_left_pixel_color.g, top_left_pixel_color.b));
+				SDL_FreeSurface(sdl_surface);
 			}
 			terrain->add_texture(g_gr->images().get(terrain->texture_paths()[j]));
 		}

=== modified file 'src/ui_basic/table.cc'
--- src/ui_basic/table.cc	2016-01-11 15:18:51 +0000
+++ src/ui_basic/table.cc	2016-01-24 08:40:10 +0000
@@ -283,7 +283,7 @@
 		Columns::size_type const nr_columns = m_columns.size();
 		for (uint32_t i = 0, curx = 0; i < nr_columns; ++i) {
 			const Column& column = m_columns[i];
-			int const curw  = column.width;
+			int const curw = column.width;
 			Align alignment = mirror_alignment(column.alignment);
 
 			const Image* entry_picture = er.get_picture(i);
@@ -291,11 +291,10 @@
 
 			Point point(curx, y);
 			int picw = 0;
-			int pich = 0;
 
-			if (entry_picture) {
+			if (entry_picture != nullptr) {
 				picw = entry_picture->width();
-				pich = entry_picture->height();
+				const int pich = entry_picture->height();
 
 				int draw_x = point.x;
 
@@ -319,22 +318,9 @@
 						draw_x += curw - blit_width;
 					}
 
-					// Temporary texture for the scaled image
-					Texture* scaled_texture = new Texture(blit_width, max_pic_height);
-
-					// Initialize the rectangle
-					scaled_texture->fill_rect(Rect(0, 0, blit_width, max_pic_height),
-									RGBAColor(255, 255, 255, 0));
-
 					// Create the scaled image
-					scaled_texture->blit(Rect(0, 0, blit_width, max_pic_height),
-							 *entry_picture,
-							 Rect(0, 0, picw, pich),
-							 1.,
-							 BlendMode::UseAlpha);
-
-					// This will now blit with any appropriate cropping
-					dst.blit(Point(draw_x, point.y + 1), scaled_texture);
+					dst.blitrect_scale(Rect(draw_x, point.y + 1, blit_width, max_pic_height),
+					                   entry_picture, Rect(0, 0, picw, pich), 1., BlendMode::UseAlpha);
 
 					// For text alignment below
 					picw = blit_width;

=== modified file 'src/ui_fsmenu/loadgame.cc'
--- src/ui_fsmenu/loadgame.cc	2016-01-17 08:29:59 +0000
+++ src/ui_fsmenu/loadgame.cc	2016-01-24 08:40:10 +0000
@@ -435,16 +435,16 @@
 	m_games_data.clear();
 	m_table.clear();
 
-	SavegameData* gamedata = new SavegameData();
 
 	if (m_settings && !m_settings->settings().saved_games.empty()) {
+		SavegameData gamedata;
 		for (uint32_t i = 0; i < m_settings->settings().saved_games.size(); ++i) {
-			gamedata->filename = m_settings->settings().saved_games.at(i).path;
-			m_games_data.push_back(*gamedata);
+			gamedata.filename = m_settings->settings().saved_games.at(i).path;
+			m_games_data.push_back(gamedata);
 
 			UI::Table<uintptr_t const>::EntryRecord & te =
 				m_table.add(m_games_data.size() - 1);
-			te.set_string(0, FileSystem::filename_without_ext(gamedata->filename.c_str()).c_str());
+			te.set_string(0, FileSystem::filename_without_ext(gamedata.filename.c_str()).c_str());
 		}
 	} else { // Normal case
 		// Fill it with all files we find.
@@ -465,7 +465,7 @@
 				continue;
 			}
 
-			gamedata = new SavegameData();
+			SavegameData gamedata;
 
 			std::string savename = gamefilename;
 			if (m_is_replay) savename += WLGF_SUFFIX;
@@ -474,30 +474,30 @@
 				continue;
 			}
 
-			gamedata->filename = gamefilename;
+			gamedata.filename = gamefilename;
 
 			try {
 				Widelands::GameLoader gl(savename.c_str(), m_game);
 				gl.preload_game(gpdp);
 
-				gamedata->gametype = gpdp.get_gametype();
+				gamedata.gametype = gpdp.get_gametype();
 
 				if (!m_is_replay) {
 					if (m_settings->settings().multiplayer) {
-						if (gamedata->gametype == GameController::GameType::SINGLEPLAYER) {
+						if (gamedata.gametype == GameController::GameType::SINGLEPLAYER) {
 							continue;
 						}
-					} else if (gamedata->gametype > GameController::GameType::SINGLEPLAYER) {
+					} else if (gamedata.gametype > GameController::GameType::SINGLEPLAYER) {
 						continue;
 					}
 				}
 
-				gamedata->mapname = gpdp.get_mapname();
-				gamedata->gametime = gpdp.get_gametime();
-				gamedata->nrplayers = gpdp.get_number_of_players();
-				gamedata->version = gpdp.get_version();
+				gamedata.mapname = gpdp.get_mapname();
+				gamedata.gametime = gpdp.get_gametime();
+				gamedata.nrplayers = gpdp.get_number_of_players();
+				gamedata.version = gpdp.get_version();
 
-				gamedata->savetimestamp = gpdp.get_savetimestamp();
+				gamedata.savetimestamp = gpdp.get_savetimestamp();
 				time_t t;
 				time(&t);
 				struct tm * currenttime  = localtime(&t);
@@ -506,9 +506,9 @@
 				int8_t current_month = currenttime->tm_mon;
 				int8_t current_day = currenttime->tm_mday;
 
-				struct tm * savedate  = localtime(&gamedata->savetimestamp);
+				struct tm * savedate  = localtime(&gamedata.savetimestamp);
 
-				if (gamedata->savetimestamp > 0) {
+				if (gamedata.savetimestamp > 0) {
 					if (savedate->tm_year == current_year &&
 						 savedate->tm_mon == current_month &&
 						 savedate->tm_mday == current_day) {  // Today
@@ -518,7 +518,7 @@
 
 						/** TRANSLATORS: Display date for choosing a savegame/replay */
 						/** TRANSLATORS: hour:minute */
-						gamedata->savedatestring = (boost::format(_("Today, %1%:%2%"))
+						gamedata.savedatestring = (boost::format(_("Today, %1%:%2%"))
 															 % savedate->tm_hour % minute).str();
 					} else if ((savedate->tm_year == current_year &&
 									savedate->tm_mon == current_month &&
@@ -531,13 +531,13 @@
 
 						/** TRANSLATORS: Display date for choosing a savegame/replay */
 						/** TRANSLATORS: hour:minute */
-						gamedata->savedatestring = (boost::format(_("Yesterday, %1%:%2%"))
+						gamedata.savedatestring = (boost::format(_("Yesterday, %1%:%2%"))
 															 % savedate->tm_hour % minute).str();
 					} else {  // Older
 
 						/** TRANSLATORS: Display date for choosing a savegame/replay */
 						/** TRANSLATORS: month day, year */
-						gamedata->savedatestring = (boost::format(_("%2% %1%, %3%"))
+						gamedata.savedatestring = (boost::format(_("%2% %1%, %3%"))
 							 % savedate->tm_mday
 							 % localize_month(savedate->tm_mon)
 							 % (1900 + savedate->tm_year)).str();
@@ -546,18 +546,18 @@
 
 				{
 					i18n::Textdomain td("win_conditions");
-					gamedata->wincondition = _(gpdp.get_win_condition());
+					gamedata.wincondition = _(gpdp.get_win_condition());
 				}
-				gamedata->minimap_path = gpdp.get_minimap_path();
-				m_games_data.push_back(*gamedata);
+				gamedata.minimap_path = gpdp.get_minimap_path();
+				m_games_data.push_back(gamedata);
 
 				UI::Table<uintptr_t const>::EntryRecord & te =
 					m_table.add(m_games_data.size() - 1);
-				te.set_string(0, gamedata->savedatestring);
+				te.set_string(0, gamedata.savedatestring);
 
 				if (m_is_replay || m_settings->settings().multiplayer) {
 					std::string gametypestring;
-					switch (gamedata->gametype) {
+					switch (gamedata.gametype) {
 						case GameController::GameType::SINGLEPLAYER:
 							/** TRANSLATORS: "Single Player" entry in the Game Mode table column. */
 							/** TRANSLATORS: "Keep this to 6 letters maximum. */
@@ -572,7 +572,7 @@
 							/** TRANSLATORS: Make sure that this translation is consistent with the tooltip. */
 							/** TRANSLATORS: %1% is the number of players */
 							gametypestring = (boost::format(_("H (%1%)"))
-												 % static_cast<unsigned int>(gamedata->nrplayers)).str();
+												 % static_cast<unsigned int>(gamedata.nrplayers)).str();
 							break;
 						case GameController::GameType::NETCLIENT:
 							/** TRANSLATORS: "Multiplayer" entry in the Game Mode table column. */
@@ -581,20 +581,20 @@
 							/** TRANSLATORS: Make sure that this translation is consistent with the tooltip. */
 							/** TRANSLATORS: %1% is the number of players */
 							gametypestring = (boost::format(_("MP (%1%)"))
-												% static_cast<unsigned int>(gamedata->nrplayers)).str();
+												% static_cast<unsigned int>(gamedata.nrplayers)).str();
 							break;
 						case GameController::GameType::REPLAY:
 							gametypestring = "";
 							break;
 					}
 					te.set_string(1, gametypestring);
-					te.set_string(2, map_filename(gamedata->filename, gamedata->mapname));
+					te.set_string(2, map_filename(gamedata.filename, gamedata.mapname));
 				} else {
-					te.set_string(1, map_filename(gamedata->filename, gamedata->mapname));
+					te.set_string(1, map_filename(gamedata.filename, gamedata.mapname));
 				}
 			} catch (const WException & e) {
 				//  we simply skip illegal entries
-				gamedata->errormessage =
+				gamedata.errormessage =
 						((boost::format("%s\n\n%s\n\n%s"))
 						 /** TRANSLATORS: Error message introduction for when an old savegame can't be loaded */
 						 % _("This file has the wrong format and can’t be loaded."
@@ -603,9 +603,9 @@
 						 % _("Error message:")
 						 % e.what()).str();
 
-				const std::string fs_filename = FileSystem::filename_without_ext(gamedata->filename.c_str());
-				gamedata->mapname = fs_filename;
-				m_games_data.push_back(*gamedata);
+				const std::string fs_filename = FileSystem::filename_without_ext(gamedata.filename.c_str());
+				gamedata.mapname = fs_filename;
+				m_games_data.push_back(gamedata);
 
 				UI::Table<uintptr_t const>::EntryRecord & te =
 					m_table.add(m_games_data.size() - 1);