From 8b12a4a7476dcd24de72381397e4158f27d94339 Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Sat, 9 Jan 2016 13:49:38 +0000 Subject: [PATCH 1/3] Add tests for SpatiallyPartitioned. --- .../OpenRA.Game/SpatiallyPartitionedTest.cs | 92 +++++++++++++++++++ OpenRA.Test/OpenRA.Test.csproj | 2 + 2 files changed, 94 insertions(+) create mode 100644 OpenRA.Test/OpenRA.Game/SpatiallyPartitionedTest.cs diff --git a/OpenRA.Test/OpenRA.Game/SpatiallyPartitionedTest.cs b/OpenRA.Test/OpenRA.Game/SpatiallyPartitionedTest.cs new file mode 100644 index 0000000000..4d89685c85 --- /dev/null +++ b/OpenRA.Test/OpenRA.Game/SpatiallyPartitionedTest.cs @@ -0,0 +1,92 @@ +#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.Drawing; +using NUnit.Framework; +using OpenRA.Primitives; + +namespace OpenRA.Test.OpenRA.Game +{ + [TestFixture] + class SpatiallyPartitionedTest + { + [TestCase(TestName = "SpatiallyPartitioned.At works")] + public void SpatiallyPartitionedAtTest() + { + var partition = new SpatiallyPartitioned(5, 5, 2); + + var a = new object(); + partition.Add(a, new Rectangle(0, 0, 1, 1)); + CollectionAssert.Contains(partition.At(new int2(0, 0)), a, "a is not present after add"); + CollectionAssert.DoesNotContain(partition.At(new int2(0, 1)), a, "a is present in the wrong location"); + CollectionAssert.DoesNotContain(partition.At(new int2(1, 0)), a, "a is present in the wrong location"); + + var b = new object(); + partition.Add(b, new Rectangle(1, 1, 2, 2)); + CollectionAssert.DoesNotContain(partition.At(new int2(0, 1)), b, "b is present in the wrong location"); + CollectionAssert.DoesNotContain(partition.At(new int2(1, 0)), b, "b is present in the wrong location"); + CollectionAssert.Contains(partition.At(new int2(1, 1)), b, "b is not present after add"); + CollectionAssert.Contains(partition.At(new int2(2, 2)), b, "b is not present after add"); + CollectionAssert.DoesNotContain(partition.At(new int2(2, 3)), b, "b is present in the wrong location"); + CollectionAssert.DoesNotContain(partition.At(new int2(3, 2)), b, "b is present in the wrong location"); + CollectionAssert.DoesNotContain(partition.At(new int2(3, 3)), b, "b is present in the wrong location"); + + partition.Update(b, new Rectangle(4, 4, 1, 1)); + CollectionAssert.Contains(partition.At(new int2(0, 0)), a, "a wrongly changed location when b was updated"); + CollectionAssert.Contains(partition.At(new int2(4, 4)), b, "b is not present at the new location in the extreme corner of the partition"); + CollectionAssert.DoesNotContain(partition.At(new int2(1, 1)), b, "b is still present at the old location after update"); + + partition.Remove(a); + CollectionAssert.DoesNotContain(partition.At(new int2(0, 0)), a, "a is still present after removal"); + CollectionAssert.Contains(partition.At(new int2(4, 4)), b, "b wrongly changed location when a was removed"); + } + + [TestCase(TestName = "SpatiallyPartitioned.InBox works")] + public void SpatiallyPartitionedInBoxTest() + { + var partition = new SpatiallyPartitioned(5, 5, 2); + + var a = new object(); + partition.Add(a, new Rectangle(0, 0, 1, 1)); + CollectionAssert.DoesNotContain(partition.InBox(new Rectangle(0, 0, 0, 0)), a, "Searching an empty area should not return a"); + CollectionAssert.Contains(partition.InBox(new Rectangle(0, 0, 1, 1)), a, "a is not present after add"); + CollectionAssert.DoesNotContain(partition.InBox(new Rectangle(0, 1, 1, 1)), a, "a is present in the wrong location"); + CollectionAssert.DoesNotContain(partition.InBox(new Rectangle(1, 0, 1, 1)), a, "a is present in the wrong location"); + + var b = new object(); + partition.Add(b, new Rectangle(1, 1, 2, 2)); + CollectionAssert.DoesNotContain(partition.InBox(new Rectangle(0, 1, 1, 1)), b, "b is present in the wrong location"); + CollectionAssert.DoesNotContain(partition.InBox(new Rectangle(1, 0, 1, 1)), b, "b is present in the wrong location"); + CollectionAssert.Contains(partition.InBox(new Rectangle(1, 1, 1, 1)), b, "b is not present after add"); + CollectionAssert.Contains(partition.InBox(new Rectangle(2, 2, 1, 1)), b, "b is not present after add"); + CollectionAssert.DoesNotContain(partition.InBox(new Rectangle(2, 3, 1, 1)), b, "b is present in the wrong location"); + CollectionAssert.DoesNotContain(partition.InBox(new Rectangle(3, 2, 1, 1)), b, "b is present in the wrong location"); + CollectionAssert.DoesNotContain(partition.InBox(new Rectangle(3, 3, 1, 1)), b, "b is present in the wrong location"); + + CollectionAssert.AreEquivalent(new[] { b }, partition.InBox(new Rectangle(1, 1, 1, 1)), + "Searching within a single partition bin did not return the correct result"); + CollectionAssert.AllItemsAreUnique(partition.InBox(new Rectangle(0, 0, 5, 5)), + "Searching the whole partition returned duplicates of some items"); + CollectionAssert.AreEquivalent(new[] { a, b }, partition.InBox(new Rectangle(0, 0, 5, 5)), + "Searching the whole partition did not return all items"); + CollectionAssert.AreEquivalent(new[] { a, b }, partition.InBox(new Rectangle(-10, -10, 25, 25)), + "Searching an area larger than the partition did not return all items"); + + partition.Update(b, new Rectangle(4, 4, 1, 1)); + CollectionAssert.Contains(partition.InBox(new Rectangle(0, 0, 1, 1)), a, "a wrongly changed location when b was updated"); + CollectionAssert.Contains(partition.InBox(new Rectangle(4, 4, 1, 1)), b, "b is not present at the new location in the extreme corner of the partition"); + CollectionAssert.DoesNotContain(partition.InBox(new Rectangle(1, 1, 1, 1)), b, "b is still present at the old location after update"); + + partition.Remove(a); + CollectionAssert.DoesNotContain(partition.InBox(new Rectangle(0, 0, 1, 1)), a, "a is still present after removal"); + CollectionAssert.Contains(partition.InBox(new Rectangle(4, 4, 1, 1)), b, "b wrongly changed location when a was removed"); + } + } +} diff --git a/OpenRA.Test/OpenRA.Test.csproj b/OpenRA.Test/OpenRA.Test.csproj index ec093e2fc8..0150735602 100644 --- a/OpenRA.Test/OpenRA.Test.csproj +++ b/OpenRA.Test/OpenRA.Test.csproj @@ -43,12 +43,14 @@ False + + From 8ff08d4e191060ef9dfacf8fcfdc922d2ff5cc2e Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Sat, 9 Jan 2016 15:29:49 +0000 Subject: [PATCH 2/3] Add IntegerDivisionRoundingAwayFromZero. This provides the mirror of the integer division operator, which rounds towards zero. --- OpenRA.Game/Exts.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/OpenRA.Game/Exts.cs b/OpenRA.Game/Exts.cs index 950a088ae6..3f06ada4a3 100644 --- a/OpenRA.Game/Exts.cs +++ b/OpenRA.Game/Exts.cs @@ -327,6 +327,15 @@ namespace OpenRA return root; } + public static int IntegerDivisionRoundingAwayFromZero(int dividend, int divisor) + { + int remainder; + var quotient = Math.DivRem(dividend, divisor, out remainder); + if (remainder == 0) + return quotient; + return quotient + (Math.Sign(dividend) == Math.Sign(divisor) ? 1 : -1); + } + public static string JoinWith(this IEnumerable ts, string j) { return string.Join(j, ts); From a15d1801e173df9eb89471508ae28acf27897159 Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Sat, 9 Jan 2016 15:52:29 +0000 Subject: [PATCH 3/3] Calculate better upper bounds in SpatiallyPartitioned. SpatiallyPartitioned was calculating larger that necessary bounding regions as this was simple to do and does not impact correctness. By being careful to calculate the upper bounds exactly we can avoid checking more bins than we need to during updates and queries. --- .../Primitives/SpatiallyPartitioned.cs | 34 +++++++++++-------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/OpenRA.Game/Primitives/SpatiallyPartitioned.cs b/OpenRA.Game/Primitives/SpatiallyPartitioned.cs index 6bc9833300..fe0a5710fe 100644 --- a/OpenRA.Game/Primitives/SpatiallyPartitioned.cs +++ b/OpenRA.Game/Primitives/SpatiallyPartitioned.cs @@ -25,8 +25,8 @@ namespace OpenRA.Primitives public SpatiallyPartitioned(int width, int height, int binSize) { this.binSize = binSize; - rows = height / binSize + 1; - cols = width / binSize + 1; + rows = Exts.IntegerDivisionRoundingAwayFromZero(height, binSize); + cols = Exts.IntegerDivisionRoundingAwayFromZero(width, binSize); itemBoundsBins = Exts.MakeArray(rows * cols, _ => new Dictionary()); } @@ -58,15 +58,21 @@ namespace OpenRA.Primitives return new Rectangle(col * binSize, row * binSize, binSize, binSize); } + void BoundsToBinRowsAndCols(Rectangle bounds, out int minRow, out int maxRow, out int minCol, out int maxCol) + { + minRow = Math.Max(0, bounds.Top / binSize); + minCol = Math.Max(0, bounds.Left / binSize); + maxRow = Math.Min(rows, Exts.IntegerDivisionRoundingAwayFromZero(bounds.Bottom, binSize)); + maxCol = Math.Min(cols, Exts.IntegerDivisionRoundingAwayFromZero(bounds.Right, binSize)); + } + void MutateBins(T actor, Rectangle bounds, Action, 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); + int minRow, maxRow, minCol, maxCol; + BoundsToBinRowsAndCols(bounds, out minRow, out maxRow, out minCol, out maxCol); - for (var row = top; row <= bottom; row++) - for (var col = left; col <= right; col++) + for (var row = minRow; row < maxRow; row++) + for (var col = minCol; col < maxCol; col++) action(BinAt(row, col), actor, bounds); } @@ -81,18 +87,16 @@ namespace OpenRA.Primitives public IEnumerable 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); + int minRow, maxRow, minCol, maxCol; + BoundsToBinRowsAndCols(box, out minRow, out maxRow, out minCol, out maxCol); // We want to return any items intersecting the box. // If the box covers multiple bins, we must handle items that are contained in multiple bins and avoid // returning them more than once. We shall use a set to track these. // PERF: If we are only looking inside one bin, we can avoid the cost of performing this tracking. - var items = top == bottom && left == right ? null : new HashSet(); - for (var row = top; row <= bottom; row++) - for (var col = left; col <= right; col++) + var items = minRow >= maxRow || minCol >= maxCol ? null : new HashSet(); + for (var row = minRow; row < maxRow; row++) + for (var col = minCol; col < maxCol; col++) { var binBounds = BinBounds(row, col); foreach (var kvp in BinAt(row, col))