Merge pull request #8824 from pchote/heightmap-clamp

Implement heightmap-aware bounds clamping.

OpenRA.Game/Map/Map.cs

@@ -739,10 +739,10 @@ namespace OpenRA
 
 		public bool Contains(MPos uv)
 		{
-			// TODO: Checking against the bounds excludes valid parts of the map if MaxTerrainHeight > 0.
+			// The first check ensures that the cell is within the valid map region, avoiding
-			// Unfortunatley, doing this properly leads to memory corruption issues in the (unsafe) radar
+			// potential crashes in deeper code.  All CellLayers have the same geometry, and
-			// rendering code.
+			// CustomTerrain is convenient (cellProjection may be null and others are Lazy).
-			return Bounds.Contains(uv.U, uv.V) && ProjectedCellsCovering(uv).All(containsTest);
+			return CustomTerrain.Contains(uv) && ProjectedCellsCovering(uv).All(containsTest);
 		}
 
 		public bool Contains(PPos puv)
@@ -933,12 +933,62 @@ namespace OpenRA
 
 		public MPos Clamp(MPos uv)
 		{
+			if (MaximumTerrainHeight == 0)
+				return (MPos)Clamp((PPos)uv);
+
 			// Already in bounds, so don't need to do anything.
-			if (Contains(uv))
+			if (ProjectedCellsCovering(uv).Any(containsTest))
 				return uv;
 
-			// TODO: Account for terrain height
+			// Clamping map coordinates is trickier than it might first look!
-			return (MPos)Clamp((PPos)uv);
+			// This needs to handle three nasty cases:
+			//  * The requested cell is well outside the map region
+			//  * The requested cell is near the top edge inside the map but outside the projected layer
+			//  * The clamped projected cell lands on a cliff face with no associated map cell
+			//
+			// Handling these cases properly requires abuse of our knowledge of the projection transform.
+			//
+			// The U coordinate doesn't change significantly in the projection, so clamp this
+			// straight away and ensure the point is somewhere inside the map
+			uv = cellProjection.Clamp(new MPos(uv.U.Clamp(Bounds.Left, Bounds.Right), uv.V));
+
+			// Project this guessed cell and take the first available cell
+			// If it is projected outside the layer, then make another guess.
+			var allProjected = ProjectedCellsCovering(uv);
+			var projected = allProjected.Any() ? allProjected.First()
+				: new PPos(uv.U, uv.V.Clamp(Bounds.Top, Bounds.Bottom));
+
+			// Clamp the projected cell to the map area
+			projected = Clamp(projected);
+
+			// Project the cell back into map coordinates.
+			// This may fail if the projected cell covered a cliff or another feature
+			// where there is a large change in terrain height.
+			var unProjected = Unproject(projected);
+			if (!unProjected.Any())
+			{
+				// Adjust V until we find a cell that works
+				for (var x = 2; x <= 2 * MaximumTerrainHeight; x++)
+				{
+					var dv = ((x & 1) == 1 ? 1 : -1) * x / 2;
+					var test = new PPos(projected.U, projected.V + dv);
+					if (!Contains(test))
+						continue;
+
+					unProjected = Unproject(test);
+					if (unProjected.Any())
+						break;
+				}
+
+				// This shouldn't happen.  But if it does, return the original value and hope the caller doesn't explode.
+				if (!unProjected.Any())
+				{
+					Log.Write("debug", "Failed to clamp map cell {0} to map bounds", uv);
+					return uv;
+				}
+			}
+
+			return projected.V == Bounds.Bottom ? unProjected.MaxBy(x => x.V) : unProjected.MinBy(x => x.V);
 		}
 
 		public PPos Clamp(PPos puv)

OpenRA.Mods.Common/Traits/World/TerrainGeometryOverlay.cs

@@ -98,7 +98,21 @@ namespace OpenRA.Mods.Common.Traits
 				}
 			}
 
-			lr.LineWidth = 1;
+			// Clamped cell
+			var clamped = map.Clamp(mouseCell);
+			{
+				var pos = map.CenterOfCell(clamped.ToCPos(map));
+				var tile = map.MapTiles.Value[clamped];
+				var ti = tileSet.GetTileInfo(tile);
+				var ramp = ti != null ? (int)ti.RampType : 0;
+
+				var screen = map.CellCorners[ramp].Select(c => wr.ScreenPxPosition(pos + c).ToFloat2()).ToArray();
+				for (var i = 0; i < 4; i++)
+				{
+					var j = (i + 1) % 4;
+					lr.DrawLine(screen[i], screen[j], Color.White);
+				}
+			}
 		}
 	}
 }

OpenRA.Mods.Common/Widgets/RadarWidget.cs

@@ -65,21 +65,13 @@ namespace OpenRA.Mods.Common.Widgets
 		{
 			base.Initialize(args);
 
-			var width = world.Map.Bounds.Width;
-			var height = world.Map.Bounds.Height;
-			var rb = RenderBounds;
-			previewScale = Math.Min(rb.Width * 1f / width, rb.Height * 1f / height);
-			previewOrigin = new int2((int)((rb.Width - previewScale * width) / 2), (int)((rb.Height - previewScale * height) / 2));
-			mapRect = new Rectangle(previewOrigin.X, previewOrigin.Y, (int)(previewScale * width), (int)(previewScale * height));
-
 			// The four layers are stored in a 2x2 grid within a single texture
-			radarSheet = new Sheet(new Size(2 * width, 2 * height).NextPowerOf2());
+			var s = world.Map.MapSize;
+			radarSheet = new Sheet(new Size(2 * s.X, 2 * s.Y).NextPowerOf2());
 			radarSheet.CreateBuffer();
 			radarData = radarSheet.GetData();
 
-			terrainSprite = new Sprite(radarSheet, new Rectangle(0, 0, width, height), TextureChannel.Alpha);
+			MapBoundsChanged();
-			shroudSprite = new Sprite(radarSheet, new Rectangle(width, 0, width, height), TextureChannel.Alpha);
-			actorSprite = new Sprite(radarSheet, new Rectangle(0, height, width, height), TextureChannel.Alpha);
 
 			// Set initial terrain data
 			foreach (var cell in world.Map.AllCells)
@@ -89,14 +81,27 @@ namespace OpenRA.Mods.Common.Widgets
 			world.Map.CustomTerrain.CellEntryChanged += UpdateTerrainCell;
 		}
 
+		void MapBoundsChanged()
+		{
+			var b = world.Map.Bounds;
+			var rb = RenderBounds;
+			previewScale = Math.Min(rb.Width * 1f / b.Width, rb.Height * 1f / b.Height);
+			previewOrigin = new int2((int)((rb.Width - previewScale * b.Width) / 2), (int)((rb.Height - previewScale * b.Height) / 2));
+			mapRect = new Rectangle(previewOrigin.X, previewOrigin.Y, (int)(previewScale * b.Width), (int)(previewScale * b.Height));
+
+			var s = world.Map.MapSize;
+			terrainSprite = new Sprite(radarSheet, b, TextureChannel.Alpha);
+			shroudSprite = new Sprite(radarSheet, new Rectangle(b.Location + new Size(s.X, 0), b.Size), TextureChannel.Alpha);
+			actorSprite = new Sprite(radarSheet, new Rectangle(b.Location + new Size(0, s.Y), b.Size), TextureChannel.Alpha);
+		}
+
 		void UpdateTerrainCell(CPos cell)
 		{
-			if (!world.Map.Contains(cell))
-				return;
-
-			var stride = radarSheet.Size.Width;
 			var uv = cell.ToMPos(world.Map);
 
+			if (!world.Map.CustomTerrain.Contains(uv))
+				return;
+
 			var custom = world.Map.CustomTerrain[uv];
 			Color color;
 			if (custom == byte.MaxValue)
@@ -107,27 +112,22 @@ namespace OpenRA.Mods.Common.Widgets
 			else
 				color = world.TileSet[custom].Color;
 
-			var dx = terrainSprite.Bounds.Left - world.Map.Bounds.Left;
+			var stride = radarSheet.Size.Width;
-			var dy = terrainSprite.Bounds.Top - world.Map.Bounds.Top;
 
 			unsafe
 			{
 				fixed (byte* colorBytes = &radarData[0])
 				{
 					var colors = (int*)colorBytes;
-					colors[(uv.V + dy) * stride + uv.U + dx] = color.ToArgb();
+					colors[uv.V * stride + uv.U] = color.ToArgb();
 				}
 			}
 		}
 
 		void UpdateShroudCell(PPos projectedCell)
 		{
-			if (!world.Map.Bounds.Contains(projectedCell.U, projectedCell.V))
-				return;
-
 			var stride = radarSheet.Size.Width;
-			var dx = shroudSprite.Bounds.Left - world.Map.Bounds.Left;
+			var dx = world.Map.MapSize.X;
-			var dy = shroudSprite.Bounds.Top - world.Map.Bounds.Top;
 
 			var color = 0;
 			var rp = world.RenderPlayer;
@@ -144,7 +144,7 @@ namespace OpenRA.Mods.Common.Widgets
 				fixed (byte* colorBytes = &radarData[0])
 				{
 					var colors = (int*)colorBytes;
-					colors[(projectedCell.V + dy) * stride + projectedCell.U + dx] = color;
+					colors[projectedCell.V * stride + projectedCell.U + dx] = color;
 				}
 			}
 		}
@@ -304,8 +304,7 @@ namespace OpenRA.Mods.Common.Widgets
 
 				// The actor layer is updated every tick
 				var stride = radarSheet.Size.Width;
-				var dx = actorSprite.Bounds.Left - world.Map.Bounds.Left;
+				var dy = world.Map.MapSize.Y;
-				var dy = actorSprite.Bounds.Top - world.Map.Bounds.Top;
 
 				Array.Clear(radarData, 4 * (actorSprite.Bounds.Top * stride + actorSprite.Bounds.Left), 4 * actorSprite.Bounds.Height * stride);
 
@@ -325,7 +324,7 @@ namespace OpenRA.Mods.Common.Widgets
 								var uv = cell.First.ToMPos(world.Map);
 
 								if (world.Map.Bounds.Contains(uv.U, uv.V))
-									colors[(uv.V + dy) * stride + uv.U + dx] = cell.Second.ToArgb();
+									colors[(uv.V + dy) * stride + uv.U] = cell.Second.ToArgb();
 							}
 						}
 					}