Fixed sparse and marker-overlapping labels on Volcano and Relative Abundance plots

pwiz_tools/Shared/zedgraph/ZedGraph/GraphObjList.cs

@@ -1,6 +1,6 @@
 //============================================================================
 //ZedGraph Class Library - A Flexible Line Graph/Bar Graph Library in C#
-//Copyright � 2004  John Champion
+//Copyright � 2004  John Champion
 //
 //This library is free software; you can redistribute it and/or
 //modify it under the terms of the GNU Lesser General Public
@@ -289,7 +289,9 @@ public bool FindPoint( PointF mousePt, PaneBase pane, Graphics g, float scaleFac
             // the same ZOrder value.
 			for ( int i=0; i<Count; i++ )
 			{
-				if ( this[i].PointInBox( mousePt, pane, g, scaleFactor ) )
+				// Skip hidden objects so an invisible object (e.g. a sampled-out or pruned
+				// label left in the list) cannot screen a visible one during hit-testing.
+				if ( this[i].IsVisible && this[i].PointInBox( mousePt, pane, g, scaleFactor ) )
 				{
 					if ( ( index >= 0 && this[i].ZOrder > this[index].ZOrder ) || index < 0 )
 						index = i;

pwiz_tools/Shared/zedgraph/ZedGraph/LabelLayout.cs

@@ -115,6 +115,26 @@ private void FillDensityGrid()
                 }
             }
 
+            foreach (var markerRect in EnumerateMarkerRectangles())
+            {
+                foreach (var cell in GetRectangleCells(markerRect))
+                {
+                    var intersect = RectangleF.Intersect(markerRect, cell.Bounds);
+                    if (intersect != RectangleF.Empty)
+                    {
+                        cell.Density += intersect.Height * intersect.Width;
+                        cell.PointCount++;
+                    }
+                }
+            }
+        }
+
+        /// <summary>
+        /// Enumerates the screen-pixel rectangles of every visible point marker on the graph.
+        /// Shared by the density grid and the overlap pruner.
+        /// </summary>
+        private IEnumerable<RectangleF> EnumerateMarkerRectangles()
+        {
             foreach (var line in GetMarkerLinesSnapshot())
             {
                 int pointCount;
@@ -129,26 +149,14 @@ private void FillDensityGrid()
 
                 for (var i = 0; i < pointCount; i++)
                 {
+                    RectangleF markerRect;
                     try
                     {
-                        if (!line.GetCoords(this._graph, i, out var coords))
-                        {
+                        if (!line.GetCoords(_graph, i, out var coords))
                             continue;
-                        }
 
                         var sides = Array.ConvertAll(coords.Split(','), int.Parse);
-                        var markerRect = new Rectangle(sides[0], sides[1], sides[2] - sides[0],
-                            sides[3] - sides[1]);
-
-                        foreach (var cell in GetRectangleCells(markerRect))
-                        {
-                            var intersect = RectangleF.Intersect(markerRect, cell.Bounds);
-                            if (intersect != Rectangle.Empty)
-                            {
-                                cell.Density += intersect.Height * intersect.Width;
-                                cell.PointCount++;
-                            }
-                        }
+                        markerRect = new RectangleF(sides[0], sides[1], sides[2] - sides[0], sides[3] - sides[1]);
                     }
                     catch (InvalidOperationException)
                     {
@@ -158,6 +166,7 @@ private void FillDensityGrid()
                     {
                         break;
                     }
+                    yield return markerRect;
                 }
             }
         }
@@ -707,6 +716,216 @@ public bool ApplyPlacements(LayoutResult result, Graphics g)
             return true;
         }
 
+        /// <summary>
+        /// Removes labels whose final (post-annealing) rectangles either overlap another kept
+        /// label or sit squarely on top of a foreign data point (cover its marker center). The
+        /// pre-annealing sampler targets a label density the annealer can usually satisfy, but on
+        /// very dense plots some labels still land on top of each other or on markers. This second
+        /// pass enforces those invariants on the actual placed rectangles. Selected labels are
+        /// always kept. The processing order is deterministic (selected first, then by a stable
+        /// hash of the label text) so repeated layouts of the same data prune the same labels.
+        /// Pruned labels are hidden, their connectors removed, and they are dropped from the
+        /// layout so they are not persisted.
+        /// </summary>
+        public void PruneOverlappingLabels(Graphics g)
+        {
+            if (_labeledPoints.Count == 0)
+                return;
+
+            var entries = new List<LabelRect>();
+            foreach (var labeledPoint in _labeledPoints.Values)
+            {
+                var rect = _graph.GetRectScreen(labeledPoint.Label, g);
+                if (rect.Width <= 0 || rect.Height <= 0)
+                    continue;
+                entries.Add(new LabelRect(labeledPoint, rect));
+            }
+            if (entries.Count == 0)
+                return;
+
+            // Selected labels first (never pruned), then a stable hash of the label text so the
+            // survivors do not depend on alphabetical ordering and two runs over the same data agree.
+            // The final tiebreaker on a position-independent identity keeps the order deterministic
+            // even when two points share the same label text (List.Sort is not a stable sort).
+            entries.Sort((a, b) =>
+            {
+                if (a.Point.IsSelected != b.Point.IsSelected)
+                    return a.Point.IsSelected ? -1 : 1;
+                var hashCompare = a.Hash.CompareTo(b.Hash);
+                if (hashCompare != 0)
+                    return hashCompare;
+                var textCompare = string.CompareOrdinal(a.Text, b.Text);
+                if (textCompare != 0)
+                    return textCompare;
+                return string.CompareOrdinal(a.TieBreak, b.TieBreak);
+            });
+
+            var bucketSize = 1;
+            foreach (var entry in entries)
+                bucketSize = Math.Max(bucketSize, (int)Math.Ceiling(Math.Max(entry.Rect.Width, entry.Rect.Height)));
+
+            // Spatial hash of point-marker centers so a label that sits squarely on top of a data
+            // point (covers its center) can be pruned. A label is allowed to overlap the marker of
+            // its own point - the connector already ties them together - so that one is excluded per
+            // candidate below.
+            var markerCentersByBucket = new Dictionary<long, List<PointF>>();
+            foreach (var markerRect in EnumerateMarkerRectangles())
+            {
+                if (markerRect.Width <= 0 || markerRect.Height <= 0)
+                    continue;
+                var center = new PointF(markerRect.X + markerRect.Width / 2f, markerRect.Y + markerRect.Height / 2f);
+                var key = BucketKey(center, bucketSize);
+                if (!markerCentersByBucket.TryGetValue(key, out var centers))
+                {
+                    centers = new List<PointF>();
+                    markerCentersByBucket[key] = centers;
+                }
+                centers.Add(center);
+            }
+
+            var keptByBucket = new Dictionary<long, List<RectangleF>>();
+            var pruned = new List<LabeledPoint>();
+            foreach (var entry in entries)
+            {
+                if (!entry.Point.IsSelected &&
+                    (OverlapsKept(entry.Rect, bucketSize, keptByBucket) ||
+                     CoversForeignMarker(entry, bucketSize, markerCentersByBucket)))
+                {
+                    pruned.Add(entry.Point);
+                    continue;
+                }
+                AddToBuckets(entry.Rect, bucketSize, keptByBucket);
+            }
+
+            foreach (var point in pruned)
+            {
+                point.Label.IsVisible = false;
+                if (point.Connector != null)
+                    _graph.GraphObjList.Remove(point.Connector);
+                _labeledPoints.Remove(point.Label);
+            }
+        }
+
+        // Distance (squared, in pixels) within which a marker center is treated as a label's own
+        // point rather than a foreign one it is sitting on top of.
+        private const float OWN_MARKER_TOLERANCE_SQ = 4f;
+
+        /// <summary>
+        /// Returns true if the label rectangle covers the center of any point marker other than the
+        /// marker of the label's own point.
+        /// </summary>
+        private bool CoversForeignMarker(LabelRect entry, int bucketSize, Dictionary<long, List<PointF>> markerCentersByBucket)
+        {
+            // Own-marker exclusion assumes a non-ordinal (Linear) X axis: ownCenter is transformed
+            // from the data value while the marker centers come from LineItem.GetCoords, which uses
+            // the ordinal index on Text/ordinal axes. Both plots that use this layout (volcano and
+            // relative abundance) have Linear X axes, so the two agree to sub-pixel precision.
+            var ownCenter = _graph.TransformCoord(entry.Point.Point.X, entry.Point.Point.Y, CoordType.AxisXYScale);
+            foreach (var bucket in BucketKeys(entry.Rect, bucketSize))
+            {
+                if (!markerCentersByBucket.TryGetValue(bucket, out var centers))
+                    continue;
+                foreach (var center in centers)
+                {
+                    if (!entry.Rect.Contains(center))
+                        continue;
+                    var dx = center.X - ownCenter.X;
+                    var dy = center.Y - ownCenter.Y;
+                    if (dx * dx + dy * dy <= OWN_MARKER_TOLERANCE_SQ)
+                        continue;
+                    return true;
+                }
+            }
+            return false;
+        }
+
+        private static long BucketKey(PointF pt, int bucketSize)
+        {
+            var bx = (int)Math.Floor(pt.X / bucketSize);
+            var by = (int)Math.Floor(pt.Y / bucketSize);
+            return ((long)bx << 32) ^ (uint)by;
+        }
+
+        private static bool OverlapsKept(RectangleF rect, int bucketSize, Dictionary<long, List<RectangleF>> keptByBucket)
+        {
+            foreach (var bucket in BucketKeys(rect, bucketSize))
+            {
+                if (!keptByBucket.TryGetValue(bucket, out var rects))
+                    continue;
+                foreach (var kept in rects)
+                {
+                    if (kept.IntersectsWith(rect))
+                        return true;
+                }
+            }
+            return false;
+        }
+
+        private static void AddToBuckets(RectangleF rect, int bucketSize, Dictionary<long, List<RectangleF>> keptByBucket)
+        {
+            foreach (var bucket in BucketKeys(rect, bucketSize))
+            {
+                if (!keptByBucket.TryGetValue(bucket, out var rects))
+                {
+                    rects = new List<RectangleF>();
+                    keptByBucket[bucket] = rects;
+                }
+                rects.Add(rect);
+            }
+        }
+
+        private static IEnumerable<long> BucketKeys(RectangleF rect, int bucketSize)
+        {
+            var minX = (int)Math.Floor(rect.Left / bucketSize);
+            var maxX = (int)Math.Floor(rect.Right / bucketSize);
+            var minY = (int)Math.Floor(rect.Top / bucketSize);
+            var maxY = (int)Math.Floor(rect.Bottom / bucketSize);
+            for (var bx = minX; bx <= maxX; bx++)
+            {
+                for (var by = minY; by <= maxY; by++)
+                    yield return ((long)bx << 32) ^ (uint)by;
+            }
+        }
+
+        private static uint StableTextHash(string text)
+        {
+            unchecked
+            {
+                const uint fnvOffset = 2166136261;
+                const uint fnvPrime = 16777619;
+                var hash = fnvOffset;
+                if (!string.IsNullOrEmpty(text))
+                {
+                    foreach (var ch in text)
+                    {
+                        hash ^= ch;
+                        hash *= fnvPrime;
+                    }
+                }
+                return hash;
+            }
+        }
+
+        private readonly struct LabelRect
+        {
+            public LabelRect(LabeledPoint point, RectangleF rect)
+            {
+                Point = point;
+                Rect = rect;
+                Text = point.Label?.Text ?? string.Empty;
+                Hash = StableTextHash(Text);
+                // Stable, position-independent identity used as the final sort tiebreaker so
+                // that points sharing the same label text still prune in a deterministic order.
+                TieBreak = point.UniqueID?.ToString() ?? string.Empty;
+            }
+
+            public LabeledPoint Point { get; }
+            public RectangleF Rect { get; }
+            public string Text { get; }
+            public uint Hash { get; }
+            public string TieBreak { get; }
+        }
+
         /// <summary>
         /// Calculates the total goal function for the current layout (all labels).
         /// </summary>

pwiz_tools/Skyline/Controls/Graphs/LabelLayoutRunner.cs

@@ -35,7 +35,7 @@ internal sealed class LabelLayoutRunner
         private const int LABEL_LAYOUT_PROGRESS_INTERVAL_MS = 200;
         private const int LABEL_LAYOUT_DEBOUNCE_MS = 100;
         private const double MAX_LABELS_PER_CELL = 4.0;
-        private const double MAX_LABEL_AREA_RATIO = 0.5;
+        private const double MAX_LABEL_AREA_RATIO = 0.3;
 
         private Thread _labelLayoutWorkerThread;
         private CancellationTokenSource _labelLayoutCts;
@@ -173,7 +173,7 @@ private static List<LabeledPoint> SamplePointsByDensityGrid(LabelLayout labelLay
                 var cellPointCount = labelLayout.CellFromPoint(cellEntry.Item2).PointCount;
                 var cutoff = cellPointCount > MAX_LABELS_PER_CELL ? MAX_LABELS_PER_CELL / cellPointCount : 1.0;
 
-                if (cellEntry.Item1.Point.IsSelected || (double)cellEntry.Item1.Hash / maxHash <= (cutoff * areaSamplingRate))
+                if (cellEntry.Item1.Point.IsSelected || (double)cellEntry.Item1.Hash / maxHash <= Math.Min(cutoff, areaSamplingRate))
                 {
                     keep.Add(cellEntry.Item1.Point);
                 }
@@ -455,6 +455,10 @@ private void StartDebounced(LabelLayoutRequest request)
                                 {
                                     if (pane.ApplyLabelLayout(workerResult.LabelLayout, workerResult.Result, g))
                                     {
+                                        // Remove any labels the annealer could not separate. The sampler
+                                        // targets a workable density, but the densest plots still leave
+                                        // overlaps that only show up in the final placed positions.
+                                        workerResult.LabelLayout.PruneOverlappingLabels(g);
                                         saveLayout?.Invoke(pane.Layout?.PointsLayout);
                                         invalidate?.Invoke();
                                     }

pwiz_tools/Skyline/TestFunctional/LabelLayoutTest.cs

@@ -35,14 +35,14 @@ namespace pwiz.SkylineTestFunctional
     public class LabelLayoutTest : AbstractFunctionalTestEx
     {
         private const float LABEL_TOLERANCE_PX = 2f;
-        private const int EXPECTED_POINT_COUNT = 14;
+        private const int EXPECTED_POINT_COUNT = 13;
         private static readonly ExpectedPointSnapshot[] EXPECTED_RANDOM_POINTS =
         {
             new ExpectedPointSnapshot(1, "EENGDFASFR", 55f, 545123.8f),
             new ExpectedPointSnapshot(4, "HEEEVERPAVEK", 86f, 191400.8f),
             new ExpectedPointSnapshot(7, "MLSGFIPLKPTVK", 98f, 138878.5f),
-            new ExpectedPointSnapshot(10, "TSDQIHFFFAK", 113f, 79212.3f),
-            new ExpectedPointSnapshot(13, "WTNPDGTTSK", 84f, 199644.5f),
+            new ExpectedPointSnapshot(9, "TSDQIHFFFAK", 113f, 79212.3f),
+            new ExpectedPointSnapshot(12, "WTNPDGTTSK", 84f, 199644.5f),
         };
 
         [TestMethod]