Rewrite the FPS cap implementation

The FPS cap, originally implemented in 2007, is very poorly done. It
doesn't take frame time variance into account, and is therefore almost
guaranteed to cause missed frames all the time. It doesn't increase timer
granularity on Windows, which causes SDL_Delay() to often take much longer
than intended. And it's hardcoded for 50 FPS, which fits poorly with 60 Hz
displays.

This new implementation fixes all those issues.

My experience is that the game feels much, much smoother with the new
implementation, perfectly competitive with 1.12. In my opinion, performance
is now at an acceptable level for a stable release.

changelog

@@ -38,6 +38,8 @@ Version 1.13.8+dev:
    * When set to remember your password, Wesnoth now encrypts it. It is still
      possible to obtain the password from preferences, but it's no longer as
      trivial as before.
+ * Performance:
+   * Rewrote the FPS cap implementation. This greatly improves smoothness ingame.
  * Units:
    * Added new lvl0 Giant Scorpling, leveling into the Giant Scorpion.
  * User Interface:
@@ -53,6 +55,8 @@ Version 1.13.8+dev:
  * Miscellaneous and Bug Fixes:
    * Add --report/-R command line switch for printing the same report from the
      Game Version dialog's clipboard function to stdout.
+   * Removed the --max-fps command line switch. The new FPS cap implementation
+     queries the screen refresh rate instead.
  * WFL Engine
    * Add owner key to terrain space callable, for villages
    * Location formulas in [tunnel] now have a teleport_unit variable

players_changelog

@@ -30,6 +30,9 @@ Version 1.13.8+dev:
      possible to obtain the password from preferences, but it's no longer as
      trivial as before.
 
+ * Performance:
+   * Rewrote the FPS cap implementation. This greatly improves smoothness ingame.
+
  * User Interface:
    * Unit recall dialog now sorts the units by both level and required XP for
      their next level-up (issue #1738).

src/commandline_options.cpp

@@ -97,7 +97,6 @@ commandline_options::commandline_options (const std::vector<std::string>& args)
 	multiplayer_scenario(),
 	multiplayer_side(),
 	multiplayer_turns(),
-	max_fps(),
 	noaddons(false),
 	nocache(false),
 	nodelay(false),
@@ -214,7 +213,6 @@ commandline_options::commandline_options (const std::vector<std::string>& args)
 	display_opts.add_options()
 		("fps", "displays the number of frames per second the game is currently running at, in a corner of the screen.")
 		("fullscreen,f", "runs the game in full screen mode.")
-		("max-fps", po::value<int>(), "the maximum fps the game tries to run at. Values should be between 1 and 1000, the default is 50.")
 		("new-widgets", "there is a new WIP widget toolkit this switch enables the new toolkit (VERY EXPERIMENTAL don't file bug reports since most are known). Parts of the library are deemed stable and will work without this switch.")
 		("resolution,r", po::value<std::string>(), "sets the screen resolution. <arg> should have format XxY. Example: --resolution 800x600")
 		("windowed,w", "runs the game in windowed mode.")
@@ -368,8 +366,6 @@ commandline_options::commandline_options (const std::vector<std::string>& args)
 		log_precise_timestamps = true;
 	if (vm.count("log-strict"))
 		parse_log_strictness(vm["log-strict"].as<std::string>());
-	if (vm.count("max-fps"))
-		max_fps = vm["max-fps"].as<int>();
 	if (vm.count("mp-test"))
 		mptest = true;
 	if (vm.count("multiplayer"))

src/commandline_options.hpp

@@ -125,8 +125,6 @@ public:
 	boost::optional<std::vector<std::pair<unsigned int, std::string> > > multiplayer_side;
 	/// Non-empty if --turns was given on the command line. Dependent on --multiplayer.
 	boost::optional<std::string> multiplayer_turns;
-	/// Max FPS specified by --max-fps option.
-	boost::optional<int> max_fps;
 	/// True if --noaddons was given on the command line. Disables the loading of all add-ons.
 	bool noaddons;
 	/// True if --nocache was given on the command line. Disables cache usage.

src/display.cpp

@@ -55,6 +55,10 @@
 
 #include <cmath>
 
+#ifdef _WIN32
+#include <Windows.h>
+#endif
+
 // Includes for bug #17573
 
 static lg::log_domain log_display("display");
@@ -177,7 +181,7 @@ display::display(const display_context * dc, CVideo& video, std::weak_ptr<wb::ma
 	reports_object_(&reports_object),
 	scroll_event_("scrolled"),
 	complete_redraw_event_("completely_redrawn"),
-	nextDraw_(0),
+	frametimes_(50),
 	reportRects_(),
 	reportSurfaces_(),
 	reports_(),
@@ -248,10 +252,18 @@ display::display(const display_context * dc, CVideo& video, std::weak_ptr<wb::ma
 		create_buttons();
 	}
 
+#ifdef _WIN32
+	// Increase timer resolution to prevent delays getting much longer than they should.
+	timeBeginPeriod(1u);
+#endif
 }
 
 display::~display()
 {
+#ifdef _WIN32
+	timeEndPeriod(1u);
+#endif
+
 	singleton_ = nullptr;
 	resources::fake_units = nullptr;
 }
@@ -1661,9 +1673,7 @@ void display::draw_init()
 
 void display::draw_wrap(bool update, bool force)
 {
-	static const int time_between_draws = preferences::draw_delay();
-	const int current_time = SDL_GetTicks();
-	const int wait_time = nextDraw_ - current_time;
+	static const int time_between_draws = 1000 / screen_.current_refresh_rate();
 
 	if(redrawMinimap_) {
 		redrawMinimap_ = false;
@@ -1672,20 +1682,17 @@ void display::draw_wrap(bool update, bool force)
 
 	if(update) {
 		update_display();
+
+		frametimes_.push_back(SDL_GetTicks() - last_frame_finished_);
+		int longest_frame = *std::max_element(frametimes_.begin(), frametimes_.end());
+		int wait_time = time_between_draws - longest_frame;
+
 		if(!force && !benchmark && wait_time > 0) {
 			// If it's not time yet to draw, delay until it is
 			SDL_Delay(wait_time);
 		}
 
-		// Set the theoretical next draw time
-		nextDraw_ += time_between_draws;
-
-		// If the next draw already should have been finished,
-		// we'll enter an update frenzy, so make sure that the
-		// too late value doesn't keep growing.
-		// Note: if force is used too often,
-		// we can also get the opposite effect.
-		nextDraw_ = std::max<int>(nextDraw_, SDL_GetTicks());
+		last_frame_finished_ = SDL_GetTicks();
 	}
 }
 

src/display.hpp

@@ -63,7 +63,10 @@ namespace wb {
 
 #include "overlay.hpp"
 
+#include <boost/circular_buffer.hpp>
+
 #include "utils/functional.hpp"
+#include <cstdint>
 #include <deque>
 #include <list>
 #include <map>
@@ -738,11 +741,8 @@ protected:
 	 */
 	events::generic_event complete_redraw_event_;
 
-	/**
-	 * Holds the tick count for when the next drawing event is scheduled.
-	 * Drawing shouldn't occur before this time.
-	 */
-	int nextDraw_;
+	boost::circular_buffer<unsigned> frametimes_; // in milliseconds
+	uint32_t last_frame_finished_ = 0u;
 
 	// Not set by the initializer:
 	std::map<std::string, SDL_Rect> reportRects_;
@@ -1030,8 +1030,6 @@ private:
 
 	bool dirty_;
 
-
-
 public:
 	void replace_overlay_map(overlay_map* overlays) { overlays_ = overlays; }
 

src/game_launcher.cpp

@@ -186,20 +186,6 @@ game_launcher::game_launcher(const commandline_options& cmdline_opts, const char
 		video().set_fullscreen(true);
 	if (cmdline_opts_.load)
 		load_data_.reset(new savegame::load_game_metadata{ *cmdline_opts_.load });
-	if (cmdline_opts_.max_fps) {
-		int fps;
-		//FIXME: remove the next line once the weird util.cpp specialized template lexical_cast_default() linking issue is solved
-		fps = lexical_cast_default<int>("", 50);
-		fps = *cmdline_opts_.max_fps;
-		fps = std::min<int>(fps, 1000);
-		fps = std::max<int>(fps, 1);
-		fps = 1000 / fps;
-		// increase the delay to avoid going above the maximum
-		if(1000 % fps != 0) {
-			++fps;
-		}
-		preferences::set_draw_delay(fps);
-	}
 	if (cmdline_opts_.nogui || cmdline_opts_.headless_unit_test) {
 		no_sound = true;
 		preferences::disable_preferences_save();

src/preferences/general.cpp

@@ -49,8 +49,6 @@ bool no_preferences_save = false;
 
 bool fps = false;
 
-int draw_delay_ = 20;
-
 config prefs;
 }
 
@@ -979,16 +977,6 @@ void set_show_fps(bool value)
 	fps = value;
 }
 
-int draw_delay()
-{
-	return draw_delay_;
-}
-
-void set_draw_delay(int value)
-{
-	draw_delay_ = value;
-}
-
 bool use_color_cursors()
 {
 	return get("color_cursors", true);

src/preferences/general.hpp

@@ -236,9 +236,6 @@ namespace preferences {
 	 */
 	int mouse_scroll_threshold();
 
-	int draw_delay();
-	void set_draw_delay(int value);
-
 	bool animate_map();
 	void set_animate_map(bool value);
 

src/video.cpp

@@ -235,6 +235,10 @@ void CVideo::init_window()
 		preferences::min_window_height
 	);
 
+	SDL_DisplayMode currentDisplayMode;
+	SDL_GetCurrentDisplayMode(window->get_display_index(), &currentDisplayMode);
+	refresh_rate_ = currentDisplayMode.refresh_rate != 0 ? currentDisplayMode.refresh_rate : 60;
+
 	event_handler_.join_global();
 
 	update_framebuffer();

src/video.hpp

@@ -75,6 +75,7 @@ public:
 	void set_resolution(const unsigned width, const unsigned height);
 
 	std::pair<int,int> current_resolution();
+	int current_refresh_rate() { return refresh_rate_; }
 
 	//functions to get the dimensions of the current video-mode
 	int getx() const;
@@ -194,6 +195,7 @@ private:
 
 	int updatesLocked_;
 	int flip_locked_;
+	int refresh_rate_;
 };
 
 //an object which will lock the display for the duration of its lifetime.