Moving a Group (shape container) now moves all contained shapes. (#690)

OffsetX/Y, ExtentX/Y, Width and Height are always calculated to prevent desync between Group and contained shapes.
PHPOffice · Sep 12, 2024 · 04700ed · 04700ed
1 parent 8b75773
commit 04700ed
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 48 deletions.
diff --git a/docs/changes/1.2.0.md b/docs/changes/1.2.0.md
@@ -5,6 +5,7 @@
 ## Enhancements
 
 - `phpoffice/phpspreadsheet`: Allow version 1.9 or 2.0 by [@Progi1984](https://github.com/Progi1984) fixing [#790](https://github.com/PHPOffice/PHPPresentation/pull/790), [#812](https://github.com/PHPOffice/PHPPresentation/pull/812) in [#816](https://github.com/PHPOffice/PHPPresentation/pull/816)
+- Group Shape: moving the shape now moves all contained shapes. Offsets and size are calculated based on the contained shapes by [@DennisBirkholz](https://github.com/DennisBirkholz) in [#690](https://github.com/PHPOffice/PHPPresentation/pull/690)
 
 ## Bug fixes
 

diff --git a/src/PhpPresentation/Shape/Group.php b/src/PhpPresentation/Shape/Group.php
@@ -20,28 +20,13 @@
 namespace PhpOffice\PhpPresentation\Shape;
 
 use PhpOffice\PhpPresentation\AbstractShape;
-use PhpOffice\PhpPresentation\GeometryCalculator;
 use PhpOffice\PhpPresentation\ShapeContainerInterface;
 use PhpOffice\PhpPresentation\Traits\ShapeCollection;
 
 class Group extends AbstractShape implements ShapeContainerInterface
 {
     use ShapeCollection;
 
-    /**
-     * Extent X.
-     *
-     * @var int
-     */
-    protected $extentX;
-
-    /**
-     * Extent Y.
-     *
-     * @var int
-     */
-    protected $extentY;
-
     public function __construct()
     {
         parent::__construct();
@@ -52,22 +37,33 @@ public function __construct()
      */
     public function getOffsetX(): int
     {
-        if (empty($this->offsetX)) {
-            $offsets = GeometryCalculator::calculateOffsets($this);
-            $this->offsetX = $offsets[GeometryCalculator::X];
-            $this->offsetY = $offsets[GeometryCalculator::Y];
+        $offsetX = null;
+
+        foreach ($this->getShapeCollection() as $shape) {
+            if ($offsetX === null) {
+                $offsetX = $shape->getOffsetX();
+            } else {
+                $offsetX = \min($offsetX, $shape->getOffsetX());
+            }
         }
 
-        return $this->offsetX;
+        return $offsetX ?? 0;
     }
 
     /**
-     * Ignores setting the X Offset, preserving the default behavior.
+     * Change the X offset by moving all contained shapes.
      *
      * @return $this
      */
-    public function setOffsetX(int $pValue = 0)
+    public function setOffsetX(int $pValue = 0): self
     {
+        $offsetX = $this->getOffsetX();
+        $diff = $pValue - $offsetX;
+
+        foreach ($this->getShapeCollection() as $shape) {
+            $shape->setOffsetX($shape->getOffsetX() + $diff);
+        }
+
         return $this;
     }
 
@@ -76,22 +72,33 @@ public function setOffsetX(int $pValue = 0)
      */
     public function getOffsetY(): int
     {
-        if (empty($this->offsetY)) {
-            $offsets = GeometryCalculator::calculateOffsets($this);
-            $this->offsetX = $offsets[GeometryCalculator::X];
-            $this->offsetY = $offsets[GeometryCalculator::Y];
+        $offsetY = null;
+
+        foreach ($this->getShapeCollection() as $shape) {
+            if ($offsetY === null) {
+                $offsetY = $shape->getOffsetY();
+            } else {
+                $offsetY = \min($offsetY, $shape->getOffsetY());
+            }
         }
 
-        return $this->offsetY;
+        return $offsetY ?? 0;
     }
 
     /**
-     * Ignores setting the Y Offset, preserving the default behavior.
+     * Change the Y offset by moving all contained shapes.
      *
      * @return $this
      */
-    public function setOffsetY(int $pValue = 0)
+    public function setOffsetY(int $pValue = 0): self
     {
+        $offsetY = $this->getOffsetY();
+        $diff = $pValue - $offsetY;
+
+        foreach ($this->getShapeCollection() as $shape) {
+            $shape->setOffsetY($shape->getOffsetY() + $diff);
+        }
+
         return $this;
     }
 
@@ -100,35 +107,51 @@ public function setOffsetY(int $pValue = 0)
      */
     public function getExtentX(): int
     {
-        if (null === $this->extentX) {
-            $extents = GeometryCalculator::calculateExtents($this);
-            $this->extentX = $extents[GeometryCalculator::X] - $this->getOffsetX();
-            $this->extentY = $extents[GeometryCalculator::Y] - $this->getOffsetY();
+        $extentX = 0;
+
+        foreach ($this->getShapeCollection() as $shape) {
+            $extentX = \max($extentX, $shape->getOffsetX() + $shape->getWidth());
         }
 
-        return $this->extentX;
+        return $extentX - $this->getOffsetX();
     }
 
     /**
      * Get Y Extent.
      */
     public function getExtentY(): int
     {
-        if (null === $this->extentY) {
-            $extents = GeometryCalculator::calculateExtents($this);
-            $this->extentX = $extents[GeometryCalculator::X] - $this->getOffsetX();
-            $this->extentY = $extents[GeometryCalculator::Y] - $this->getOffsetY();
+        $extentY = 0;
+
+        foreach ($this->getShapeCollection() as $shape) {
+            $extentY = \max($extentY, $shape->getOffsetY() + $shape->getHeight());
         }
 
-        return $this->extentY;
+        return $extentY - $this->getOffsetY();
+    }
+
+    /**
+     * Calculate the width based on the size/position of the contained shapes.
+     */
+    public function getWidth(): int
+    {
+        return $this->getExtentX();
+    }
+
+    /**
+     * Calculate the height based on the size/position of the contained shapes.
+     */
+    public function getHeight(): int
+    {
+        return $this->getExtentY();
     }
 
     /**
      * Ignores setting the width, preserving the default behavior.
      *
-     * @return self
+     * @return $this
      */
-    public function setWidth(int $pValue = 0)
+    public function setWidth(int $pValue = 0): self
     {
         return $this;
     }
@@ -138,7 +161,7 @@ public function setWidth(int $pValue = 0)
      *
      * @return $this
      */
-    public function setHeight(int $pValue = 0)
+    public function setHeight(int $pValue = 0): self
     {
         return $this;
     }

diff --git a/tests/PhpPresentation/Tests/Shape/GroupTest.php b/tests/PhpPresentation/Tests/Shape/GroupTest.php
@@ -79,10 +79,10 @@ public function testOffsetX(): void
         $line1 = new Line(10, 20, 30, 50);
         $object->addShape($line1);
 
-        self::assertEquals(10, $object->getOffsetX());
+        self::assertEquals($line1->getOffsetX(), $object->getOffsetX());
 
-        self::assertInstanceOf('PhpOffice\\PhpPresentation\\Shape\\Group', $object->setOffsetX(mt_rand(1, 100)));
-        self::assertEquals(10, $object->getOffsetX());
+        self::assertInstanceOf(Group::class, $object->setOffsetX(mt_rand(1, 100)));
+        self::assertEquals($line1->getOffsetX(), $object->getOffsetX());
     }
 
     public function testOffsetY(): void
@@ -91,10 +91,10 @@ public function testOffsetY(): void
         $line1 = new Line(10, 20, 30, 50);
         $object->addShape($line1);
 
-        self::assertEquals(20, $object->getOffsetY());
+        self::assertEquals($line1->getOffsetY(), $object->getOffsetY());
 
-        self::assertInstanceOf('PhpOffice\\PhpPresentation\\Shape\\Group', $object->setOffsetY(mt_rand(1, 100)));
-        self::assertEquals(20, $object->getOffsetY());
+        self::assertInstanceOf(Group::class, $object->setOffsetY(mt_rand(1, 100)));
+        self::assertEquals($line1->getOffsetY(), $object->getOffsetY());
     }
 
     public function testExtentsAndOffsetsForOneShape(): void