Fixed infinite loop in GetTolColumnFromPixels. (#2382)

* Fixed infinite loop in GetTolColumnFromPixels. Also fixed a crash when saving workbook with groupshape as a shape in a chart.

* Fixed positioning of shapes in charts

* get top and left no longer returns 0.

* fixed From To calculation for chart drawings.

* fixed absolute positioning

---------

Co-authored-by: Jan Källman <jan.kallman@epplussoftware.com>

Author: AdrianEPPlus

src/EPPlus/Drawing/ExcelDrawing.cs

@@ -788,7 +788,7 @@ internal int GetPixelLeft()
                 {
                     return (int)(Math.Round(From.X * _drawings._screenWidth));
                 }
-                return 0;
+                return Position == null ? 0 : Position.X / EMU_PER_PIXEL;
             }
             if (CellAnchor == eEditAs.Absolute)
             {
@@ -818,7 +818,7 @@ internal int GetPixelTop()
                 {
                     return (int)(Math.Round(From.Y * _drawings._screenHeight));
                 }
-                return 0;
+                return Position == null ? 0 : Position.Y / EMU_PER_PIXEL;
             }
 
             if (CellAnchor == eEditAs.Absolute)
@@ -928,14 +928,7 @@ internal void SetPixelTop(double pixels)
             _doNotAdjust = true;
             if (CellAnchor == eEditAs.Absolute)
             {
-                if (_collectionType == DrawingsCollectionType.Worksheet)
-                {
-                    Position.Y = (int)(pixels * EMU_PER_PIXEL);
-                }
-                else
-                {
-                    From.Y= (double)pixels/_drawings._screenHeight;
-                }
+                Position.Y = (int)(pixels * EMU_PER_PIXEL);
             }
             else
             {
@@ -977,14 +970,7 @@ internal void SetPixelLeft(double pixels)
             _doNotAdjust = true;
             if (CellAnchor == eEditAs.Absolute)
             {
-                if (_collectionType == DrawingsCollectionType.Worksheet)
-                {
-                    Position.X = (int)(pixels * EMU_PER_PIXEL);
-                }
-                else
-                {
-                    From.X = (double)pixels / _drawings._screenWidth;
-                }
+                Position.X = (int)(pixels * EMU_PER_PIXEL);
             }
             else
             {
@@ -1143,7 +1129,7 @@ internal void GetToColumnFromPixels(double pixels, out int col, out int colOff,
             double pixOff = pixels - (PixelHelper.GetColumnWidth(ws, fromColumn + 1) - fromColumnOff / EMU_PER_PIXEL);
             double offset = (double)fromColumnOff / EMU_PER_PIXEL + pixels;
             col = fromColumn + 2;
-            while (pixOff >= 0)
+            while (pixOff >= 0 && col < ExcelPackage.MaxColumns)
             {
                 offset = pixOff;
                 pixOff -= PixelHelper.GetColumnWidth(ws, col++);
@@ -1266,6 +1252,11 @@ private void AdjustFromToXY(double x, double y)
             }
             else
             {
+                if (EditAs != eEditAs.Absolute)
+                {
+                    To.X = x + To.X - From.X;
+                    To.Y = y + To.Y - From.Y;
+                }
                 From.X = x;
                 From.Y = y;
             }
@@ -1532,8 +1523,18 @@ private void SetWidthChartShape(double PixelWidth)
             }
             if (To != null)
             {
-                To.X = (From.X + PixelWidth / _drawings._screenWidth);
-                if (To.X > 1) To.X = 1; else if (To.X < 0) To.X = 0;
+                var widthFraction = PixelWidth / _drawings._screenWidth;
+                To.X = From.X + widthFraction;
+                if (To.X > 1)
+                {
+                    To.X = 1;
+                    From.X = Math.Max(0, 1 - widthFraction);
+                }
+                else if (To.X < 0)
+                {
+                    From.X = 0;
+                    To.X = widthFraction;
+                }
             }
             if (Size != null)
             {
@@ -1548,12 +1549,18 @@ private void SetHeightChartShape(double PixelHeight)
             }
             if (To != null)
             {
-                
-                
-                if (To.X > 1) To.X = 1; else if (To.X < 0) To.X = 0;
-
-                To.Y = (From.Y + PixelHeight / _drawings._screenHeight);
-                if (To.Y > 1) To.Y = 1; else if (To.Y < 0) To.Y = 0;
+                var heightFraction = PixelHeight / _drawings._screenHeight;
+                To.Y = From.Y + heightFraction;
+                if (To.Y > 1)
+                {
+                    To.Y = 1;
+                    From.Y = Math.Max(0, 1 - heightFraction);
+                }
+                else if (To.Y < 0)
+                {
+                    From.Y = 0;
+                    To.Y = heightFraction;
+                }
             }
             if (Size != null)
             {

src/EPPlus/Drawing/ExcelGroupShape.cs

@@ -120,7 +120,7 @@ private void AdjustXmlAndMoveToGroup(ExcelDrawing d)
             }
             else if (d._drawings._collectionType == DrawingsCollectionType.Chart)
             {
-                if (d is not ExcelGroupShape)
+                if (d.ParentGroup is ExcelGroupShape)
                 {
                     d.RemoveFromToNodes();
                     d.Position = new ExcelDrawingCoordinate(d.NameSpaceManager, d.TopNode, d.GetPositionSize);

src/EPPlus/ExcelWorksheet.cs

@@ -2556,6 +2556,7 @@ internal void Save(bool hasLoadedPivotTables)
                 }
             }
         }
+
         private void SaveSlicers()
         {
             SlicerXmlSources.Save();

src/EPPlusTest/Drawing/Chart/ChartShapeTest.cs

@@ -21,6 +21,18 @@ public void ShapeInChart()
             SaveAndCleanup(p);
         }
 
+        [TestMethod]
+        public void ShapeInChartTest2()
+        {
+            using var p = OpenTemplatePackage("ShapeInChartTest2.xlsx");
+            var ws = p.Workbook.Worksheets[0];
+            var chart = ws.Drawings[0] as ExcelChartStandard;
+            var cdr = chart.Drawings[0];
+            cdr.SetPosition(150, 200);
+            cdr.SetSize(200, 200);
+
+            SaveAndCleanup(p);
+        }
 
         [TestMethod]
         public void ShapeInChartTest()
@@ -257,6 +269,7 @@ public void GroupShapesWithPictures()
             Assert.IsTrue(chart.Drawings.Count == 5);
             var group = pic1.Group(pic2, pic3, pic4, pic5);
             Assert.IsTrue(chart.Drawings.Count == 1);
+            SaveWorkbook("groupshapes2.xlsx", p);
         }
         [TestMethod]
         public void GroupShapesWithGroupShapes()
@@ -272,11 +285,14 @@ public void GroupShapesWithGroupShapes()
             var equal = chart.Drawings.AddShape("Equal", eShapeStyle.MathEqual);
             var roundRect = chart.Drawings.AddShape("RoundRect", eShapeStyle.Round1Rect);
             var triangle = chart.Drawings.AddShape("Triangle", eShapeStyle.Triangle);
-
+             
             Assert.IsTrue(chart.Drawings.Count == 4);
             var group1 = arrow.Group(equal);
             var group2 = group1.Group(roundRect, triangle);
-            Assert.IsTrue(chart.Drawings.Count == 1);
+            //Assert.IsTrue(chart.Drawings.Count == 1);
+            group2.SetPosition(100, 100);
+            //group2.SetPosition(20, 20);
+            SaveWorkbook("groupshapes1.xlsx", p);
         }
         [TestMethod]
         public void GroupShapesMixed()

src/EPPlusTest/Issues/DrawingIssues.cs

@@ -216,6 +216,13 @@ public void s1045()
             Console.WriteLine("Done (no crash).");
             SaveAndCleanup(destPackage);
         }
+
+        [TestMethod]
+        public void s1055()
+        {
+            using var p = OpenTemplatePackage("s1055.xlsx");
+            SaveAndCleanup(p);
+        }
     }
 }
 

src/EPPlusTest/TestBase.cs

@@ -249,7 +249,7 @@ protected static void SaveWorkbook(string name, ExcelPackage pck)
             var fi = new FileInfo(_worksheetPath + name);
             if (fi.Exists)
             {
-                //fi.Delete();
+                    fi.Delete();
             }
             pck.SaveAs(fi);
         }