widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

GunChleoc has proposed merging lp:~gunchleoc/widelands/bug-1818494-ingame-zoom-freezes into lp:widelands.

Commit message:
Skip calculating changes to the map view's viewport when nothing would change. This prevents the map view from being busy when the user leans on the CTRL+/- keys.

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/~gunchleoc/widelands/bug-1818494-ingame-zoom-freezes/+merge/364304

I think this should go into Build 20, because it can make the player think that Widelands has crashed, while it's just the mapview being busy.
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~gunchleoc/widelands/bug-1818494-ingame-zoom-freezes into lp:widelands.

=== modified file 'src/wui/mapview.cc'
--- src/wui/mapview.cc	2019-02-23 11:00:49 +0000
+++ src/wui/mapview.cc	2019-03-12 08:32:31 +0000
@@ -387,6 +387,10 @@
 	const Transition transition = animate_map_panning_ ? passed_transition : Transition::Jump;
 	switch (transition) {
 	case Transition::Jump: {
+		if (view_ == target_view) {
+			// We're already there
+			return;
+		}
 		view_ = target_view;
 		MapviewPixelFunctions::normalize_pix(map_, &view_.viewpoint);
 		changeview();
@@ -394,6 +398,10 @@
 	}
 
 	case Transition::Smooth: {
+		if (!view_plans_.empty() && view_plans_.back().back().view == target_view) {
+			// We're already there
+			return;
+		}
 		const TimestampedView current = animation_target_view();
 		const auto plan =
 		   plan_map_transition(current.t, map_, current.view, target_view, get_w(), get_h());
@@ -509,6 +517,10 @@
 	const TimestampedView current = animation_target_view();
 	switch (transition) {
 	case Transition::Jump: {
+		if (view_.zoom == new_zoom) {
+			// We're already there
+			return;
+		}
 		// Zoom around the current mouse position. See
 		// http://stackoverflow.com/questions/2916081/zoom-in-on-a-point-using-scale-and-translate
 		// for a good explanation of this math.
@@ -518,6 +530,10 @@
 	}
 
 	case Transition::Smooth: {
+		if (!view_plans_.empty() && view_plans_.back().back().view.zoom == new_zoom) {
+			// We're already there
+			return;
+		}
 		const int w = get_w();
 		const int h = get_h();
 		const auto plan = plan_zoom_transition(

=== modified file 'src/wui/mapview.h'
--- src/wui/mapview.h	2019-02-23 11:00:49 +0000
+++ src/wui/mapview.h	2019-03-12 08:32:31 +0000
@@ -75,6 +75,10 @@
 		View() : View(Vector2f::zero(), 1.0f) {
 		}
 
+		bool operator==(const View& other) const {
+			return (zoom == other.zoom) && (viewpoint == other.viewpoint);
+		}
+
 		// Mappixel of top-left pixel of this MapView.
 		Vector2f viewpoint;