From 4d0c46164e1ba3111e3d0bc03cef556c93cfa120 Mon Sep 17 00:00:00 2001 From: Jyrki Vesterinen Date: Sun, 26 Nov 2017 20:07:34 +0200 Subject: [PATCH] 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 | 2 ++ players_changelog | 2 ++ src/display.cpp | 44 ++++++++++++++++++++++++-------------------- src/display.hpp | 7 ++++++- src/sdl/surface.hpp | 13 +++++++++++++ 5 files changed, 47 insertions(+), 21 deletions(-) diff --git a/changelog b/changelog index b71a03335279..2fd4ca183c76 100644 --- a/changelog +++ b/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 diff --git a/players_changelog b/players_changelog index 5e594f24fc37..417f9a2416e6 100644 --- a/players_changelog +++ b/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) diff --git a/src/display.cpp b/src/display.cpp index 4eb69d9d3209..8bc9e01811fc 100644 --- a/src/display.cpp +++ b/src/display.cpp @@ -53,8 +53,10 @@ #include +#include #include #include +#include #ifdef _WIN32 #include @@ -988,8 +990,8 @@ std::vector display::get_fog_shroud_images(const map_location& loc, ima { std::vector names; - map_location adjacent[6]; - get_adjacent_tiles(loc,adjacent); + std::array adjacent; + get_adjacent_tiles(loc, adjacent.data()); enum visibility {FOG=0, SHROUD=1, CLEAR=2}; visibility tiles[6]; @@ -1073,11 +1075,11 @@ std::vector display::get_fog_shroud_images(const map_location& loc, ima return res; } -std::vector display::get_terrain_images(const map_location &loc, +const std::vector& display::get_terrain_images(const map_location &loc, const std::string& timeid, TERRAIN_TYPE terrain_type) { - std::vector res; + terrain_image_vector_.clear(); terrain_builder::TERRAIN_TYPE builder_terrain_type = (terrain_type == FOREGROUND ? @@ -1091,8 +1093,12 @@ std::vector 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 adjs; + std::array 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 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 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 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 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 images_fg = get_terrain_images(loc,tod.id, FOREGROUND); - std::vector images_bg = get_terrain_images(loc,tod.id, BACKGROUND); - - num_images_fg = images_fg.size(); - num_images_bg = images_bg.size(); - // unshrouded terrain (the normal case) + const std::vector& 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(); + const std::vector& 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_) { diff --git a/src/display.hpp b/src/display.hpp index 725ec5694fc7..aa204efe6bfe 100644 --- a/src/display.hpp +++ b/src/display.hpp @@ -716,7 +716,8 @@ class display : public filter_context, public video2::draw_layering enum TERRAIN_TYPE { BACKGROUND, FOREGROUND}; - std::vector get_terrain_images(const map_location &loc, + // Warning: the returned vector will be invalidated on the next call! + const std::vector& get_terrain_images(const map_location &loc, const std::string& timeid, TERRAIN_TYPE terrain_type); @@ -801,6 +802,10 @@ class display : public filter_context, public video2::draw_layering /** Animated flags for each team */ std::vector > 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 terrain_image_vector_; + public: /** * The layers to render something on. This value should never be stored diff --git a/src/sdl/surface.hpp b/src/sdl/surface.hpp index 5d31e1307714..25aa386a857b 100644 --- a/src/sdl/surface.hpp +++ b/src/sdl/surface.hpp @@ -33,6 +33,11 @@ class surface add_surface_ref(surface_); } + surface(surface&& s) : surface_(s.get()) + { + s.surface_ = nullptr; + } + ~surface() { free_surface(); @@ -54,6 +59,14 @@ class surface 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; }