Run all rendering with pim lock if possible

client/CPlayerInterface.cpp

@@ -119,6 +119,7 @@ CPlayerInterface::CPlayerInterface(PlayerColor Player)
 	
 	duringMovement = false;
 	ignoreEvents = false;
+	locked = false;
 }
 
 CPlayerInterface::~CPlayerInterface()
@@ -1528,31 +1529,11 @@ bool CPlayerInterface::ctrlPressed() const
 
 void CPlayerInterface::update()
 {
-	// Updating GUI requires locking pim mutex (that protects screen and GUI state).
+	if (!locked)
-	// When ending the game, the pim mutex might be hold by other thread,
-	// that will notify us about the ending game by setting terminate_cond flag.
-
-	bool acquiredTheLockOnPim = false; //for tracking whether pim mutex locking succeeded
-	while(!terminate_cond.get() && !(acquiredTheLockOnPim = pim->try_lock())) //try acquiring long until it succeeds or we are told to terminate
-		boost::this_thread::sleep(boost::posix_time::milliseconds(15));
-
-	if(!acquiredTheLockOnPim)
 	{
-		// We broke the while loop above and not because of mutex, so we must be terminating.
+		logGlobal->errorStream() << "Non synchronized update of PlayerInterface";
-		assert(terminate_cond.get());
 		return;
 	}
-
-	// If we are here, pim mutex has been successfully locked - let's store it in a safe RAII lock.
-	boost::unique_lock<boost::recursive_mutex> un(*pim, boost::adopt_lock);
-
-	// While mutexes were locked away we may be have stopped being the active interface
-	if(LOCPLINT != this)
-		return;
-
-	// Make sure that gamestate won't change when GUI objects may obtain its parts on event processing or drawing request
-	boost::shared_lock<boost::shared_mutex> gsLock(cb->getGsMutex());
-
 	//if there are any waiting dialogs, show them
 	if((howManyPeople <= 1 || makingTurn) && !dialogs.empty() && !showingDialog->get())
 	{
@@ -1580,6 +1561,38 @@ void CPlayerInterface::update()
 		GH.drawFPSCounter();
 }
 
+void CPlayerInterface::runLocked(std::function<void(IUpdateable * )> functor)
+{
+	// Updating GUI requires locking pim mutex (that protects screen and GUI state).
+	// When ending the game, the pim mutex might be hold by other thread,
+	// that will notify us about the ending game by setting terminate_cond flag.
+
+	bool acquiredTheLockOnPim = false; //for tracking whether pim mutex locking succeeded
+	while(!terminate_cond.get() && !(acquiredTheLockOnPim = pim->try_lock())) //try acquiring long until it succeeds or we are told to terminate
+		boost::this_thread::sleep(boost::posix_time::milliseconds(15));
+
+	if(!acquiredTheLockOnPim)
+	{
+		// We broke the while loop above and not because of mutex, so we must be terminating.
+		assert(terminate_cond.get());
+		return;
+	}
+
+	// If we are here, pim mutex has been successfully locked - let's store it in a safe RAII lock.
+	boost::unique_lock<boost::recursive_mutex> un(*pim, boost::adopt_lock);
+
+	// While mutexes were locked away we may be have stopped being the active interface
+	if(LOCPLINT != this)
+		return;
+		
+	// Make sure that gamestate won't change when GUI objects may obtain its parts on event processing or drawing request
+	boost::shared_lock<boost::shared_mutex> gsLock(cb->getGsMutex());		
+	
+	locked = true;	
+	functor(this);
+	locked = false;
+}
+
 int CPlayerInterface::getLastIndex( std::string namePrefix)
 {
 	using namespace boost::filesystem;

client/CPlayerInterface.h

@@ -129,7 +129,8 @@ public:
 		}
 	} spellbookSettings;
 
-	void update();
+	void update() override;
+	void runLocked(std::function<void(IUpdateable * )> functor) override;
 	void initializeHeroTownList();
 	int getLastIndex(std::string namePrefix);
 
@@ -289,6 +290,8 @@ private:
 	bool duringMovement;
 	bool ignoreEvents;
 	
+	bool locked;
+	
 	void doMoveHero(const CGHeroInstance *h, CGPath path);
 };
 

client/CPreGame.cpp

@@ -538,6 +538,12 @@ void CGPreGame::update()
 		GH.drawFPSCounter();
 }
 
+void CGPreGame::runLocked(std::function<void(IUpdateable * )> cb)
+{
+	boost::unique_lock<boost::recursive_mutex> lock(*CPlayerInterface::pim);
+	cb(this);	
+}
+
 void CGPreGame::openCampaignScreen(std::string name)
 {
 	if (vstd::contains(CGPreGameConfig::get().getCampaigns().Struct(), name))

client/CPreGame.h

@@ -603,6 +603,7 @@ public:
 
 	~CGPreGame();
 	void update();
+	void runLocked(std::function<void(IUpdateable * )> cb) override;
 	void openSel(CMenuScreen::EState type, CMenuScreen::EMultiMode multi = CMenuScreen::SINGLE_PLAYER);
 
 	void openCampaignScreen(std::string name);

client/gui/CGuiHandler.cpp

@@ -409,21 +409,28 @@ void CGuiHandler::fakeMouseMove()
 
 void CGuiHandler::renderFrame()
 {
+	auto doUpdate = [](IUpdateable * target)
+	{
+		if(nullptr != target)
+			target -> update();
+		// draw the mouse cursor and update the screen
+		CCS->curh->render();
+
+		#ifndef	VCMI_SDL1
+		if(0 != SDL_RenderCopy(mainRenderer, screenTexture, nullptr, nullptr))
+			logGlobal->errorStream() << __FUNCTION__ << " SDL_RenderCopy " << SDL_GetError();
+
+		SDL_RenderPresent(mainRenderer);				
+		#endif		
+		
+	};
+	
 	if(curInt)
-		curInt->update(); // calls a update and drawing process of the loaded game interface object at the moment
+		curInt->runLocked(doUpdate);
-
+	else
-	// draw the mouse cursor and update the screen
+		doUpdate(nullptr);
-	CCS->curh->render();
-
-	#ifndef	VCMI_SDL1
-	if(0 != SDL_RenderCopy(mainRenderer, screenTexture, nullptr, nullptr))
-		logGlobal->errorStream() << __FUNCTION__ << " SDL_RenderCopy " << SDL_GetError();
-
-	SDL_RenderPresent(mainRenderer);				
-	#endif			
-	
-	mainFPSmng->framerateDelay(); // holds a constant FPS
 	
+	mainFPSmng->framerateDelay(); // holds a constant FPS	
 }
 
 

client/gui/CIntObject.cpp

@@ -4,6 +4,14 @@
 #include "SDL_Extensions.h"
 #include "../CMessage.h"
 
+void IUpdateable::runLocked(std::function<void(IUpdateable*)> cb)
+{
+	boost::unique_lock<boost::recursive_mutex> lock(updateGuard);	
+	cb(this);
+}
+
+
+
 CIntObject::CIntObject(int used_, Point pos_):
 	parent_m(nullptr),
 	active_m(0),

client/gui/CIntObject.h

@@ -31,7 +31,10 @@ public:
 
 class IUpdateable
 {
+	boost::recursive_mutex updateGuard;
 public:
+	virtual void runLocked(std::function<void(IUpdateable * )> cb);
+	
 	virtual void update()=0;
 	virtual ~IUpdateable(){}; //d-tor
 };