cpp/openttd-patchpack/source Changeset - r24884:75b20bf9a7fd

Changeset - r24884:75b20bf9a7fd

Parent rev.

Child rev.

[Not reviewed]

master

0 4 0

Michael Lutz - 4 years ago 2021-01-16 15:43:11
michi@icosahedron.de

Change: Lock the video buffer when drawing inside the game loop to properly account for threaded drawing.

4 files changed with 75 insertions and 50 deletions:

src/screenshot.cpp

src/video/video_driver.hpp

src/video/win32_v.cpp

src/video/win32_v.h

0 comments (0 inline, 0 general)

src/screenshot.cpp

➞

Show inline comments

@@ @@ -22,12 +22,13 @@ @@
 #include "error.h"
 #include "textbuf_gui.h"
 #include "window_gui.h"
 #include "window_func.h"
 #include "tile_map.h"
 #include "landscape.h"
 #include "video/video_driver.hpp"
 #include "table/strings.h"
 #include "safeguards.h"
 static const char * const SCREENSHOT_NAME = "screenshot"; ///< Default filename of a saved screenshot.
@@ @@ -878,12 +879,14 @@ void MakeScreenshotWithConfirm(Screensho @@
  * @param name the name to give to the screenshot.
  * @return true iff the screenshot was made successfully
  * @see MakeScreenshotWithConfirm
  */
 bool MakeScreenshot(ScreenshotType t, const char *name)
+{
 	VideoDriver::VideoBufferLocker lock;
 	if (t == SC_VIEWPORT) {
 		/* First draw the dirty parts of the screen and only then change the name
 		 * of the screenshot. This way the screenshot will always show the name
 		 * of the previous screenshot in the 'successful' message instead of the
 		 * name of the new screenshot (or an empty name). */
 		UndrawMouseCursor();

src/video/video_driver.hpp

➞

Show inline comments

@@ @@ -124,12 +124,31 @@ public: @@
 	 * Get the currently active instance of the video driver.
 	 */
 	static VideoDriver *GetInstance() {
 		return static_cast<VideoDriver*>(*DriverFactoryBase::GetActiveDriver(Driver::DT_VIDEO));
+	}
 	/**
 	 * Helper struct to ensure the video buffer is locked and ready for drawing. The destructor
 	 * will make sure the buffer is unlocked no matter how the scope is exited.
 	 */
 	struct VideoBufferLocker {
 		VideoBufferLocker()
+		{
 			this->unlock = VideoDriver::GetInstance()->LockVideoBuffer();
+		}
 		~VideoBufferLocker()
+		{
 			if (this->unlock) VideoDriver::GetInstance()->UnlockVideoBuffer();
+		}
 	private:
 		bool unlock; ///< Stores if the lock did anything that has to be undone.
 	};
 protected:
 	const uint ALLOWED_DRIFT = 5; ///< How many times videodriver can miss deadlines without it being overly compensated.
 	/**
 	 * Get the resolution of the main screen.
 	 */

src/video/win32_v.cpp

➞

Show inline comments

@@ @@ -23,14 +23,12 @@ @@
 #include "../window_gui.h"
 #include "../window_func.h"
 #include "../framerate_type.h"
 #include "win32_v.h"
 #include <windows.h>
 #include <imm.h>
 #include <mutex>
 #include <condition_variable>
 #include "../safeguards.h"
 /* Missing define in MinGW headers. */
 #ifndef MAPVK_VK_TO_CHAR
 #define MAPVK_VK_TO_CHAR    (2)
@@ @@ -50,20 +48,12 @@ static struct { @@
 } _wnd;
 bool _window_maximize;
 static Dimension _bck_resolution;
 DWORD _imm_props;
 /** Whether the drawing is/may be done in a separate thread. */
 static bool _draw_threaded;
 /** Mutex to keep the access to the shared memory controlled. */
 static std::recursive_mutex *_draw_mutex = nullptr;
 /** Signal to draw the next frame. */
 static std::condition_variable_any *_draw_signal = nullptr;
 /** Should we keep continue drawing? */
 static volatile bool _draw_continue;
 /** Local copy of the palette for use in the drawing thread. */
 static Palette _local_palette;
 bool VideoDriver_Win32Base::ClaimMousePointer()
+{
 	MyShowCursor(false, true);
@@ @@ -931,40 +921,40 @@ void VideoDriver_Win32Base::InputLoop() @@
 void VideoDriver_Win32Base::MainLoop()
+{
 	MSG mesg;
 	std::thread draw_thread;
-	if (_draw_threaded) {
+	if (this->draw_threaded) {
 		/* Initialise the mutex first, because that's the thing we *need*
 		 * directly in the newly created thread. */
 		try {
 			_draw_signal = new std::condition_variable_any();
 			_draw_mutex = new std::recursive_mutex();
 			this->draw_signal = new std::condition_variable_any();
 			this->draw_mutex = new std::recursive_mutex();
 		} catch (...) {
-			_draw_threaded = false;
+			this->draw_threaded = false;
+		}
 		if (_draw_threaded) {
 			this->draw_lock = std::unique_lock<std::recursive_mutex>(*_draw_mutex);
 		if (this->draw_threaded) {
 			this->draw_lock = std::unique_lock<std::recursive_mutex>(*this->draw_mutex);
 			_draw_continue = true;
 			_draw_threaded = StartNewThread(&draw_thread, "ottd:draw-win32", &VideoDriver_Win32Base::PaintThreadThunk, this);
 			this->draw_continue = true;
 			this->draw_threaded = StartNewThread(&draw_thread, "ottd:draw-win32", &VideoDriver_Win32Base::PaintThreadThunk, this);
 			/* Free the mutex if we won't be able to use it. */
-			if (!_draw_threaded) {
+			if (!this->draw_threaded) {
 				this->draw_lock.unlock();
 				this->draw_lock.release();
 				delete _draw_mutex;
 				delete _draw_signal;
 				_draw_mutex = nullptr;
 				_draw_signal = nullptr;
 				delete this->draw_mutex;
 				delete this->draw_signal;
 				this->draw_mutex = nullptr;
 				this->draw_signal = nullptr;
 			} else {
 				DEBUG(driver, 1, "Threaded drawing enabled");
 				/* Wait till the draw thread has started itself. */
-				_draw_signal->wait(*_draw_mutex);
+				this->draw_signal->wait(*this->draw_mutex);
+			}
+		}
+	}
 	_wnd.running = true;
@@ @@ -979,34 +969,34 @@ void VideoDriver_Win32Base::MainLoop() @@
 		if (_exit_game) break;
 		/* Flush GDI buffer to ensure we don't conflict with the drawing thread. */
 		GdiFlush();
 		if (this->Tick()) {
 			if (_draw_mutex != nullptr && !HasModalProgress()) {
 				_draw_signal->notify_one();
 			if (this->draw_mutex != nullptr && !HasModalProgress()) {
 				this->draw_signal->notify_one();
 			} else {
 				this->Paint();
+			}
+		}
 		this->SleepTillNextTick();
+	}
 	if (_draw_threaded) {
 		_draw_continue = false;
 	if (this->draw_threaded) {
 		this->draw_continue = false;
 		/* Sending signal if there is no thread blocked
 		 * is very valid and results in noop */
-		_draw_signal->notify_all();
+		this->draw_signal->notify_all();
 		if (this->draw_lock.owns_lock()) this->draw_lock.unlock();
 		this->draw_lock.release();
 		draw_thread.join();
 		delete _draw_mutex;
 		delete _draw_signal;
 		delete this->draw_mutex;
 		delete this->draw_signal;
-		_draw_mutex = nullptr;
+		this->draw_mutex = nullptr;
+	}
+}
 void VideoDriver_Win32Base::ClientSizeChanged(int w, int h)
+{
 	/* Allocate backing store of the new size. */
@@ @@ -1022,44 +1012,44 @@ void VideoDriver_Win32Base::ClientSizeCh @@
+	}
+}
 bool VideoDriver_Win32Base::ChangeResolution(int w, int h)
+{
 	std::unique_lock<std::recursive_mutex> lock;
-	if (_draw_mutex != nullptr) lock = std::unique_lock<std::recursive_mutex>(*_draw_mutex);
+	if (this->draw_mutex != nullptr) lock = std::unique_lock<std::recursive_mutex>(*this->draw_mutex);
 	if (_window_maximize) ShowWindow(this->main_wnd, SW_SHOWNORMAL);
 	_wnd.width = _wnd.width_org = w;
 	_wnd.height = _wnd.height_org = h;
 	return this->MakeWindow(_fullscreen); // _wnd.fullscreen screws up ingame resolution switching
+}
 bool VideoDriver_Win32Base::ToggleFullscreen(bool full_screen)
+{
 	std::unique_lock<std::recursive_mutex> lock;
-	if (_draw_mutex != nullptr) lock = std::unique_lock<std::recursive_mutex>(*_draw_mutex);
+	if (this->draw_mutex != nullptr) lock = std::unique_lock<std::recursive_mutex>(*this->draw_mutex);
 	return this->MakeWindow(full_screen);
+}
 void VideoDriver_Win32Base::AcquireBlitterLock()
+{
-	if (_draw_mutex != nullptr) _draw_mutex->lock();
+	if (this->draw_mutex != nullptr) this->draw_mutex->lock();
+}
 void VideoDriver_Win32Base::ReleaseBlitterLock()
+{
-	if (_draw_mutex != nullptr) _draw_mutex->unlock();
+	if (this->draw_mutex != nullptr) this->draw_mutex->unlock();
+}
 void VideoDriver_Win32Base::EditBoxLostFocus()
+{
 	std::unique_lock<std::recursive_mutex> lock;
-	if (_draw_mutex != nullptr) lock = std::unique_lock<std::recursive_mutex>(*_draw_mutex);
+	if (this->draw_mutex != nullptr) lock = std::unique_lock<std::recursive_mutex>(*this->draw_mutex);
 	CancelIMEComposition(this->main_wnd);
 	SetCompositionPos(this->main_wnd);
 	SetCandidatePos(this->main_wnd);
+}
@@ @@ -1107,22 +1097,26 @@ float VideoDriver_Win32Base::GetDPIScale @@
 	return cur_dpi > 0 ? cur_dpi / 96.0f : 1.0f; // Default Windows DPI value is 96.
+}
 bool VideoDriver_Win32Base::LockVideoBuffer()
+{
 	if (_draw_threaded) this->draw_lock.lock();
 	if (this->buffer_locked) return false;
 	this->buffer_locked = true;
 	if (this->draw_threaded) this->draw_lock.lock();
 	_screen.dst_ptr = this->GetVideoPointer();
 	return true;
+}
 void VideoDriver_Win32Base::UnlockVideoBuffer()
+{
 	if (_draw_threaded) this->draw_lock.unlock();
 	if (this->draw_threaded) this->draw_lock.unlock();
 	this->buffer_locked = false;
+}
 static FVideoDriver_Win32GDI iFVideoDriver_Win32GDI;
 const char *VideoDriver_Win32GDI::Start(const StringList &param)
@@ @@ -1134,13 +1128,13 @@ const char *VideoDriver_Win32GDI::Start( @@
 	this->MakePalette();
 	this->AllocateBackingStore(_cur_resolution.width, _cur_resolution.height);
 	this->MakeWindow(_fullscreen);
 	MarkWholeScreenDirty();
-	_draw_threaded = !GetDriverParam(param, "no_threads") && !GetDriverParam(param, "no_thread") && std::thread::hardware_concurrency() > 1;
+	this->draw_threaded = !GetDriverParam(param, "no_threads") && !GetDriverParam(param, "no_thread") && std::thread::hardware_concurrency() > 1;
 	return nullptr;
+}
 void VideoDriver_Win32GDI::Stop()
+{
@@ @@ -1281,25 +1275,25 @@ void VideoDriver_Win32GDI::Paint() @@
 	this->dirty_rect = {};
+}
 void VideoDriver_Win32GDI::PaintThread()
+{
 	/* First tell the main thread we're started */
 	std::unique_lock<std::recursive_mutex> lock(*_draw_mutex);
 	_draw_signal->notify_one();
 	std::unique_lock<std::recursive_mutex> lock(*this->draw_mutex);
 	this->draw_signal->notify_one();
 	/* Now wait for the first thing to draw! */
-	_draw_signal->wait(*_draw_mutex);
+	this->draw_signal->wait(*this->draw_mutex);
-	while (_draw_continue) {
+	while (this->draw_continue) {
 		this->Paint();
 		/* Flush GDI buffer to ensure drawing here doesn't conflict with any GDI usage in the main WndProc. */
 		GdiFlush();
-		_draw_signal->wait(*_draw_mutex);
+		this->draw_signal->wait(*this->draw_mutex);
+	}
+}
 #ifdef _DEBUG
 /* Keep this function here..
@@ @@ -1354,13 +1348,13 @@ const char *VideoDriver_Win32OpenGL::Sta @@
 		_cur_resolution = old_res;
 		return err;
+	}
 	this->ClientSizeChanged(_wnd.width, _wnd.height);
-	_draw_threaded = false;
+	this->draw_threaded = false;
 	MarkWholeScreenDirty();
 	return nullptr;
+}
 void VideoDriver_Win32OpenGL::Stop()

src/video/win32_v.h

➞

Show inline comments

@@ @@ -8,17 +8,19 @@ @@
 /** @file win32_v.h Base of the Windows video driver. */
 #ifndef VIDEO_WIN32_H
 #define VIDEO_WIN32_H
 #include "video_driver.hpp"
 #include <mutex>
 #include <condition_variable>
 /** Base class for Windows video drivers. */
 class VideoDriver_Win32Base : public VideoDriver {
 public:
 	VideoDriver_Win32Base() : main_wnd(nullptr), fullscreen(false) {}
+	VideoDriver_Win32Base() : main_wnd(nullptr), fullscreen(false), draw_mutex(nullptr), draw_signal(nullptr) {}
 	void Stop() override;
 	void MakeDirty(int left, int top, int width, int height) override;
 	void MainLoop() override;
@@ @@ -33,15 +35,22 @@ public: @@
 	bool ClaimMousePointer() override;
 	void EditBoxLostFocus() override;
 protected:
 	HWND    main_wnd;      ///< Handle to system window.
 	bool    fullscreen;    ///< Whether to use (true) fullscreen mode.
 	Rect    dirty_rect;    ///< Region of the screen that needs redrawing.
 	HWND main_wnd;          ///< Handle to system window.
 	bool fullscreen;        ///< Whether to use (true) fullscreen mode.
 	Rect dirty_rect;        ///< Region of the screen that needs redrawing.
 	bool draw_threaded;          ///< Whether the drawing is/may be done in a separate thread.
 	bool buffer_locked;          ///< Video buffer was locked by the main thread.
 	volatile bool draw_continue; ///< Should we keep continue drawing?
 	std::recursive_mutex *draw_mutex;         ///< Mutex to keep the access to the shared memory controlled.
 	std::condition_variable_any *draw_signal; ///< Signal to draw the next frame.
 	Dimension GetScreenSize() const override;
 	float GetDPIScale() override;
 	void InputLoop() override;
 	bool LockVideoBuffer() override;
 	void UnlockVideoBuffer() override;

0 comments (0 inline, 0 general)