Fix: jumping effect when scrolling viewport over bottom edge of the map (Patch by adf88, #6583)

3 files changed, 33 insertions, 114 deletions

diff --git a/src/landscape.cpp b/src/landscape.cpp
index eb1e404f0..79b24baf7 100644
--- a/src/landscape.cpp
+++ b/src/landscape.cpp
@@ -96,13 +96,16 @@ static SnowLine *_snow_line = NULL;
  * @param x X viewport 2D coordinate.
  * @param y Y viewport 2D coordinate.
  * @param clamp_to_map Clamp the coordinate outside of the map to the closest, non-void tile within the map.
+ * @param[out] clamped Whether coordinates were clamped.
  * @return 3D world coordinate of point visible at the given screen coordinate (3D perspective).
  *
  * @note Inverse of #RemapCoords2 function. Smaller values may get rounded.
  * @see InverseRemapCoords
  */
-Point InverseRemapCoords2(int x, int y, bool clamp_to_map)
+Point InverseRemapCoords2(int x, int y, bool clamp_to_map, bool *clamped)
 {
+	if (clamped != NULL) *clamped = false; // Not clamping yet.
+
 	/* Initial x/y world coordinate is like if the landscape
 	 * was completely flat on height 0. */
 	Point pt = InverseRemapCoords(x, y);
@@ -116,8 +119,10 @@ Point InverseRemapCoords2(int x, int y, bool clamp_to_map)
 		 * of extra tiles. This is mostly due to the tiles on the north side of
 		 * the map possibly being drawn higher due to the extra height levels. */
 		int extra_tiles = CeilDiv(_settings_game.construction.max_heightlevel * TILE_HEIGHT, TILE_PIXELS);
+		Point old_pt = pt;
 		pt.x = Clamp(pt.x, -extra_tiles * TILE_SIZE, max_x);
 		pt.y = Clamp(pt.y, -extra_tiles * TILE_SIZE, max_y);
+		if (clamped != NULL) *clamped = (pt.x != old_pt.x) || (pt.y != old_pt.y);
 	}
 
 	/* Now find the Z-world coordinate by fix point iteration.
@@ -133,8 +138,10 @@ Point InverseRemapCoords2(int x, int y, bool clamp_to_map)
 	pt.x += z;
 	pt.y += z;
 	if (clamp_to_map) {
+		Point old_pt = pt;
 		pt.x = Clamp(pt.x, min_coord, max_x);
 		pt.y = Clamp(pt.y, min_coord, max_y);
+		if (clamped != NULL) *clamped = *clamped || (pt.x != old_pt.x) || (pt.y != old_pt.y);
 	}
 
 	return pt;
diff --git a/src/landscape.h b/src/landscape.h
index cf366e09c..d24d0d190 100644
--- a/src/landscape.h
+++ b/src/landscape.h
@@ -116,7 +116,7 @@ static inline Point InverseRemapCoords(int x, int y)
 	return pt;
 }
 
-Point InverseRemapCoords2(int x, int y, bool clamp_to_map = false);
+Point InverseRemapCoords2(int x, int y, bool clamp_to_map = false, bool *clamped = NULL);
 
 uint ApplyFoundationToSlope(Foundation f, Slope *s);
 /**
diff --git a/src/viewport.cpp b/src/viewport.cpp
index da1085161..7fe280a20 100644
--- a/src/viewport.cpp
+++ b/src/viewport.cpp
@@ -1631,121 +1631,33 @@ void Window::DrawViewport() const
 }
 
 /**
- * Continue criteria for the SearchMapEdge function.
- * @param iter       Value to check.
- * @param iter_limit Maximum value for the iter
- * @param sy         Screen y coordinate calculated for the tile at hand
- * @param sy_limit   Limit to the screen y coordinate
- * @return True when we should continue searching.
- */
-typedef bool ContinueMapEdgeSearch(int iter, int iter_limit, int sy, int sy_limit);
-
-/** Continue criteria for searching a no-longer-visible tile in negative direction, starting at some tile. */
-static inline bool ContinueLowerMapEdgeSearch(int iter, int iter_limit, int sy, int sy_limit) { return iter > 0          && sy > sy_limit; }
-/** Continue criteria for searching a no-longer-visible tile in positive direction, starting at some tile. */
-static inline bool ContinueUpperMapEdgeSearch(int iter, int iter_limit, int sy, int sy_limit) { return iter < iter_limit && sy < sy_limit; }
-
-/**
- * Searches, starting at the given tile, by applying the given offset to iter, for a no longer visible tile.
- * The whole sense of this function is keeping the to-be-written code small, thus it is a little bit abstracted
- * so the same function can be used for both the X and Y locations. As such a reference to one of the elements
- * in curr_tile was needed.
- * @param curr_tile  A tile
- * @param iter       Reference to either the X or Y of curr_tile.
- * @param iter_limit Upper search limit for the iter value.
- * @param offset     Search in steps of this size
- * @param sy_limit   Search limit to be passed to the criteria
- * @param continue_criteria Search as long as this criteria is true
- * @return The final value of iter.
- */
-static int SearchMapEdge(Point &curr_tile, int &iter, int iter_limit, int offset, int sy_limit, ContinueMapEdgeSearch continue_criteria)
-{
-	int sy;
-	do {
-		iter = Clamp(iter + offset, 0, iter_limit);
-		sy = GetViewportY(curr_tile);
-	} while (continue_criteria(iter, iter_limit, sy, sy_limit));
-
-	return iter;
-}
-
-/**
- * Determine the clamping of either the X or Y coordinate to the map.
- * @param curr_tile   A tile
- * @param iter        Reference to either the X or Y of curr_tile.
- * @param iter_limit  Upper search limit for the iter value.
- * @param start       Start value for the iteration.
- * @param other_ref   Reference to the opposite axis in curr_tile than of iter.
- * @param other_value Start value for of the opposite axis
- * @param vp_value    Value of the viewport location in the opposite axis as for iter.
- * @param other_limit Limit for the other value, so if iter is X, then other_limit is for Y.
- * @param vp_top      Top of the viewport.
- * @param vp_bottom   Bottom of the viewport.
- * @return Clamped version of vp_value.
+ * Ensure that a given viewport has a valid scroll position.
+ *
+ * There must be a visible piece of the map in the center of the viewport.
+ * If there isn't, the viewport will be scrolled to nearest such location.
+ *
+ * @param vp The viewport.
+ * @param[in,out] scroll_x Viewport X scroll.
+ * @param[in,out] scroll_y Viewport Y scroll.
  */
-static inline int ClampXYToMap(Point &curr_tile, int &iter, int iter_limit, int start, int &other_ref, int other_value, int vp_value, int other_limit, int vp_top, int vp_bottom)
+static inline void ClampViewportToMap(const ViewPort *vp, int *scroll_x, int *scroll_y)
 {
-	bool upper_edge = other_value < _settings_game.construction.max_heightlevel / 4;
+	/* Centre of the viewport is hot spot. */
+	Point pt = {
+		*scroll_x + vp->virtual_width / 2,
+		*scroll_y + vp->virtual_height / 2
+	};
 
-	/*
-	 * First get an estimate of the tiles relevant for us at that edge.  Relevant in the sense
-	 * "at least close to the visible area". Thus, we don't look at exactly each tile, inspecting
-	 * e.g. every tenth should be enough. After all, the desired screen limit is set such that
-	 * the bordermost tiles are painted in the middle of the screen when one hits the limit,
-	 * i.e. it is no harm if there is some small error in that calculation
-	 */
-
-	other_ref = upper_edge ? 0 : other_limit;
-	iter = start;
-	int min_iter = SearchMapEdge(curr_tile, iter, iter_limit, upper_edge ? -10 : +10, vp_top,    upper_edge ? ContinueLowerMapEdgeSearch : ContinueUpperMapEdgeSearch);
-	iter = start;
-	int max_iter = SearchMapEdge(curr_tile, iter, iter_limit, upper_edge ? +10 : -10, vp_bottom, upper_edge ? ContinueUpperMapEdgeSearch : ContinueLowerMapEdgeSearch);
+	/* Find nearest tile that is within borders of the map. */
+	bool clamped;
+	pt = InverseRemapCoords2(pt.x, pt.y, true, &clamped);
 
-	max_iter = min(max_iter + _settings_game.construction.max_heightlevel / 4, iter_limit);
-	min_iter = min(min_iter, max_iter);
-
-	/* Now, calculate the highest heightlevel of these tiles. Again just as an estimate. */
-	int max_heightlevel_at_edge = 0;
-	for (iter = min_iter; iter <= max_iter; iter += 10) {
-		max_heightlevel_at_edge = max(max_heightlevel_at_edge, (int)TileHeight(TileXY(curr_tile.x, curr_tile.y)));
+	if (clamped) {
+		/* Convert back to viewport coordinates and remove centering. */
+		pt = RemapCoords2(pt.x, pt.y);
+		*scroll_x = pt.x - vp->virtual_width / 2;
+		*scroll_y = pt.y - vp->virtual_height / 2;
 	}
-
-	/* Based on that heightlevel, calculate the limit. For the upper edge a tile with height zero would
-	 * get a limit of zero, on the other side it depends on the number of tiles along the axis. */
-	return upper_edge ?
-			max(vp_value, -max_heightlevel_at_edge * (int)(TILE_HEIGHT * 2 * ZOOM_LVL_BASE)) :
-			min(vp_value, (other_limit * TILE_SIZE * 4 - max_heightlevel_at_edge * TILE_HEIGHT * 2) * ZOOM_LVL_BASE);
-}
-
-static inline void ClampViewportToMap(const ViewPort *vp, int &x, int &y)
-{
-	int original_y = y;
-
-	/* Centre of the viewport is hot spot */
-	x += vp->virtual_width / 2;
-	y += vp->virtual_height / 2;
-
-	/* Convert viewport coordinates to map coordinates
-	 * Calculation is scaled by 4 to avoid rounding errors */
-	int vx = -x + y * 2;
-	int vy =  x + y * 2;
-
-	/* Find out which tile corresponds to (vx,vy) if one assumes height zero.  The cast is necessary to prevent C++ from
-	 * converting the result to an uint, which gives an overflow instead of a negative result... */
-	int tx = vx / (int)(TILE_SIZE * 4 * ZOOM_LVL_BASE);
-	int ty = vy / (int)(TILE_SIZE * 4 * ZOOM_LVL_BASE);
-
-	Point curr_tile;
-	vx = ClampXYToMap(curr_tile, curr_tile.y, MapMaxY(), ty, curr_tile.x, tx, vx, MapMaxX(), original_y, original_y + vp->virtual_height);
-	vy = ClampXYToMap(curr_tile, curr_tile.x, MapMaxX(), tx, curr_tile.y, ty, vy, MapMaxY(), original_y, original_y + vp->virtual_height);
-
-	/* Convert map coordinates to viewport coordinates */
-	x = (-vx + vy) / 2;
-	y = ( vx + vy) / 4;
-
-	/* Remove centering */
-	x -= vp->virtual_width / 2;
-	y -= vp->virtual_height / 2;
 }
 
 /**
@@ -1765,7 +1677,7 @@ void UpdateViewportPosition(Window *w)
 		SetViewportPosition(w, pt.x, pt.y);
 	} else {
 		/* Ensure the destination location is within the map */
-		ClampViewportToMap(vp, w->viewport->dest_scrollpos_x, w->viewport->dest_scrollpos_y);
+		ClampViewportToMap(vp, &w->viewport->dest_scrollpos_x, &w->viewport->dest_scrollpos_y);
 
 		int delta_x = w->viewport->dest_scrollpos_x - w->viewport->scrollpos_x;
 		int delta_y = w->viewport->dest_scrollpos_y - w->viewport->scrollpos_y;
@@ -1785,7 +1697,7 @@ void UpdateViewportPosition(Window *w)
 								w->viewport->scrollpos_y == w->viewport->dest_scrollpos_y);
 		}
 
-		ClampViewportToMap(vp, w->viewport->scrollpos_x, w->viewport->scrollpos_y);
+		ClampViewportToMap(vp, &w->viewport->scrollpos_x, &w->viewport->scrollpos_y);
 
 		SetViewportPosition(w, w->viewport->scrollpos_x, w->viewport->scrollpos_y);
 		if (update_overlay) RebuildViewportOverlay(w);