Merge lp:~widelands-dev/widelands/bug-1818494-reset-zoom-crash into lp:widelands

[GunChleoc]

Fix/catch some memory issues
- Limit the editor undo stack to 500 items
- In fields_to_draw, make sure not to resize the vector to more memory than available,
  because this can crash when zooming out.
  Simply skip the unavailable bottom fields when drawing and log it.

This should help with a crash that a user experienced when using the editor for a long time or playing long games, then zooming out.

The problem is that Widelands doesn't have enough memory allocated to grow the vector to the required size in fields_to_draw.reset(). So, I have added a safeguard there which will log to console when there isn't enough memory and then simply not update the additional fields. The user will see fields not properly drawn/updated at the bottom of the screen then.

The problem happens only after a long time -> I found that the editor's "Undo" stack is unlimited, so I have putting an arbitrary cap on that.

No solution for the in-game memory consumption yet. We do have steadily growing containers there which are the statistics, but that can't be helped. There might be others that we haven't found yet though.

[Klaus Halfmann (klaus-halfmann)]

Wow, we should get the Map(s) this user created :-)

To reproduce the first problem we should edit the biggest maxium possible
map with as many objects a possible and zomm out and in like mad.
Never restart to get the overflow in that undo stack?.

An in a normal game this could happen, too?

Can we perhaps limit the maximum Zoom-factor to avoid this?

Some questions / comments inline.

[Klaus Halfmann (klaus-halfmann)]

Found a (related?) Bug at #1819311 before I coould do the > 500 undos :-)

OTOH from the code review it sould not harm.

I will do more tests with this huge (nice) map, lets see what we will find.

review: Approve (code reieww, compile, testing)

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 4574. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/504234539.
Appveyor build 4361. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1818494_reset_zoom_crash-4361.

[GunChleoc (gunchleoc)]

Thanks for the review!

@bunnybot merge

=== modified file 'src/editor/tools/history.cc'
--- src/editor/tools/history.cc	2019-02-23 11:00:49 +0000
+++ src/editor/tools/history.cc	2019-03-10 07:49:40 +0000
@@ -27,6 +27,9 @@
 
 // === EditorActionArgs === //
 
+constexpr size_t kMaximumUndoActions = 500;
+constexpr size_t kTooManyUndoActionsDeleteBatch = 50;
+
 EditorActionArgs::EditorActionArgs(EditorInteractive& base)
    : sel_radius(base.get_sel_radius()),
      change_by(0),
@@ -115,6 +118,11 @@
 		undo_stack_.push_front(ac);
 		undo_button_.set_enabled(true);
 		redo_button_.set_enabled(false);
+		if (undo_stack_.size() > kMaximumUndoActions) {
+			for (size_t i = 0; i < kTooManyUndoActionsDeleteBatch; ++i) {
+				undo_stack_.pop_back();
+			}
+		}
 	}
 	return tool.handle_click(ind, world, center, parent, ac.args, &map);
 }

=== modified file 'src/graphic/gl/fields_to_draw.cc'
--- src/graphic/gl/fields_to_draw.cc	2019-02-23 11:00:49 +0000
+++ src/graphic/gl/fields_to_draw.cc	2019-03-10 07:49:40 +0000
@@ -106,7 +106,17 @@
 
 	w_ = max_fx_ - min_fx_ + 1;
 	h_ = max_fy_ - min_fy_ + 1;
-	const size_t dimension = w_ * h_;
+	assert(w_ > 0);
+	assert(h_ > 0);
+
+	// Ensure that there is enough memory for the resize operation
+	size_t dimension = w_ * h_;
+	const size_t max_dimension = fields_.max_size();
+	if (dimension > max_dimension) {
+		log("WARNING: Not enough memory allocated to redraw the whole map!\nWe recommend that you restart Widelands\n");
+		dimension = max_dimension;
+	}
+	// Now resize the vector
 	if (fields_.size() != dimension) {
 		fields_.resize(dimension);
 	}