widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-1818494-reset-zoom-crash into lp:widelands.

Commit message:
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.


Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1818494 in widelands: "Crash on "Reset zoom" bzr9005-201903031251 (Release)"
  https://bugs.launchpad.net/widelands/+bug/1818494

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1818494-reset-zoom-crash/+merge/364208

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.

-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1818494-reset-zoom-crash into lp:widelands.

=== 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);
 	}