Fix for the label clipping in the label layout algorithm

pwiz_tools/Shared/zedgraph/ZedGraph/GraphPane.cs

@@ -146,6 +146,10 @@ public class GraphPane : PaneBase, ICloneable, ISerializable
 
 		private LabelLayout _labelLayout;
 
+		/// <summary>
+		/// Indicates if this graph pane has the LabelLayout object attached.
+		/// Setting it to false removes the layout object.
+		/// </summary>
 		public bool EnableLabelLayout
 		{
 			get => _labelLayout != null;
@@ -1561,7 +1565,10 @@ public void AdjustLabelSpacings(List<LabeledPoint> labPoints, List<LabeledPoint.
                         labeledPoint.Label.IsVisible = true;
                     }
                     else
+                    {
                         labeledPoint.Label.IsVisible = false;
+                        GraphObjList.Remove(labeledPoint.Connector);
+                    }
                 }
 
                 if (visiblePoints.Any())

pwiz_tools/Shared/zedgraph/ZedGraph/LabelLayout.cs

@@ -78,6 +78,7 @@ private class GridCell
             public float _density;
             // public PointF _gradient;
             public static Dictionary<Color, Brush> _brushes = new Dictionary<Color, Brush>();
+            public Point _indices;
         }
 
         // First index row, second index line
@@ -100,6 +101,7 @@ private void FillDensityGrid()
                     {
                         _location = location,
                         _bounds = new RectangleF(location, new SizeF(_cellSize, _cellSize)),
+                        _indices = new Point(j, i)
                     };
                 }
             }
@@ -129,15 +131,40 @@ private void FillDensityGrid()
             }
         }
 
+        private bool GetPointMarkerRectangle(PointF pt, out RectangleF rect)
+        {
+            rect = RectangleF.Empty;
+            foreach (var line in _graph.CurveList.OfType<LineItem>().Where(c => c.Symbol.Type != SymbolType.None))
+            {
+                for (var i = 0; i < line.Points.Count; i++)
+                {
+                    var screenPt = _graph.TransformCoord(line.Points[i].X, line.Points[i].Y, CoordType.AxisXYScale);
+                    if (Math.Abs(screenPt.X - pt.X) < 1 && Math.Abs(screenPt.Y - pt.Y) < 1 )
+
+                    {
+                        if (!line.GetCoords(this._graph, i, out var coords))
+                        {
+                            continue;
+                        }
+                        var sides = Array.ConvertAll(coords.Split(','), int.Parse);
+                        rect = new Rectangle(sides[0], sides[1], sides[2] - sides[0], sides[3] - sides[1]);
+                        return true;
+                    }                }
+            }
+            return false;
+        }
+
         /// <summary>
         /// Calculates goal function for a labeled point and a suggested label position.
         /// All coordinates are in screen pixels.
         /// </summary>
         /// <param name="pt">Center of the label box, in pixels</param>
-        /// <param name="targetPoint">point being labeled.</param>
+        /// <param name="targetPoint">data point being labeled.</param>
         /// <param name="labelSize"> in pixels </param>
+        /// <param name="targetMarkerRect"> enclosing rectangle of the target point marker. We want to avoid
+        /// overlaps with it as much as possible.</param>
         /// <returns>goal function value.</returns>
-        private float GoalFuncion(PointF pt, PointF targetPoint, SizeF labelSize)
+        private float GoalFunction(PointF pt, PointF targetPoint, SizeF labelSize, RectangleF targetMarkerRect)
         {
             var pathCellCoord = CellIndexesFromXY(targetPoint);
             if (!IndexesWithinGrid(pathCellCoord))
@@ -147,12 +174,16 @@ private float GoalFuncion(PointF pt, PointF targetPoint, SizeF labelSize)
             // Distance to the label is measured from the center or right/left ends, whichever is closer.
             var distPoints = new[]
                 { pt - new SizeF(labelSize.Width / 2, 0), pt, pt + new SizeF(labelSize.Width / 2, 0) };
+            var graphWidth = _graph.Chart.Rect.Width;
+            var graphHeight = _graph.Chart.Rect.Height;
             var dist = distPoints.Min(p =>
             {
                 var diff = new SizeF(p.X - targetPoint.X, p.Y - targetPoint.Y);
-                return diff.Width * diff.Width + diff.Height * diff.Height;
+                // calculate the distance relative to the chart size to make sure it works 
+                // for both large and small graphs
+                return diff.Width * diff.Width / (graphWidth * graphWidth) + diff.Height * diff.Height / (graphHeight * graphHeight);
             });
-            var rect = new RectangleF(pt.X - labelSize.Width / 2, pt.Y - labelSize.Height / 2, labelSize.Width,
+            var rect = new RectangleF(pt.X - labelSize.Width / 2, pt.Y, labelSize.Width,
                 labelSize.Height);
             var totalOverlap = 0.0;
             foreach (var cell in GetRectangleCells(rect))
@@ -161,6 +192,9 @@ private float GoalFuncion(PointF pt, PointF targetPoint, SizeF labelSize)
                 totalOverlap += 1.0 * intersect.Height * intersect.Width / (_cellSize * _cellSize) * cell._density;
             }
 
+            // overlap with the target point is bad, we should penalize it heavily
+            if (RectangleF.Intersect(rect, targetMarkerRect) != RectangleF.Empty)
+                totalOverlap += 1500;
             // penalize this point if there is more points between it and its label
             // we find the cells between the two points by traversing the vector intersection
             // with the grid
@@ -218,7 +252,13 @@ private float GoalFuncion(PointF pt, PointF targetPoint, SizeF labelSize)
                     penalty += 2000;
             }
 
-            return (float)((0.025 * dist + totalOverlap) + penalty + 0.2 * pathDensity);
+            // penalize the goal if the label is completely or partially outside of the chart area
+            var visibleArea = RectArea(RectangleF.Intersect(rect, _graph.Chart.Rect));
+            var clipPenalty = 0.0f;
+            if (visibleArea > 0)
+                clipPenalty = (1 - visibleArea / RectArea(rect)) * 500.0f;
+
+            return (float)((20000 * dist + totalOverlap) + penalty + 0.2 * pathDensity) + clipPenalty;
         }
 
         private IEnumerable<GridCell> GetRectangleCells(RectangleF rect)
@@ -256,16 +296,24 @@ private bool IndexesWithinGrid(Point pt)
             return pt.X >= 0 && pt.X < _densityGridSize.Width && pt.Y >= 0 && pt.Y < _densityGridSize.Height;
         }
 
+        /// <summary>
+        /// Uniform distribution searches the available area more efficiently.
+        /// </summary>
         private int GetRandom(float range)
         {
-            return (int)((_randGenerator.NextDouble() - 0.5) * (_randGenerator.NextDouble() - 0.5) * range * 1.5);
+            return (int)((_randGenerator.NextDouble() - 0.5) * range * 0.75);
         }
 
         private static Rectangle ToRectangle(RectangleF rect)
         {
             return new Rectangle((int)rect.X, (int)rect.Y, (int)rect.Width, (int)rect.Height);
         }
 
+        private float RectArea(RectangleF rect) { return rect.Width * rect.Height; }
+
+        public const int SEARCH_COUNT_COARSE = 80;
+        public const int SEARCH_COUNT_FINE = 15;
+
         /**
          * Algorighm overview:
          * Divide the graph into a grid with cell size equals the label height (the smallest dimension).
@@ -276,23 +324,41 @@ private static Rectangle ToRectangle(RectangleF rect)
          * of the label relative to it's data point.
          *  The algorighm works in screen coordinates (pixels). There is no need to use user coordinates here.
          *  Returns true if the label has been successfully placed, false otherwise.
+         * Note that TextObj location is top-center, not top-left
          */
         public bool PlaceLabel(LabeledPoint labPoint, Graphics g)
         {
             var labelRect = _graph.GetRectScreen(labPoint.Label, g);
+            // do not attempt placement if the chart is too small
+            if (labelRect.Height > _graph.Chart.Rect.Height)
+                return false;
+            if ((labelRect.Width / 2) > _graph.Chart.Rect.Width)
+                return false;
+
             var targetPoint = _graph.TransformCoord(labPoint.Point.X, labPoint.Point.Y, CoordType.AxisXYScale);
-            var labelLength = (int)Math.Ceiling(1.0 * labelRect.Width / _cellSize);
+            var labelLength = (int)Math.Ceiling(1.0 * labelRect.Width / _cellSize); // label length in grid units
 
             var pointCell = new Point((int)((targetPoint.X - _chartOffset.X) / _cellSize),
                 (int)((targetPoint.Y - _chartOffset.Y) / _cellSize));
             if (!new Rectangle(Point.Empty, _densityGridSize).Contains(pointCell))
                 return false;
             var goal = float.MaxValue;
             var goalCell = Point.Empty;
-            var gridRect = new Rectangle(Point.Empty, _densityGridSize);
+            var gridRect = Rectangle.Empty;
+            // 4 is more or less arbitrary here, just to avoid search area 1 cell wide and make the search more efficient.
+            if (labelLength < _densityGridSize.Width - 4)
+                gridRect = new Rectangle(labelLength / 2 + 1, 0, _densityGridSize.Width - labelLength, _densityGridSize.Height - 1);
+            else 
+                gridRect = new Rectangle(labelLength / 2 + 1, 0, _densityGridSize.Width - labelLength/2, _densityGridSize.Height - 1);
             var points = new List<Point>();
-            for (var count = 80; count > 0; count--)
+
+            GetPointMarkerRectangle(targetPoint, out var targetMarkerRect);
+            var totalCount = SEARCH_COUNT_COARSE * 5;
+            for (var count = SEARCH_COUNT_COARSE; count > 0; count--)
             {
+                // make sure we are not stuck in this loop if the search area is exhausted.
+                 if (totalCount-- <= 0)
+                     break;
                 var randomGridPoint = pointCell +
                                       new Size(GetRandom(_densityGridSize.Width), GetRandom(_densityGridSize.Height));
 
@@ -308,7 +374,7 @@ public bool PlaceLabel(LabeledPoint labPoint, Graphics g)
                 }
 
                 points.Add(randomGridPoint);
-                var goalEstimate = GoalFuncion(CellFromPoint(randomGridPoint)._location, targetPoint, labelRect.Size);
+                var goalEstimate = GoalFunction(CellFromPoint(randomGridPoint)._location, targetPoint, labelRect.Size, targetMarkerRect);
                 if (goalEstimate < goal)
                 {
                     goal = goalEstimate;
@@ -319,12 +385,24 @@ public bool PlaceLabel(LabeledPoint labPoint, Graphics g)
             var roughGoal = goal;
             // Search the cell neighborhood for a better position
             var goalPoint = _densityGrid[goalCell.Y][goalCell.X]._location;
-            for (var count = 15; count > 0; count--)
+            var chartRect = _graph.Chart.Rect;
+            var allowedRect = RectangleF.Empty;
+            if (chartRect.Width > labelRect.Width * 1.2)
+            {
+                allowedRect = new RectangleF(chartRect.X + labelRect.Width / 2, chartRect.Y,
+                    chartRect.Width - labelRect.Width, chartRect.Height - labelRect.Height);
+            }
+            else
+            {
+                allowedRect = new RectangleF(chartRect.X + labelRect.Width / 2, chartRect.Y,
+                    chartRect.Width - labelRect.Width / 2, chartRect.Height - labelRect.Height);
+            }
+            for (var count = SEARCH_COUNT_FINE; count > 0; count--)
             {
                 var p = goalPoint + new Size(GetRandom(_cellSize * 2), GetRandom(_cellSize * 2));
-                if (!_graph.Chart.Rect.Contains(p))
+                if (!allowedRect.Contains(p))   // label should not overlap chart's borders
                     continue;
-                var goalEstimate1 = GoalFuncion(p, targetPoint, labelRect.Size);
+                var goalEstimate1 = GoalFunction(p, targetPoint, labelRect.Size, targetMarkerRect);
                 if (goalEstimate1 < goal)
                 {
                     goal = goalEstimate1;
@@ -354,6 +432,21 @@ public bool PlaceLabel(LabeledPoint labPoint, Graphics g)
             return true;
         }
 
+        // mostly for debugging support
+        public float CalculateGoalFunction(LabeledPoint labPoint)
+        {
+            if (labPoint == null)
+                return 0;
+            var targetPoint = _graph.TransformCoord(labPoint.Point.X, labPoint.Point.Y, CoordType.AxisXYScale);
+            var hasTargetMarker = GetPointMarkerRectangle(targetPoint, out var targetMarkerRect);
+            RectangleF labelRect;
+            using (var g = Graphics.FromHwnd(IntPtr.Zero))
+                labelRect = _graph.GetRectScreen(labPoint.Label, g);
+            var labScreenCoords = _graph.TransformCoord(labPoint.Label.Location.X, labPoint.Label.Location.Y, CoordType.AxisXYScale);
+            labScreenCoords = new PointF(labScreenCoords.X, labScreenCoords.Y - labelRect.Height/2);
+            return GoalFunction(labScreenCoords, targetPoint, labelRect.Size, targetMarkerRect);
+        }
+
         /// <summary>
         /// Places the label at the specified coordinates and updates the density grid so that
         /// the future calls to PlaceLabel take avoid overlaps and crossovers with this label

pwiz_tools/Shared/zedgraph/ZedGraph/ZedGraphControl.Events.cs

@@ -19,6 +19,7 @@
 
 using System;
 using System.ComponentModel;
+using System.Diagnostics;
 using System.Drawing;
 using System.Drawing.Drawing2D;
 using System.Windows.Forms;
@@ -1232,7 +1233,9 @@ private void HandleLabelDrag(Point mousePt)
 			_dragText.UpdateLabelLocation(xScale.AddInterval(_dragText.LabelPosition.X, startX, mouseX),
 				yScale.AddInterval(_dragText.LabelPosition.Y, startY, mouseY), _dragPane);
 
-			Invalidate();
+            if (_dragPane.Layout != null)
+                Trace.WriteLine(string.Format(@"Goal function: {0}", _dragPane.Layout.CalculateGoalFunction(_dragText)));
+            Invalidate();
 		}
 
 		private void HandleLabelDragFinish(MouseEventArgs e)

pwiz_tools/Skyline/Controls/Graphs/SummaryRelativeAbundanceGraphPane.cs

@@ -157,9 +157,21 @@ private void UpdateFormatting(IEnumerable<MatchRgbHexColor> colorRows)
         /// </summary>
         public void OnLabelOverlapPropertyChange(object sender, PropertyChangedEventArgs e)
         {
-            if (e.PropertyName == @"GroupComparisonAvoidLabelOverlap")
+            if (e.PropertyName == nameof(Settings.Default.GroupComparisonAvoidLabelOverlap))
+            {
+                try
+                {
+                    Settings.Default.PropertyChanged -= OnLabelOverlapPropertyChange;
+                    Settings.Default.GroupComparisonSuspendLabelLayout = false;
+                }
+                finally
+                {
+                    Settings.Default.PropertyChanged += OnLabelOverlapPropertyChange;
+                }
+                _labelsLayout = null;
                 GraphSummary.UpdateUI();
-            else if (e.PropertyName == @"GroupComparisonSuspendLabelLayout")
+            }
+            else if (e.PropertyName == nameof(Settings.Default.GroupComparisonSuspendLabelLayout))
             {
                 if (!Settings.Default.GroupComparisonSuspendLabelLayout)
                 {
@@ -364,14 +376,15 @@ public override void UpdateGraph(bool selectionChanged)
                 UpdateAxes();
             if (Settings.Default.GroupComparisonAvoidLabelOverlap)
             {
-                if (Settings.Default.GroupComparisonSuspendLabelLayout)
-                {
-                    AdjustLabelSpacings(_labeledPoints, _labelsLayout);
-                    _labelsLayout = GraphSummary.GraphControl.GraphPane.Layout?.PointsLayout;
-                }
+                AdjustLabelSpacings(_labeledPoints, _labelsLayout);
+                _labelsLayout = GraphSummary.GraphControl.GraphPane.Layout?.PointsLayout;
             }
             else
-                DotPlotUtil.AdjustLabelLocations(_labeledPoints, GraphSummary.GraphControl.GraphPane.YAxis.Scale, GraphSummary.GraphControl.GraphPane.Rect.Height);
+            {
+                EnableLabelLayout = false;
+                DotPlotUtil.AdjustLabelLocations(_labeledPoints, GraphSummary.GraphControl.GraphPane.YAxis.Scale,
+                    GraphSummary.GraphControl.GraphPane.Rect.Height);
+            }
         }
 
         private void this_AxisChangeEvent(GraphPane pane)

pwiz_tools/Skyline/Controls/GroupComparison/FoldChangeForm.cs

@@ -38,6 +38,7 @@ public partial class FoldChangeForm : DockableFormEx
         private IDocumentUIContainer _documentContainer;
         private string _groupComparisonName;
         private Form _owner;
+        protected bool _dataChanged = true;
         public FoldChangeForm()
         {
             InitializeComponent();
@@ -86,7 +87,9 @@ protected override void OnShown(EventArgs e)
             {
                 if (null != _documentContainer)
                 {
-                    FoldChangeBindingSource = FindOrCreateBindingSource(_documentContainer, _groupComparisonName);
+                    var newBindingSource = FindOrCreateBindingSource(_documentContainer, _groupComparisonName);
+                    _dataChanged = newBindingSource != FoldChangeBindingSource;
+                    FoldChangeBindingSource = newBindingSource;
                     if (IsHandleCreated)
                     {
                         FoldChangeBindingSource.AddRef();