Duplicate sites to be explicitly marked as duplicate so any potential access exception actually explains this

Author: RudyTheDev

SharpVoronoiLib/Exceptions/VoronoiDoesntHaveSitesException.cs

@@ -2,11 +2,5 @@
 
 namespace SharpVoronoiLib.Exceptions;
 
-public class VoronoiDoesntHaveSitesException : Exception
-{
-    public VoronoiDoesntHaveSitesException()
-        : base("This data is not ready yet, you must add sites to the plane first.")
-    {
-            
-    }
-}
+public class VoronoiDoesntHaveSitesException() 
+    : Exception("This data is not ready yet, you must add sites to the plane first.");

SharpVoronoiLib/Exceptions/VoronoiNotTessellatedException.cs

@@ -2,11 +2,5 @@
 
 namespace SharpVoronoiLib.Exceptions;
 
-public class VoronoiNotTessellatedException : Exception
-{
-    public VoronoiNotTessellatedException()
-        : base("This data is not ready yet, you must tessellate the plane first.")
-    {
-            
-    }
-}
+public class VoronoiNotTessellatedException() 
+    : Exception("This data is not ready yet, you must tessellate the plane first.");

SharpVoronoiLib/Exceptions/VoronoiSiteSkippedAsDuplicateException.cs

@@ -0,0 +1,16 @@
+using System;
+
+namespace SharpVoronoiLib.Exceptions;
+
+public class VoronoiSiteSkippedAsDuplicateException() 
+    : Exception("This site was skipped during tessellation because it duplicates another site. " +
+                "If sites are not generated by the library and may contain duplicates, " +
+                "you must handle duplicates yourself and either not generate duplicates or skip these sites afterwards. " +
+                "Note that handling duplicate sites is not well-defined for tesselation algorithms in practice. " +
+                "For the purpose of this library and from standard practice, duplicate sites can be considered collapsed/merged with their original.");
+
+// Note that we can theoretically include the same cell data for a duplicate site as for the original site.
+// A purely mathematical result is possible - duplicate sites would have the same cell.
+// But this would require the implementation to actually handle duplicate cells, edges, points, etc.
+// In other words, if the implementation does not handle initial duplicates, then handling duplicates later is way more complicated.
+// So the default practice for libraries dealing with this is to simply skip/merge duplicates.

SharpVoronoiLib/Nearest Site Lookup/BruteForceNearestSiteLookup.cs

@@ -1,5 +1,4 @@
-using System;
-using System.Collections.Generic;
+using System.Collections.Generic;
 
 namespace SharpVoronoiLib;
 

SharpVoronoiLib/Tessellation/Fortune/FortunesTessellation.cs

@@ -15,7 +15,10 @@ public List<VoronoiEdge> Run(List<VoronoiSite> sites, double minX, double minY,
         foreach (VoronoiSite site in sites)
         {
             if (!siteCache.Add(site))
+            {
+                site.MarkSkippedAsDuplicate();
                 continue;
+            }
 
             if (site == null) throw new ArgumentNullException(nameof(sites));
 

SharpVoronoiLib/Tessellation/Fortune/MinHeap.cs

@@ -1,5 +1,4 @@
 using System;
-using System.Diagnostics;
 
 namespace SharpVoronoiLib;
 

SharpVoronoiLib/Voronoi/VoronoiSite.cs

@@ -22,12 +22,19 @@ public class VoronoiSite
 
     /// <summary>
     /// The state of this site.
-    /// If may be untesselated, if the algorith hasn't been run yet.
-    /// Or it may be skipped if it's a duplicate to another site.
+    /// It may be untesselated, if the algorithm hasn't been run yet.
+    /// Or it may be skipped if it's a duplicate to another site, in which case <see cref="SkippedAsDuplicate"/> will be true.
     /// </summary>
     [PublicAPI]
     public bool Tesselated => _tessellated;
 
+    /// <summary>
+    /// When not <see cref="Tesselated"/>, but the main <see cref="VoronoiPlane"/> is,
+    /// this may be true if tessellation skipped this site because it duplicates another site.
+    /// </summary>
+    [PublicAPI]
+    public bool SkippedAsDuplicate => _skippedAsDuplicate;
+
     /// <summary>
     /// The edges that make up this cell.
     /// The vertices of these edges are the <see cref="Points"/>.
@@ -38,8 +45,7 @@ public IEnumerable<VoronoiEdge> Cell
     {
         get
         {
-            if (!_tessellated)
-                throw new VoronoiNotTessellatedException();
+            ThrowIfUnavailable();
 
             return cell;
         }
@@ -54,8 +60,7 @@ public IEnumerable<VoronoiEdge> ClockwiseCell
     {
         get
         {
-            if (!_tessellated)
-                throw new VoronoiNotTessellatedException();
+            ThrowIfUnavailable();
 
             if (_clockwiseCell == null)
             {
@@ -75,8 +80,7 @@ public IEnumerable<VoronoiSite> Neighbours
     {
         get
         {
-            if (!_tessellated)
-                throw new VoronoiNotTessellatedException();
+            ThrowIfUnavailable();
 
             return neighbours;
         }
@@ -90,8 +94,7 @@ public IEnumerable<VoronoiPoint> Points
     {
         get
         {
-            if (!_tessellated)
-                throw new VoronoiNotTessellatedException();
+            ThrowIfUnavailable();
 
             if (_points == null)
             {
@@ -128,8 +131,7 @@ public IEnumerable<VoronoiPoint> ClockwisePoints
     {
         get
         {
-            if (!_tessellated)
-                throw new VoronoiNotTessellatedException();
+            ThrowIfUnavailable();
 
             if (_clockwisePoints == null)
             {
@@ -151,8 +153,7 @@ public VoronoiEdge? LiesOnEdge
     {
         get
         {
-            if (!_tessellated)
-                throw new VoronoiNotTessellatedException();
+            ThrowIfUnavailable();
 
             return _liesOnEdge;
         }
@@ -167,8 +168,7 @@ public VoronoiPoint? LiesOnCorner
     {
         get
         {
-            if (!_tessellated)
-                throw new VoronoiNotTessellatedException();
+            ThrowIfUnavailable();
 
             return _liesOnCorner;
         }
@@ -184,8 +184,7 @@ public VoronoiPoint Centroid
     {
         get
         {
-            if (!_tessellated)
-                throw new VoronoiNotTessellatedException();
+            ThrowIfUnavailable();
 
             if (_centroid != null)
                 return _centroid;
@@ -202,6 +201,7 @@ public VoronoiPoint Centroid
 
 
     private bool _tessellated;
+    private bool _skippedAsDuplicate;
 
     private List<VoronoiPoint>? _points;
     private List<VoronoiPoint>? _clockwisePoints;
@@ -231,8 +231,7 @@ public VoronoiSite(double x, double y)
     [PublicAPI]
     public bool Contains(double x, double y)
     {
-        if (!_tessellated)
-            throw new VoronoiNotTessellatedException();
+        ThrowIfUnavailable();
 
         // If we don't have points generated yet, do so now (by calling the property that does so when read)
         if (_clockwisePoints == null)
@@ -313,6 +312,9 @@ internal void Relocate(double newX, double newY)
 
         // We are no longer part of voronoi
         _tessellated = false;
+        
+        // We are not skipped as duplicate anymore as we have not been tessellated yet
+        _skippedAsDuplicate = false;
 
         // Clear all the values we used before
 
@@ -326,6 +328,10 @@ internal void Relocate(double newX, double newY)
         _centroid = null;
     }
 
+    internal void MarkSkippedAsDuplicate()
+    {
+        _skippedAsDuplicate = true;
+    }
 
     [Pure]
     private static int SortPointsClockwise(VoronoiPoint point1, VoronoiPoint point2, double x, double y)
@@ -557,11 +563,24 @@ internal void Invalidated()
         InvalidateComputedValues();
     }
 
+    
+    private void ThrowIfUnavailable()
+    {
+        if (!_tessellated)
+        {
+            if (_skippedAsDuplicate)
+                throw new VoronoiSiteSkippedAsDuplicateException();
+            
+            throw new VoronoiNotTessellatedException();
+        }
+    }
+
 
 #if DEBUG
     public override string ToString()
     {
         return "(" + X.ToString("F3") + "," + Y.ToString("F3") + ")";
     }
 #endif
-}
+}
+

UnitTests/BugReproTests.cs

@@ -1,4 +1,3 @@
-using System;
 using NUnit.Framework;
 using System.Collections.Generic;
 

UnitTests/ExceptionTest.cs

@@ -27,7 +27,7 @@ public void AccessPlaneEdgesBeforeTesselate()
 
         VoronoiPlane plane = new VoronoiPlane(0, 0, 600, 600);
 
-        plane.SetSites(new List<VoronoiSite>());
+        plane.SetSites([ ]);
 
         // Act - Assert
 
@@ -41,7 +41,7 @@ public void AccessSiteDataBeforeTesselate()
 
         VoronoiPlane plane = new VoronoiPlane(0, 0, 600, 600);
 
-        List<VoronoiSite> sites = new List<VoronoiSite>() { new VoronoiSite(100, 100) };
+        List<VoronoiSite> sites = [ new VoronoiSite(100, 100) ];
 
         plane.SetSites(sites);
 
@@ -57,4 +57,40 @@ public void AccessSiteDataBeforeTesselate()
         Assert.Throws<VoronoiNotTessellatedException>(() => _ = sites[0].Centroid);
         Assert.Throws<VoronoiNotTessellatedException>(() => _ = sites[0].Contains(100, 100));
     }
+    
+    [Test]
+    public void DuplicateSiteAccess()
+    {
+        // Arrange
+
+        VoronoiPlane plane = new VoronoiPlane(0, 0, 600, 600);
+        List<VoronoiSite> sites =
+        [
+            new VoronoiSite(100, 100),
+            new VoronoiSite(100, 100)
+        ];
+        plane.SetSites(sites);
+
+        // Act
+
+        plane.Tessellate();
+
+        // Assert
+
+        Assert.That(sites[0].Tesselated, Is.True);
+        Assert.That(sites[0].SkippedAsDuplicate, Is.False);
+
+        Assert.That(sites[1].Tesselated, Is.False);
+        Assert.That(sites[1].SkippedAsDuplicate, Is.True);
+
+        Assert.Throws<VoronoiSiteSkippedAsDuplicateException>(() => _ = sites[1].Cell);
+        Assert.Throws<VoronoiSiteSkippedAsDuplicateException>(() => _ = sites[1].ClockwiseCell);
+        Assert.Throws<VoronoiSiteSkippedAsDuplicateException>(() => _ = sites[1].Neighbours);
+        Assert.Throws<VoronoiSiteSkippedAsDuplicateException>(() => _ = sites[1].Points);
+        Assert.Throws<VoronoiSiteSkippedAsDuplicateException>(() => _ = sites[1].ClockwisePoints);
+        Assert.Throws<VoronoiSiteSkippedAsDuplicateException>(() => _ = sites[1].LiesOnEdge);
+        Assert.Throws<VoronoiSiteSkippedAsDuplicateException>(() => _ = sites[1].LiesOnCorner);
+        Assert.Throws<VoronoiSiteSkippedAsDuplicateException>(() => _ = sites[1].Centroid);
+        Assert.Throws<VoronoiSiteSkippedAsDuplicateException>(() => _ = sites[1].Contains(100, 100));
+    }
 }

UnitTests/LloydsAlgorithmTest.cs

@@ -1,5 +1,4 @@
 using System.Collections.Generic;
-using System.Linq;
 using NUnit.Framework;
 
 namespace SharpVoronoiLib.UnitTests;