Merge pull request #5587 from IvanSavenko/crashfixes_beta

Fixes for beta

client/renderSDL/RenderHandler.cpp

@@ -58,13 +58,14 @@ std::shared_ptr<CDefFile> RenderHandler::getAnimationFile(const AnimationPath &
 	auto it = animationFiles.find(actualPath);
 
 	if (it != animationFiles.end())
-		return it->second;
+	{
+		auto locked = it->second.lock();
+		if (locked)
+			return locked;
+	}
 
 	if (!CResourceHandler::get()->existsResource(actualPath))
-	{
-		animationFiles[actualPath] = nullptr;
 		return nullptr;
-	}
 
 	auto result = std::make_shared<CDefFile>(actualPath);
 
@@ -200,7 +201,11 @@ std::shared_ptr<ScalableImageShared> RenderHandler::loadImageImpl(const ImageLoc
 {
 	auto it = imageFiles.find(locator);
 	if (it != imageFiles.end())
-		return it->second;
+	{
+		auto locked = it->second.lock();
+		if (locked)
+			return locked;
+	}
 
 	auto sdlImage = loadImageFromFileUncached(locator);
 	auto scaledImage = std::make_shared<ScalableImageShared>(locator, sdlImage);

client/renderSDL/RenderHandler.h

@@ -24,9 +24,9 @@ class RenderHandler final : public IRenderHandler
 {
 	using AnimationLayoutMap = std::map<size_t, std::vector<ImageLocator>>;
 
-	std::map<AnimationPath, std::shared_ptr<CDefFile>> animationFiles;
+	std::map<AnimationPath, std::weak_ptr<CDefFile>> animationFiles;
 	std::map<AnimationPath, AnimationLayoutMap> animationLayouts;
-	std::map<SharedImageLocator, std::shared_ptr<ScalableImageShared>> imageFiles;
+	std::map<SharedImageLocator, std::weak_ptr<ScalableImageShared>> imageFiles;
 	std::map<EFonts, std::shared_ptr<const IFont>> fonts;
 	std::unique_ptr<AssetGenerator> assetGenerator;
 

client/renderSDL/SDLImage.cpp

@@ -246,7 +246,7 @@ std::shared_ptr<const ISharedImage> SDLImageShared::scaleInteger(int factor, SDL
 	else
 		algorithm = EScalingAlgorithm::XBRZ_ALPHA;
 
-	auto result = std::make_shared<SDLImageShared>(this, factor, algorithm);
+	auto result = SDLImageShared::createScaled(this, factor, algorithm);
 
 	if (surf->format->palette)
 		SDL_SetSurfacePalette(surf, originalPalette);
@@ -254,28 +254,31 @@ std::shared_ptr<const ISharedImage> SDLImageShared::scaleInteger(int factor, SDL
 	return result;
 }
 
-SDLImageShared::SDLImageShared(const SDLImageShared * from, int integerScaleFactor, EScalingAlgorithm algorithm)
+std::shared_ptr<SDLImageShared> SDLImageShared::createScaled(const SDLImageShared * from, int integerScaleFactor, EScalingAlgorithm algorithm)
 {
+	auto self = std::make_shared<SDLImageShared>(nullptr);
+
 	static tbb::task_arena upscalingArena;
 
-	upscalingInProgress = true;
+	self->upscalingInProgress = true;
 
 	auto scaler = std::make_shared<SDLImageScaler>(from->surf, Rect(from->margins, from->fullSize), true);
 
-	const auto & scalingTask = [this, algorithm, scaler]()
+	const auto & scalingTask = [self, algorithm, scaler]()
 	{
 		scaler->scaleSurfaceIntegerFactor(GH.screenHandler().getScalingFactor(), algorithm);
-		surf = scaler->acquireResultSurface();
-		fullSize = scaler->getResultDimensions().dimensions();
-		margins = scaler->getResultDimensions().topLeft();
-
-		upscalingInProgress = false;
+		self->surf = scaler->acquireResultSurface();
+		self->fullSize = scaler->getResultDimensions().dimensions();
+		self->margins = scaler->getResultDimensions().topLeft();
+		self->upscalingInProgress = false;
 	};
 
 	if(settings["video"]["asyncUpscaling"].Bool())
 		upscalingArena.enqueue(scalingTask);
 	else
 		scalingTask();
+
+	return self;
 }
 
 bool SDLImageShared::isLoading() const
@@ -352,9 +355,10 @@ Rect SDLImageShared::contentRect() const
 	if(upscalingInProgress)
 		throw std::runtime_error("Attempt to access images that is still being loaded!");
 
-	auto tmpMargins = margins;
-	auto tmpSize = Point(surf->w, surf->h);
-	return Rect(tmpMargins, tmpSize);
+	if (!surf)
+		return Rect();
+
+	return Rect(margins, Point(surf->w, surf->h));
 }
 
 const SDL_Palette * SDLImageShared::getPalette() const

client/renderSDL/SDLImage.h

@@ -49,10 +49,11 @@ public:
 	SDLImageShared(const ImagePath & filename);
 	//Create using existing surface, extraRef will increase refcount on SDL_Surface
 	SDLImageShared(SDL_Surface * from);
-	/// Creates image at specified scaling factor from source image
-	SDLImageShared(const SDLImageShared * from, int integerScaleFactor, EScalingAlgorithm algorithm);
 	~SDLImageShared();
 
+	/// Creates image at specified scaling factor from source image
+	static std::shared_ptr<SDLImageShared> createScaled(const SDLImageShared * from, int integerScaleFactor, EScalingAlgorithm algorithm);
+
 	void scaledDraw(SDL_Surface * where, SDL_Palette * palette, const Point & scaling, const Point & dest, const Rect * src, const ColorRGBA & colorMultiplier, uint8_t alpha, EImageBlitMode mode) const override;
 	void draw(SDL_Surface * where, SDL_Palette * palette, const Point & dest, const Rect * src, const ColorRGBA & colorMultiplier, uint8_t alpha, EImageBlitMode mode) const override;
 

client/renderSDL/SDL_Extensions.cpp

@@ -85,7 +85,7 @@ SDL_Surface * CSDL_Ext::newSurface(const Point & dimensions, SDL_Surface * mod)
 		std::string messagePattern = "Failed to create SDL Surface of size %d x %d, %d bpp. Reason: %s";
 		std::string message = boost::str(boost::format(messagePattern) % dimensions.x % dimensions.y % mod->format->BitsPerPixel % error);
 
-		handleFatalError(message, true);
+		throw std::runtime_error(message);
 	}
 
 	if (mod->format->palette)

client/renderSDL/ScreenHandler.cpp

@@ -368,6 +368,7 @@ EUpscalingFilter ScreenHandler::loadUpscalingFilter() const
 	float scaleY = static_cast<float>(outputResolution.x) / logicalResolution.x;
 	float scaling = std::min(scaleX, scaleY);
 	int systemMemoryMb = SDL_GetSystemRAM();
+	bool is32Bit = sizeof(void*) == 4;
 
 	if (scaling <= 1.001f)
 		return EUpscalingFilter::NONE; // running at original resolution or even lower than that - no need for xbrz
@@ -375,6 +376,9 @@ EUpscalingFilter ScreenHandler::loadUpscalingFilter() const
 	if (systemMemoryMb < 2048)
 		return EUpscalingFilter::NONE; // xbrz2 may use ~1.0 - 1.5 Gb of RAM and has notable CPU cost - avoid on low-spec hardware
 
+	if (is32Bit)
+		return EUpscalingFilter::NONE; // to be safe, avoid large numbers of memory (re)allocations when address space is small
+
 	// Only using xbrz2 for autoselection.
 	// Higher options may have high system requirements and should be only selected explicitly by player
 	return EUpscalingFilter::XBRZ_2;

config/schemas/settings.json

@@ -649,14 +649,15 @@
 			"additionalProperties" : false,
 			"required" : [ 
 				"setupCompleted", 
-				"defaultRepositoryEnabled", 
-				"defaultRepositoryURL", 
-				"extraRepositoryURL", 
-				"extraRepositoryEnabled", 
-				"autoCheckRepositories", 
+				"defaultRepositoryEnabled",
+				"defaultRepositoryURL",
+				"extraRepositoryURL",
+				"extraRepositoryEnabled",
+				"autoCheckRepositories",
 				"ignoreSslErrors",
-				"updateOnStartup", 
-				"updateConfigUrl"
+				"updateOnStartup",
+				"updateConfigUrl",
+				"trackClipboardState"
 			],
 			"properties" : {
 				"defaultRepositoryEnabled" : {
@@ -694,7 +695,15 @@
 				"updateConfigUrl" : {
 					"type" : "string",
 					"default" : "https://raw.githubusercontent.com/vcmi/vcmi-updates/master/vcmi-updates.json"
-				}
+				},
+				"trackClipboardState" : {
+					"type" : "boolean",
+					"default" : true,
+					"defaultIOS": false,
+					"defaultAndroid": false,
+					"defaultDesktop" : true
+
+				},
 			}
 		},
 		"lobby" : {

launcher/startGame/StartGameTab.cpp

@@ -50,9 +50,11 @@ StartGameTab::StartGameTab(QWidget * parent)
 	ui->buttonGameEditor->hide();
 #endif
 
-	auto clipboard = QGuiApplication::clipboard();
-
-	connect(clipboard, SIGNAL(dataChanged()), this, SLOT(clipboardDataChanged()));
+	if (settings["launcher"]["trackClipboardState"].Bool())
+	{
+		auto clipboard = QGuiApplication::clipboard();
+		connect(clipboard, SIGNAL(dataChanged()), this, SLOT(clipboardDataChanged()));
+	}
 }
 
 void StartGameTab::clipboardDataChanged()
@@ -103,7 +105,8 @@ void StartGameTab::refreshState()
 	refreshPresets();
 	refreshMods();
 
-	clipboardDataChanged();
+	if (settings["launcher"]["trackClipboardState"].Bool())
+		clipboardDataChanged();
 }
 
 void StartGameTab::refreshPresets()
@@ -405,9 +408,22 @@ void StartGameTab::on_buttonPresetExport_clicked()
 void StartGameTab::on_buttonPresetImport_clicked()
 {
 	QString presetString = QGuiApplication::clipboard()->text();
+
+	if (!presetString.startsWith("{"))
+	{
+		MessageBoxCustom::information(this, tr("Preset import failed"), tr("Failed to import preset - data in clipboard does not looks like mod preset!"));
+		return;
+	}
+
 	QByteArray presetBytes(presetString.toUtf8());
 	JsonNode presetJson(reinterpret_cast<const std::byte*>(presetBytes.data()), presetBytes.size(), "imported preset");
 
+	if (presetJson["name"].String().empty() || presetJson["mods"].Vector().empty())
+	{
+		MessageBoxCustom::information(this, tr("Preset import failed"), tr("Failed to import preset - data in clipboard does not looks like mod preset!"));
+		return;
+	}
+
 	getMainWindow()->getModView()->importPreset(presetJson);
 	getMainWindow()->switchToModsTab();
 	refreshPresets();

lib/CConfigHandler.cpp

@@ -55,6 +55,13 @@ SettingsStorage::SettingsStorage():
 {
 }
 
+SettingsStorage::~SettingsStorage()
+{
+	// hack for possible crash due to static destruction order (setting storage can be destroyed before all listeners have died)
+	for(SettingsListener * listener : listeners)
+		listener->terminate();
+}
+
 void SettingsStorage::init(const std::string & dataFilename, const std::string & schema)
 {
 	this->dataFilename = dataFilename;
@@ -132,9 +139,15 @@ SettingsListener::SettingsListener(const SettingsListener &sl):
 	parent.listeners.insert(this);
 }
 
+void SettingsListener::terminate()
+{
+	wasTerminated = true;
+}
+
 SettingsListener::~SettingsListener()
 {
-	parent.listeners.erase(this);
+	if (!wasTerminated)
+		parent.listeners.erase(this);
 }
 
 void SettingsListener::nodeInvalidated(const std::vector<std::string> &changedPath)

lib/CConfigHandler.h

@@ -48,6 +48,7 @@ class DLL_LINKAGE SettingsStorage
 public:
 	// Initialize config structure
 	SettingsStorage();
+	~SettingsStorage();
 	void init(const std::string & dataFilename, const std::string & schema);
 	
 	// Get write access to config node at path
@@ -73,11 +74,15 @@ class DLL_LINKAGE SettingsListener
 	// Callback
 	std::function<void(const JsonNode&)> callback;
 
+	// hack for crash due to static destruction order
+	bool wasTerminated = false;
+
 	SettingsListener(SettingsStorage & _parent, std::vector<std::string> _path);
 
 	// Executes callback if changedpath begins with path
 	void nodeInvalidated(const std::vector<std::string> & changedPath);
 
+	void terminate();
 public:
 	SettingsListener(const SettingsListener &sl);
 	~SettingsListener();