← Back to team overview

widelands-dev team mailing list archive

[Merge] lp:~widelands-dev/widelands/minimap_fix into lp:widelands

 

cghislai has proposed merging lp:~widelands-dev/widelands/minimap_fix into lp:widelands.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1209256 in widelands: "Saving a game not working because of minimap.png code"
  https://bugs.launchpad.net/widelands/+bug/1209256

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/minimap_fix/+merge/179569

StreamReader/Writer are implemented in Zipfilesystem. It is true streaming in the sense that nothing is buffered in the class but rather directly given to zlib and the zip file is directly updated. It allows saving of minimap images in the zip files.

An optional filesystem option may be given to ImageLoader::load so that we can load images from zip files.

A few fixes
-- 
https://code.launchpad.net/~widelands-dev/widelands/minimap_fix/+merge/179569
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/minimap_fix into lp:widelands.
=== modified file 'src/game_io/game_preload_data_packet.cc'
--- src/game_io/game_preload_data_packet.cc	2013-08-07 11:01:57 +0000
+++ src/game_io/game_preload_data_packet.cc	2013-08-10 12:23:11 +0000
@@ -97,7 +97,7 @@
 				m_number_of_players = s.get_safe_int(PLAYERS_AMOUNT_KEY_V4);
 			}
 			if (fs.FileExists(MINIMAP_FILENAME)) {
-				m_minimap_path = fs.FS_CanonicalizeName(MINIMAP_FILENAME);
+				m_minimap_path = MINIMAP_FILENAME;
 			}
 		} else {
 			throw game_data_error
@@ -156,6 +156,7 @@
 	std::unique_ptr< ::StreamWrite> sw(fs.OpenStreamWrite(MINIMAP_FILENAME));
 	if (sw.get() != nullptr) {
 		mmr.write_minimap_image(game, &ipl->player(), vp, flags, sw.get());
+		sw->Flush();
 	}
 	sw.reset();
 }

=== modified file 'src/graphic/image_loader.h'
--- src/graphic/image_loader.h	2013-02-10 16:08:37 +0000
+++ src/graphic/image_loader.h	2013-08-10 12:23:11 +0000
@@ -22,6 +22,7 @@
 
 #include <string>
 
+class FileSystem;
 class Surface;
 
 /// A thin interface that can load an Image from anywhere and turn it into a
@@ -30,7 +31,13 @@
 public:
 	virtual ~IImageLoader() {}
 
-	virtual Surface* load(const std::string& fn) const = 0;
+	/**
+	 * Loads an image into a surface and returns a pointer to it.
+	 * \param fn The image path
+	 * \param fs The filesystem to load the image from. If nullptr (default value),
+	 * loading will be attempted from the global layered filesystem (g_fs).
+	 */
+	virtual Surface* load(const std::string& fn, FileSystem* fs = nullptr) const = 0;
 };
 
 #endif /* end of include guard: IMAGE_LOADER_H */

=== modified file 'src/graphic/image_loader_impl.cc'
--- src/graphic/image_loader_impl.cc	2013-07-26 20:19:36 +0000
+++ src/graphic/image_loader_impl.cc	2013-08-10 12:23:11 +0000
@@ -30,11 +30,15 @@
 
 using namespace std;
 
-Surface* ImageLoaderImpl::load(const string& fname) const {
+Surface* ImageLoaderImpl::load(const string& fname, FileSystem* fs) const {
 	FileRead fr;
 	//log("Loading image %s.\n", fname.c_str());
 
-	fr.fastOpen(*g_fs, fname.c_str());
+	if (fs) {
+		fr.fastOpen(*fs, fname.c_str());
+	} else {
+		fr.fastOpen(*g_fs, fname.c_str());
+	}
 	SDL_Surface* sdlsurf = IMG_Load_RW(SDL_RWFromMem(fr.Data(0), fr.GetSize()), 1);
 
 	if (!sdlsurf)

=== modified file 'src/graphic/image_loader_impl.h'
--- src/graphic/image_loader_impl.h	2013-07-26 20:19:36 +0000
+++ src/graphic/image_loader_impl.h	2013-08-10 12:23:11 +0000
@@ -28,7 +28,7 @@
 public:
 	virtual ~ImageLoaderImpl() {}
 
-	Surface* load(const std::string& fname) const;
+	Surface* load(const std::string& fname, FileSystem* fs = nullptr) const;
 };
 
 

=== modified file 'src/io/filesystem/zip_filesystem.cc'
--- src/io/filesystem/zip_filesystem.cc	2013-07-26 20:19:36 +0000
+++ src/io/filesystem/zip_filesystem.cc	2013-08-10 12:23:11 +0000
@@ -28,6 +28,8 @@
 
 #include "io/filesystem/filesystem_exceptions.h"
 #include "io/filesystem/zip_exceptions.h"
+#include "io/streamread.h"
+#include "io/streamwrite.h"
 #include "wexception.h"
 
 /**
@@ -418,16 +420,105 @@
 	zipCloseFileInZip(m_zipfile);
 }
 
+//
+// Stream Read
+//
+
+ZipFilesystem::ZipStreamRead::ZipStreamRead(zipFile file, ZipFilesystem* zipfs)
+: m_unzipfile(file), m_zipfs(zipfs) {}
+ZipFilesystem::ZipStreamRead::~ZipStreamRead() {
+	m_zipfs->m_Close();
+}
+
+size_t ZipFilesystem::ZipStreamRead::Data(void* data, size_t bufsize)
+{
+	int copied = unzReadCurrentFile(m_unzipfile, data, bufsize);
+	if (copied < 0) {
+		throw new _data_error("Failed to read from zip file");
+	}
+	if (copied == 0) {
+		throw new _data_error("End of file reaced while reading zip");
+	}
+	return copied;
+}
+bool ZipFilesystem::ZipStreamRead::EndOfFile() const
+{
+	return unzReadCurrentFile(m_unzipfile, nullptr, 1) == 0;
+}
+
+
 StreamRead  * ZipFilesystem::OpenStreamRead (const std::string & fname) {
-	throw wexception
-		("OpenStreamRead(%s) not yet supported in ZipFilesystem",
-		 fname.c_str());
+	if (!FileExists(fname.c_str()) || IsDirectory(fname.c_str()))
+		throw ZipOperation_error
+			("ZipFilesystem::Load",
+			 fname,
+			 m_zipfilename,
+			 "could not open file from zipfile");
+
+	int32_t method;
+	m_OpenUnzip();
+	int result = unzOpenCurrentFile3(m_unzipfile, &method, nullptr, 1, nullptr);
+	switch (result) {
+		case ZIP_OK:
+			break;
+		default:
+			throw ZipOperation_error
+				("ZipFilesystem: Failed to open streamwrite", fname, m_zipfilename);
+	}
+	return new ZipStreamRead(m_unzipfile, this);
+}
+
+//
+// Stream write
+//
+
+ZipFilesystem::ZipStreamWrite::ZipStreamWrite(zipFile file, ZipFilesystem* zipfs)
+: m_zipfile(file), m_zipfs(zipfs) {}
+ZipFilesystem::ZipStreamWrite::~ZipStreamWrite()
+{
+	m_zipfs->m_Close();
+}
+
+void ZipFilesystem::ZipStreamWrite::Data(const void* const data, const size_t size)
+{
+	int result = zipWriteInFileInZip(m_zipfile, data, size);
+	switch (result) {
+		case ZIP_OK:
+			break;
+		default:
+			throw wexception("Failed to write into zipfile");
+	}
 }
 
 StreamWrite * ZipFilesystem::OpenStreamWrite(const std::string & fname) {
-	throw wexception
-		("OpenStreamWrite(%s) not yet supported in ZipFilesystem",
-		 fname.c_str());
+	m_OpenZip();
+
+	zip_fileinfo zi;
+
+	zi.tmz_date.tm_sec = zi.tmz_date.tm_min = zi.tmz_date.tm_hour =
+		zi.tmz_date.tm_mday = zi.tmz_date.tm_mon = zi.tmz_date.tm_year = 0;
+	zi.dosDate     = 0;
+	zi.internal_fa = 0;
+	zi.external_fa = 0;
+
+	std::string complete_filename = m_basename + "/" + fname;
+	//  create file
+	switch
+		(zipOpenNewFileInZip3
+			(m_zipfile, complete_filename.c_str(), &zi,
+		 	 0, 0, 0, 0, 0 /* comment*/,
+		 	 Z_DEFLATED,
+		 	 Z_BEST_COMPRESSION, 0,
+		 	 -MAX_WBITS, DEF_MEM_LEVEL, Z_DEFAULT_STRATEGY,
+		 	 0, 0))
+	{
+	case ZIP_OK:
+		break;
+	default:
+		throw ZipOperation_error
+			("ZipFilesystem: Failed to open streamwrite", complete_filename, m_zipfilename);
+	}
+	return new ZipStreamWrite(m_zipfile, this);
 }
 
 

=== modified file 'src/io/filesystem/zip_filesystem.h'
--- src/io/filesystem/zip_filesystem.h	2013-07-26 20:19:36 +0000
+++ src/io/filesystem/zip_filesystem.h	2013-08-10 12:23:11 +0000
@@ -27,6 +27,8 @@
 #include <minizip/zip.h>
 
 #include "io/filesystem/filesystem.h"
+#include "io/streamread.h"
+#include "io/streamwrite.h"
 #include "port.h"
 
 class ZipFilesystem : public FileSystem {
@@ -70,7 +72,7 @@
 
 	virtual std::string getBasename() {return m_zipfilename;};
 
-private:
+protected:
 	void m_OpenUnzip();
 	void m_OpenZip();
 	void m_Close();
@@ -92,6 +94,26 @@
 	std::string m_basenamezip;
 	std::string m_basename;
 
+	struct ZipStreamRead : StreamRead {
+		explicit ZipStreamRead(zipFile file, ZipFilesystem* zipfs);
+		virtual ~ZipStreamRead();
+		virtual size_t Data(void* data, size_t bufsize);
+		virtual bool EndOfFile() const;
+	private:
+		zipFile m_unzipfile;
+		ZipFilesystem* m_zipfs;
+	};
+	struct ZipStreamWrite : StreamWrite {
+		explicit ZipStreamWrite(zipFile file, ZipFilesystem* zipfs);
+		virtual ~ZipStreamWrite();
+		virtual void Data(const void* const data, size_t size);
+	private:
+		zipFile m_zipfile;
+		ZipFilesystem* m_zipfs;
+	};
+
 };
 
+
+
 #endif

=== modified file 'src/ui_basic/icon.cc'
--- src/ui_basic/icon.cc	2013-08-01 13:29:56 +0000
+++ src/ui_basic/icon.cc	2013-08-10 12:23:11 +0000
@@ -30,9 +30,7 @@
 	 const Image* picture_id)
 	:
 	Panel(parent, x, y, w, h),
-	m_pic(picture_id),
-	m_w(w),
-	m_h(h)
+	m_pic(picture_id)
 {
 	set_handle_mouse(false);
 	set_think(false);
@@ -51,15 +49,21 @@
 	m_framecolor.b = color.b;
 }
 
+void Icon::setNoFrame()
+{
+	m_draw_frame = false;
+}
+
+
 
 void Icon::draw(RenderTarget & dst) {
 	if (m_pic) {
-		int32_t w = (m_w - m_pic->width()) / 2;
-		int32_t h = (m_h - m_pic->height()) / 2;
+		int32_t w = (get_w() - m_pic->width()) / 2;
+		int32_t h = (get_h() - m_pic->height()) / 2;
 		dst.blit(Point(w, h), m_pic);
 	}
 	if (m_draw_frame) {
-		dst.draw_rect(Rect(0, 0, m_w, m_h), m_framecolor);
+		dst.draw_rect(Rect(0, 0, get_w(), get_h()), m_framecolor);
 	}
 }
 

=== modified file 'src/ui_basic/icon.h'
--- src/ui_basic/icon.h	2013-08-01 13:29:56 +0000
+++ src/ui_basic/icon.h	2013-08-10 12:23:11 +0000
@@ -36,12 +36,12 @@
 
 	void setIcon(const Image* picture_id);
 	void setFrame(const RGBColor& color);
+	void setNoFrame();
 
 	virtual void draw(RenderTarget &);
 
 private:
 	const Image* m_pic;
-	int32_t      m_w, m_h;
 	bool         m_draw_frame;
 	RGBColor    m_framecolor;
 };

=== modified file 'src/ui_fsmenu/loadgame.cc'
--- src/ui_fsmenu/loadgame.cc	2013-08-06 11:54:06 +0000
+++ src/ui_fsmenu/loadgame.cc	2013-08-10 12:23:11 +0000
@@ -49,6 +49,7 @@
 	m_buth (get_h() * 9 / 200),
 	m_fs   (fs_small()),
 	m_fn   (ui_fn()),
+	m_minimap_max_size(get_w() * 15 / 100),
 
 // "Data holder" for the savegame information
 	m_game(g),
@@ -104,7 +105,7 @@
 		 _("Minimap:"), UI::Align_Right),
 	m_minimap_icon
 		(this, get_w() * 71 / 100, get_h() * 10 / 20,
-		 get_w() * 15 / 100, get_w() * 15 / 100, nullptr),
+		 m_minimap_max_size, m_minimap_max_size, nullptr),
 	m_settings(gsp),
 	m_ctrl(gc),
 	m_image_loader(new ImageLoaderImpl())
@@ -127,7 +128,7 @@
 	m_label_players .set_font(m_fn, m_fs, UI_FONT_CLR_FG);
 	m_ta_players    .set_font(m_fn, m_fs, UI_FONT_CLR_FG);
 	m_ta_win_condition.set_font(m_fn, m_fs, UI_FONT_CLR_FG);
-	m_minimap_icon.setFrame(UI_FONT_CLR_FG);
+	m_minimap_icon.set_visible(false);
 	m_list          .set_font(m_fn, m_fs);
 	m_list.selected.connect(boost::bind(&Fullscreen_Menu_LoadGame::map_selected, this, _1));
 	m_list.double_clicked.connect(boost::bind(&Fullscreen_Menu_LoadGame::double_clicked, this, _1));
@@ -180,6 +181,9 @@
 	m_ta_players.set_text(std::string());
 	m_ta_win_condition.set_text(std::string());
 	m_minimap_icon.setIcon(nullptr);
+	m_minimap_icon.set_visible(false);
+	m_minimap_icon.setNoFrame();
+	m_minimap_image.reset();
 }
 
 
@@ -189,78 +193,88 @@
 		no_selection();
 		return;
 	}
-
-	if (const char * const name = m_list.get_selected()) {
-		Widelands::Game_Preload_Data_Packet gpdp;
-
-		try {
-			Widelands::Game_Loader gl(name, m_game);
-			gl.preload_game(gpdp);
-		} catch (const _wexception & e) {
-			if (!m_settings || m_settings->settings().saved_games.empty()) {
-				log("Save game '%s' must have changed from under us\nException: %s\n", name, e.what());
-				m_list.remove(selected);
-				return;
-			} else {
-				m_ok.set_enabled(true);
-				m_delete.set_enabled(false);
-				m_tamapname .set_text(_("Savegame from dedicated server"));
-				m_tagametime.set_text(_("Unknown gametime"));
-				return;
-			}
-		}
-
-		m_ok.set_enabled(true);
-		m_delete.set_enabled(true);
-
-		//Try to translate the map name.
-		//This will work on every official map as expected
-		//and 'fail silently' (not find a translation) for already translated campaign map names.
-		//It will also translate 'false-positively' on any user-made map which shares a name with
-		//the official maps, but this should not be a problem to worry about.
-		{
-			i18n::Textdomain td("maps");
-			m_tamapname.set_text(_(gpdp.get_mapname()));
-		}
-
-		char buf[20];
-		uint32_t gametime = gpdp.get_gametime();
-		m_tagametime.set_text(gametimestring(gametime));
-
-		if (gpdp.get_number_of_players() > 0) {
-			sprintf(buf, "%i", gpdp.get_number_of_players());
+	const char * const name = m_list.get_selected();
+	if (!name) {
+		no_selection();
+		return;
+	}
+
+	Widelands::Game_Preload_Data_Packet gpdp;
+	Widelands::Game_Loader gl(name, m_game);
+
+	try {
+		gl.preload_game(gpdp);
+	} catch (const _wexception & e) {
+		if (!m_settings || m_settings->settings().saved_games.empty()) {
+			log("Save game '%s' must have changed from under us\nException: %s\n", name, e.what());
+			m_list.remove(selected);
+			return;
 		} else {
-			sprintf(buf, "%s", _("Unknown"));
-		}
-		m_ta_players.set_text(buf);
-		m_ta_win_condition.set_text(gpdp.get_win_condition());
-
-		std::string minimap_path = gpdp.get_minimap_path();
-		// Delete former image
-		m_minimap_icon.setIcon(nullptr);
-		m_minimap_image.reset();
-		// Load the new one
-		if (!minimap_path.empty()) {
-			try {
-				std::unique_ptr<Surface> surface(m_image_loader->load(minimap_path));
-				m_minimap_image.reset(new_in_memory_image("minimap", surface.release()));
-				double scale = double(m_minimap_icon.get_w()) / m_minimap_image->width();
-				double scaleY = double(m_minimap_icon.get_h()) / m_minimap_image->height();
-				if (scaleY < scale) {
-					scale = scaleY;
-				}
-				uint16_t w = scale * m_minimap_image->width();
-				uint16_t h = scale * m_minimap_image->height();
-				const Image* resized = ImageTransformations::resize(m_minimap_image.get(), w, h);
-				// keeps our in_memory_image around and give to icon the one
-				// from resize that is handled by the cache. It is still linked to our
-				// surface
-				m_minimap_icon.setIcon(resized);
-			} catch (...) {
-			}
-		}
+			m_ok.set_enabled(true);
+			m_delete.set_enabled(false);
+			m_tamapname .set_text(_("Savegame from dedicated server"));
+			m_tagametime.set_text(_("Unknown gametime"));
+			return;
+		}
+	}
+
+	m_ok.set_enabled(true);
+	m_delete.set_enabled(true);
+
+	//Try to translate the map name.
+	//This will work on every official map as expected
+	//and 'fail silently' (not find a translation) for already translated campaign map names.
+	//It will also translate 'false-positively' on any user-made map which shares a name with
+	//the official maps, but this should not be a problem to worry about.
+	{
+		i18n::Textdomain td("maps");
+		m_tamapname.set_text(_(gpdp.get_mapname()));
+	}
+
+	char buf[20];
+	uint32_t gametime = gpdp.get_gametime();
+	m_tagametime.set_text(gametimestring(gametime));
+
+	if (gpdp.get_number_of_players() > 0) {
+		sprintf(buf, "%i", gpdp.get_number_of_players());
 	} else {
-		no_selection();
+		sprintf(buf, "%s", _("Unknown"));
+	}
+	m_ta_players.set_text(buf);
+	m_ta_win_condition.set_text(gpdp.get_win_condition());
+
+	std::string minimap_path = gpdp.get_minimap_path();
+	// Delete former image
+	m_minimap_icon.setIcon(nullptr);
+	m_minimap_icon.set_visible(false);
+	m_minimap_icon.setNoFrame();
+	m_minimap_image.reset();
+	// Load the new one
+	if (!minimap_path.empty()) {
+		try {
+			// Load the image
+			FileSystem* save_fs = g_fs->MakeSubFileSystem(name);
+			std::unique_ptr<Surface> surface(m_image_loader->load(minimap_path, save_fs));
+			m_minimap_image.reset(new_in_memory_image(std::string(name + minimap_path), surface.release()));
+			// Scale it
+			double scale = double(m_minimap_max_size) / m_minimap_image->width();
+			double scaleY = double(m_minimap_max_size) / m_minimap_image->height();
+			if (scaleY < scale) {
+				scale = scaleY;
+			}
+			uint16_t w = scale * m_minimap_image->width();
+			uint16_t h = scale * m_minimap_image->height();
+			const Image* resized = ImageTransformations::resize(m_minimap_image.get(), w, h);
+			// keeps our in_memory_image around and give to icon the one
+			// from resize that is handled by the cache. It is still linked to our
+			// surface
+			m_minimap_icon.set_size(w, h);
+			m_minimap_icon.setFrame(UI_FONT_CLR_FG);
+			m_minimap_icon.set_visible(true);
+			m_minimap_icon.setIcon(resized);
+		} catch (const std::exception & e) {
+			log("Failed to load the minimap image : %s\n", e.what());
+		}
 	}
 }
 

=== modified file 'src/ui_fsmenu/loadgame.h'
--- src/ui_fsmenu/loadgame.h	2013-08-06 11:54:06 +0000
+++ src/ui_fsmenu/loadgame.h	2013-08-10 12:23:11 +0000
@@ -66,6 +66,7 @@
 	uint32_t    m_buth;
 	uint32_t    m_fs;
 	std::string m_fn;
+	uint16_t    m_minimap_max_size;
 
 	Widelands::Game &                               m_game;
 	UI::Button                             m_back;
@@ -90,6 +91,7 @@
 	GameController                                * m_ctrl;
 	std::unique_ptr<const Image>                    m_minimap_image;
 	std::unique_ptr<const IImageLoader>             m_image_loader;
+
 };
 
 


Follow ups