Miscellaneous optimizations in display::get_terrain_images()

* The vector of surfaces is now a class member variable instead of a local
variable. This saves a memory allocation every time the function is called
- which is worth it in this case, as the function is a major performance
bottleneck.

* The surfaces are now being moved instead of copied where possible. Turns
out that freeing a SDL surface is fairly expensive in performance-critical
code.

* Pointers to ToDs are now cached, reducing the number of calls to
get_time_of_day() from 37 to 7.

In a stress-test in Aetheryn's Mod at 50 % zoom, the FPS I was getting on
my PC (Intel Core i5-4430) increased from 16 to 23.

changelog

@@ -28,6 +28,8 @@ Version 1.13.10+dev:
  * Performance:
    * GUI2 windows no longer redraw everything 50 times per second. This reduces
      CPU usage in fullscreen windows such as MP lobby by about 85 %.
+   * Miscellaneous low-level optimizations in game rendering code, improving
+     performance ingame by up to 50 %.
  * User Interface:
    * Removed broken Unit Box and Widescreen themes.
    * Fixed a bug that partially prevented movement feedback announce messages

players_changelog

@@ -13,6 +13,8 @@ Version 1.13.10+dev:
  * Performance:
    * GUI2 windows no longer redraw everything 50 times per second. This reduces
      CPU usage in fullscreen windows such as MP lobby by about 85 %.
+   * Miscellaneous low-level optimizations in game rendering code, improving
+     performance ingame by up to 50 %.
  * Miscellaneous and bug fixes:
    * Fixed ingame help showing units you haven't encountered (bug #2135)
 

src/display.cpp

@@ -53,8 +53,10 @@
 
 #include <SDL_image.h>
 
+#include <array>
 #include <cmath>
 #include <iomanip>
+#include <utility>
 
 #ifdef _WIN32
 #include <windows.h>
@@ -988,8 +990,8 @@ std::vector<surface> display::get_fog_shroud_images(const map_location& loc, ima
 {
 	std::vector<std::string> names;
 
-	map_location adjacent[6];
-	get_adjacent_tiles(loc,adjacent);
+	std::array<map_location, 6> adjacent;
+	get_adjacent_tiles(loc, adjacent.data());
 
 	enum visibility {FOG=0, SHROUD=1, CLEAR=2};
 	visibility tiles[6];
@@ -1073,11 +1075,11 @@ std::vector<surface> display::get_fog_shroud_images(const map_location& loc, ima
 	return res;
 }
 
-std::vector<surface> display::get_terrain_images(const map_location &loc,
+const std::vector<surface>& display::get_terrain_images(const map_location &loc,
 	const std::string& timeid,
 	TERRAIN_TYPE terrain_type)
 {
-	std::vector<surface> res;
+	terrain_image_vector_.clear();
 
 	terrain_builder::TERRAIN_TYPE builder_terrain_type =
 	      (terrain_type == FOREGROUND ?
@@ -1091,8 +1093,12 @@ std::vector<surface> display::get_terrain_images(const map_location &loc,
 	const time_of_day& tod = get_time_of_day(loc);
 
 	//get all the light transitions
-	map_location adjs[6];
-	get_adjacent_tiles(loc,adjs);
+	std::array<map_location, 6> adjs;
+	std::array<const time_of_day*, 6> atods;
+	get_adjacent_tiles(loc, adjs.data());
+	for(size_t d = 0; d < adjs.size(); ++d){
+		atods[d] = &get_time_of_day(adjs[d]);
+	}
 
 	for(int d=0; d<6; ++d){
 		/* concave
@@ -1106,8 +1112,8 @@ std::vector<surface> display::get_terrain_images(const map_location &loc,
 		\  tod  /
 		 \_____/*/
 
-		const time_of_day& atod1 = get_time_of_day(adjs[d]);
-		const time_of_day& atod2 = get_time_of_day(adjs[(d + 1) % 6]);
+		const time_of_day& atod1 = *atods[d];
+		const time_of_day& atod2 = *atods[(d + 1) % 6];
 
 		if(atod1.color == tod.color || atod2.color == tod.color || atod1.color != atod2.color)
 			continue;
@@ -1134,8 +1140,8 @@ std::vector<surface> display::get_terrain_images(const map_location &loc,
 		\  tod  /
 		 \_____/*/
 
-		const time_of_day& atod1 = get_time_of_day(adjs[d]);
-		const time_of_day& atod2 = get_time_of_day(adjs[(d + 1) % 6]);
+		const time_of_day& atod1 = *atods[d];
+		const time_of_day& atod2 = *atods[(d + 1) % 6];
 
 		if(atod1.color == tod.color || atod1.color == atod2.color)
 			continue;
@@ -1162,8 +1168,8 @@ std::vector<surface> display::get_terrain_images(const map_location &loc,
 		\  tod  /
 		 \_____/*/
 
-		const time_of_day& atod1 = get_time_of_day(adjs[d]);
-		const time_of_day& atod2 = get_time_of_day(adjs[(d + 1) % 6]);
+		const time_of_day& atod1 = *atods[d];
+		const time_of_day& atod2 = *atods[(d + 1) % 6];
 
 		if(atod2.color == tod.color || atod1.color == atod2.color)
 			continue;
@@ -1212,12 +1218,12 @@ std::vector<surface> display::get_terrain_images(const map_location &loc,
 			}
 
 			if (!surf.null()) {
-				res.push_back(surf);
+				terrain_image_vector_.push_back(std::move(surf));
 			}
 		}
 	}
 
-	return res;
+	return terrain_image_vector_;
 }
 
 void display::drawing_buffer_add(const drawing_layer layer,
@@ -2559,16 +2565,14 @@ void display::draw_hex(const map_location& loc) {
 	int num_images_bg = 0;
 
 	if(!shrouded(loc)) {
-		std::vector<surface> images_fg = get_terrain_images(loc,tod.id, FOREGROUND);
-		std::vector<surface> images_bg = get_terrain_images(loc,tod.id, BACKGROUND);
-
-		num_images_fg = images_fg.size();
+		// unshrouded terrain (the normal case)
+		const std::vector<surface>& images_bg = get_terrain_images(loc, tod.id, BACKGROUND);
+		drawing_buffer_add(LAYER_TERRAIN_BG, loc, xpos, ypos, images_bg);
 		num_images_bg = images_bg.size();
 
-		// unshrouded terrain (the normal case)
-		drawing_buffer_add(LAYER_TERRAIN_BG, loc, xpos, ypos, images_bg);
-
+		const std::vector<surface>& images_fg = get_terrain_images(loc, tod.id, FOREGROUND);
 		drawing_buffer_add(LAYER_TERRAIN_FG, loc, xpos, ypos, images_fg);
+		num_images_fg = images_fg.size();
 
 		// Draw the grid, if that's been enabled
 		if(grid_) {

src/display.hpp

@@ -716,7 +716,8 @@ protected:
 
 	enum TERRAIN_TYPE { BACKGROUND, FOREGROUND};
 
-	std::vector<surface> get_terrain_images(const map_location &loc,
+	// Warning: the returned vector will be invalidated on the next call!
+	const std::vector<surface>& get_terrain_images(const map_location &loc,
 					const std::string& timeid,
 					TERRAIN_TYPE terrain_type);
 
@@ -801,6 +802,10 @@ private:
 	/** Animated flags for each team */
 	std::vector<animated<image::locator> > flags_;
 
+	// This vector is a class member to avoid repeated memory allocations in get_terrain_images(),
+	// which turned out to be a significant bottleneck while profiling.
+	std::vector<surface> terrain_image_vector_;
+
 public:
 	/**
 	 * The layers to render something on. This value should never be stored

src/sdl/surface.hpp

@@ -33,6 +33,11 @@ public:
 		add_surface_ref(surface_);
 	}
 
+	surface(surface&& s) : surface_(s.get())
+	{
+		s.surface_ = nullptr;
+	}
+
 	~surface()
 	{
 		free_surface();
@@ -54,6 +59,14 @@ public:
 		return *this;
 	}
 
+	surface& operator=(surface&& s)
+	{
+		free_surface();
+		surface_ = s.surface_;
+		s.surface_ = nullptr;
+		return *this;
+	}
+
 	// Intended to be used when SDL has already freed the surface
 	void clear_without_free() { surface_ = nullptr; }