Refactored ScreenMap & improved perf of updates, removals and region lookups.

Reduce code duplication by extracting a common class to deal with spatial partitioning of actors, and use some (cached) delegates to reduce duplication further without affecting performance too much.

Speed up updates and removal of actors by caching their location so we only need to update or remove them from bins they are actually in (typically very few), compared to having to check every bin for removals which is much more work in comparison.

Speed up checking for actors inside a region by checking if items are located entirely within the bin they are located in. If so, we don't need to add them to the hash-set for de-duplication purposes which is fairly expensive.

OpenRA.Game/OpenRA.Game.csproj

@@ -143,6 +143,7 @@
     <Compile Include="Orders\UnitOrderGenerator.cs" />
     <Compile Include="Player.cs" />
     <Compile Include="Primitives\MergedStream.cs" />
+    <Compile Include="Primitives\SpatiallyPartitioned.cs" />
     <Compile Include="Selection.cs" />
     <Compile Include="Server\Connection.cs" />
     <Compile Include="Server\Exts.cs" />

OpenRA.Game/Primitives/SpatiallyPartitioned.cs

@@ -0,0 +1,108 @@
+#region Copyright & License Information
+/*
+ * Copyright 2007-2015 The OpenRA Developers (see AUTHORS)
+ * This file is part of OpenRA, which is free software. It is made
+ * available to you under the terms of the GNU General Public License
+ * as published by the Free Software Foundation. For more information,
+ * see COPYING.
+ */
+#endregion
+
+using System;
+using System.Collections.Generic;
+using System.Drawing;
+
+namespace OpenRA.Primitives
+{
+	public sealed class SpatiallyPartitioned<T>
+	{
+		readonly int rows, cols, binSize;
+		readonly Dictionary<T, Rectangle>[] itemBoundsBins;
+		readonly Dictionary<T, Rectangle> itemBounds = new Dictionary<T, Rectangle>();
+		readonly Action<Dictionary<T, Rectangle>, T, Rectangle> addItem = (bin, actor, bounds) => bin.Add(actor, bounds);
+		readonly Action<Dictionary<T, Rectangle>, T, Rectangle> removeItem = (bin, actor, bounds) => bin.Remove(actor);
+
+		public SpatiallyPartitioned(int width, int height, int binSize)
+		{
+			this.binSize = binSize;
+			rows = height / binSize + 1;
+			cols = width / binSize + 1;
+			itemBoundsBins = Exts.MakeArray(rows * cols, _ => new Dictionary<T, Rectangle>());
+		}
+
+		public void Add(T item, Rectangle bounds)
+		{
+			itemBounds.Add(item, bounds);
+			MutateBins(item, bounds, addItem);
+		}
+
+		public void Update(T item, Rectangle bounds)
+		{
+			MutateBins(item, itemBounds[item], removeItem);
+			MutateBins(item, itemBounds[item] = bounds, addItem);
+		}
+
+		public void Remove(T item)
+		{
+			MutateBins(item, itemBounds[item], removeItem);
+			itemBounds.Remove(item);
+		}
+
+		Dictionary<T, Rectangle> BinAt(int row, int col)
+		{
+			return itemBoundsBins[row * cols + col];
+		}
+
+		Rectangle BinBounds(int row, int col)
+		{
+			return new Rectangle(col * binSize, row * binSize, binSize, binSize);
+		}
+
+		void MutateBins(T actor, Rectangle bounds, Action<Dictionary<T, Rectangle>, T, Rectangle> action)
+		{
+			var top = Math.Max(0, bounds.Top / binSize);
+			var left = Math.Max(0, bounds.Left / binSize);
+			var bottom = Math.Min(rows - 1, bounds.Bottom / binSize);
+			var right = Math.Min(cols - 1, bounds.Right / binSize);
+
+			for (var row = top; row <= bottom; row++)
+				for (var col = left; col <= right; col++)
+					action(BinAt(row, col), actor, bounds);
+		}
+
+		public IEnumerable<T> At(int2 location)
+		{
+			var col = (location.X / binSize).Clamp(0, cols - 1);
+			var row = (location.Y / binSize).Clamp(0, rows - 1);
+			foreach (var kvp in BinAt(row, col))
+				if (kvp.Value.Contains(location))
+					yield return kvp.Key;
+		}
+
+		public IEnumerable<T> InBox(Rectangle box)
+		{
+			var left = (box.Left / binSize).Clamp(0, cols - 1);
+			var right = (box.Right / binSize).Clamp(0, cols - 1);
+			var top = (box.Top / binSize).Clamp(0, rows - 1);
+			var bottom = (box.Bottom / binSize).Clamp(0, rows - 1);
+
+			var items = new HashSet<T>();
+			for (var row = top; row <= bottom; row++)
+				for (var col = left; col <= right; col++)
+				{
+					var binBounds = BinBounds(row, col);
+					foreach (var kvp in BinAt(row, col))
+					{
+						var item = kvp.Key;
+						var bounds = kvp.Value;
+
+						// Return items that intersect the box. We also want to avoid returning the same item many times.
+						// If the item is contained wholly within this bin, we're good as we know it won't show up in any others.
+						// Otherwise it may appear in another bin. We use a set of seen items to avoid yielding it again.
+						if (bounds.IntersectsWith(box) && (binBounds.Contains(bounds) || items.Add(item)))
+							yield return item;
+					}
+				}
+		}
+	}
+}

OpenRA.Game/Traits/World/ScreenMap.cs

@@ -27,99 +27,71 @@ namespace OpenRA.Traits
 
 	public class ScreenMap : IWorldLoaded
 	{
-		ScreenMapInfo info;
+		static readonly IEnumerable<FrozenActor> NoFrozenActors = new FrozenActor[0];
+		readonly Func<FrozenActor, bool> frozenActorIsValid = fa => fa.IsValid;
+		readonly Func<Actor, bool> actorIsInWorld = a => a.IsInWorld;
+		readonly Cache<Player, SpatiallyPartitioned<FrozenActor>> partitionedFrozenActors;
+		readonly SpatiallyPartitioned<Actor> partitionedActors;
 		WorldRenderer worldRenderer;
-		Cache<Player, Dictionary<FrozenActor, Rectangle>[]> frozen;
-		Dictionary<Actor, Rectangle>[] actors;
-		int rows, cols;
 
 		public ScreenMap(World world, ScreenMapInfo info)
 		{
-			this.info = info;
 			var ts = Game.ModData.Manifest.TileSize;
-			cols = world.Map.MapSize.X * ts.Width / info.BinSize + 1;
+			var width = world.Map.MapSize.X * ts.Width;
-			rows = world.Map.MapSize.Y * ts.Height / info.BinSize + 1;
+			var height = world.Map.MapSize.Y * ts.Height;
-
+			partitionedFrozenActors = new Cache<Player, SpatiallyPartitioned<FrozenActor>>(
-			frozen = new Cache<Player, Dictionary<FrozenActor, Rectangle>[]>(InitializeFrozenActors);
+				_ => new SpatiallyPartitioned<FrozenActor>(width, height, info.BinSize));
-			actors = new Dictionary<Actor, Rectangle>[rows * cols];
+			partitionedActors = new SpatiallyPartitioned<Actor>(width, height, info.BinSize);
-			for (var j = 0; j < rows; j++)
-				for (var i = 0; i < cols; i++)
-					actors[j * cols + i] = new Dictionary<Actor, Rectangle>();
 		}
 
 		public void WorldLoaded(World w, WorldRenderer wr) { worldRenderer = wr; }
 
-		Dictionary<FrozenActor, Rectangle>[] InitializeFrozenActors(Player p)
+		Rectangle FrozenActorBounds(FrozenActor fa)
-		{
-			var f = new Dictionary<FrozenActor, Rectangle>[rows * cols];
-			for (var j = 0; j < rows; j++)
-				for (var i = 0; i < cols; i++)
-					f[j * cols + i] = new Dictionary<FrozenActor, Rectangle>();
-
-			return f;
-		}
-
-		public void Add(Player viewer, FrozenActor fa)
 		{
 			var pos = worldRenderer.ScreenPxPosition(fa.CenterPosition);
 			var bounds = fa.Bounds;
 			bounds.Offset(pos.X, pos.Y);
-
+			return bounds;
-			var top = Math.Max(0, bounds.Top / info.BinSize);
-			var left = Math.Max(0, bounds.Left / info.BinSize);
-			var bottom = Math.Min(rows - 1, bounds.Bottom / info.BinSize);
-			var right = Math.Min(cols - 1, bounds.Right / info.BinSize);
-
-			for (var j = top; j <= bottom; j++)
-				for (var i = left; i <= right; i++)
-					frozen[viewer][j * cols + i].Add(fa, bounds);
 		}
 
-		public void Remove(Player viewer, FrozenActor fa)
+		Rectangle ActorBounds(Actor a)
-		{
-			foreach (var bin in frozen[viewer])
-				bin.Remove(fa);
-		}
-
-		public void Add(Actor a)
 		{
 			var pos = worldRenderer.ScreenPxPosition(a.CenterPosition);
 			var bounds = a.Bounds;
 			bounds.Offset(pos.X, pos.Y);
-
+			return bounds;
-			var top = Math.Max(0, bounds.Top / info.BinSize);
-			var left = Math.Max(0, bounds.Left / info.BinSize);
-			var bottom = Math.Min(rows - 1, bounds.Bottom / info.BinSize);
-			var right = Math.Min(cols - 1, bounds.Right / info.BinSize);
-
-			for (var j = top; j <= bottom; j++)
-				for (var i = left; i <= right; i++)
-					actors[j * cols + i].Add(a, bounds);
 		}
 
-		public void Remove(Actor a)
+		public void Add(Player viewer, FrozenActor fa)
 		{
-			foreach (var bin in actors)
+			partitionedFrozenActors[viewer].Add(fa, FrozenActorBounds(fa));
-				bin.Remove(a);
+		}
+
+		public void Remove(Player viewer, FrozenActor fa)
+		{
+			partitionedFrozenActors[viewer].Remove(fa);
+		}
+
+		public void Add(Actor a)
+		{
+			partitionedActors.Add(a, ActorBounds(a));
 		}
 
 		public void Update(Actor a)
 		{
-			Remove(a);
+			partitionedActors.Update(a, ActorBounds(a));
-			Add(a);
+		}
+
+		public void Remove(Actor a)
+		{
+			partitionedActors.Remove(a);
 		}
 
-		public static readonly IEnumerable<FrozenActor> NoFrozenActors = new FrozenActor[0].AsEnumerable();
 		public IEnumerable<FrozenActor> FrozenActorsAt(Player viewer, int2 worldPx)
 		{
 			if (viewer == null)
 				return NoFrozenActors;
-
+			return partitionedFrozenActors[viewer].At(worldPx).Where(frozenActorIsValid);
-			var i = (worldPx.X / info.BinSize).Clamp(0, cols - 1);
-			var j = (worldPx.Y / info.BinSize).Clamp(0, rows - 1);
-			return frozen[viewer][j * cols + i]
-				.Where(kv => kv.Key.IsValid && kv.Value.Contains(worldPx))
-				.Select(kv => kv.Key);
 		}
 
 		public IEnumerable<FrozenActor> FrozenActorsAt(Player viewer, MouseInput mi)
@@ -129,11 +101,7 @@ namespace OpenRA.Traits
 
 		public IEnumerable<Actor> ActorsAt(int2 worldPx)
 		{
-			var i = (worldPx.X / info.BinSize).Clamp(0, cols - 1);
+			return partitionedActors.At(worldPx).Where(actorIsInWorld);
-			var j = (worldPx.Y / info.BinSize).Clamp(0, rows - 1);
-			return actors[j * cols + i]
-				.Where(kv => kv.Key.IsInWorld && kv.Value.Contains(worldPx))
-				.Select(kv => kv.Key);
 		}
 
 		public IEnumerable<Actor> ActorsAt(MouseInput mi)
@@ -141,51 +109,31 @@ namespace OpenRA.Traits
 			return ActorsAt(worldRenderer.Viewport.ViewToWorldPx(mi.Location));
 		}
 
+		static Rectangle RectWithCorners(int2 a, int2 b)
+		{
+			return Rectangle.FromLTRB(Math.Min(a.X, b.X), Math.Min(a.Y, b.Y), Math.Max(a.X, b.X), Math.Max(a.Y, b.Y));
+		}
+
 		public IEnumerable<Actor> ActorsInBox(int2 a, int2 b)
 		{
-			return ActorsInBox(Rectangle.FromLTRB(Math.Min(a.X, b.X), Math.Min(a.Y, b.Y), Math.Max(a.X, b.X), Math.Max(a.Y, b.Y)));
+			return ActorsInBox(RectWithCorners(a, b));
 		}
 
 		public IEnumerable<Actor> ActorsInBox(Rectangle r)
 		{
-			var left = (r.Left / info.BinSize).Clamp(0, cols - 1);
+			return partitionedActors.InBox(r).Where(actorIsInWorld);
-			var right = (r.Right / info.BinSize).Clamp(0, cols - 1);
-			var top = (r.Top / info.BinSize).Clamp(0, rows - 1);
-			var bottom = (r.Bottom / info.BinSize).Clamp(0, rows - 1);
-
-			var actorsChecked = new HashSet<Actor>();
-			for (var j = top; j <= bottom; j++)
-				for (var i = left; i <= right; i++)
-					foreach (var kvp in actors[j * cols + i])
-					{
-						var actor = kvp.Key;
-						if (actor.IsInWorld && kvp.Value.IntersectsWith(r) && actorsChecked.Add(actor))
-							yield return actor;
-					}
 		}
 
 		public IEnumerable<FrozenActor> FrozenActorsInBox(Player p, int2 a, int2 b)
 		{
-			return FrozenActorsInBox(p, Rectangle.FromLTRB(Math.Min(a.X, b.X), Math.Min(a.Y, b.Y), Math.Max(a.X, b.X), Math.Max(a.Y, b.Y)));
+			return FrozenActorsInBox(p, RectWithCorners(a, b));
 		}
 
 		public IEnumerable<FrozenActor> FrozenActorsInBox(Player p, Rectangle r)
 		{
-			var left = (r.Left / info.BinSize).Clamp(0, cols - 1);
+			if (p == null)
-			var right = (r.Right / info.BinSize).Clamp(0, cols - 1);
+				return NoFrozenActors;
-			var top = (r.Top / info.BinSize).Clamp(0, rows - 1);
+			return partitionedFrozenActors[p].InBox(r).Where(frozenActorIsValid);
-			var bottom = (r.Bottom / info.BinSize).Clamp(0, rows - 1);
-			var bins = frozen[p];
-
-			var actorsChecked = new HashSet<FrozenActor>();
-			for (var j = top; j <= bottom; j++)
-				for (var i = left; i <= right; i++)
-					foreach (var kvp in bins[j * cols + i])
-					{
-						var actor = kvp.Key;
-						if (actor.IsValid && kvp.Value.IntersectsWith(r) && actorsChecked.Add(actor))
-							yield return actor;
-					}
 		}
 	}
 }