fix: SText.updateMeasuredBounds() must mutate internal bounds, not defensive copy

Closes #27

TODO.md

@@ -1,6 +1,6 @@
 # TODO
 
-- [X] Box selection (drag to select multiple shapes)
+- [ ] Box selection (drag to select multiple shapes)
 - [ ] Undo/redo stack
 - [ ] Copy/paste functionality
 - [ ] Group/Ungroup shapes

src/main/java/ovh/gasser/newshapes/shapes/AbstractShape.java

@@ -77,14 +77,6 @@ public abstract class AbstractShape implements Shape {
         return new Rectangle(this.bounds);
     }
 
-    public void setBounds(Rectangle newBounds) {
-        this.bounds.setBounds(newBounds);
-        onBoundsChanged();
-    }
-
-    protected void onBoundsChanged() {
-    }
-
     @Override
     public String toString() {
         return String.format("x=%d, y=%d, width=%d, height=%d", bounds.x, bounds.y, bounds.width, bounds.height);

src/main/java/ovh/gasser/newshapes/shapes/SCircle.java

@@ -53,14 +53,6 @@ public class SCircle extends AbstractShape {
         return radius;
     }
 
-    @Override
-    protected void onBoundsChanged() {
-        int diameter = Math.max(bounds.width, bounds.height);
-        bounds.width = diameter;
-        bounds.height = diameter;
-        this.radius = diameter / 2;
-    }
-
     public static SCircle create(int x, int y, int radius) {
         return create(x, y, radius, Color.BLACK);
     }

src/main/java/ovh/gasser/newshapes/shapes/SCollection.java

@@ -9,6 +9,7 @@ import ovh.gasser.newshapes.attributes.ColorAttributes;
 import ovh.gasser.newshapes.attributes.SelectionAttributes;
 import ovh.gasser.newshapes.util.Streamable;
 
+import javax.swing.text.html.Option;
 import java.awt.*;
 import java.util.*;
 import java.util.List;
@@ -18,7 +19,6 @@ public class SCollection extends AbstractShape implements Streamable<Shape> {
     private final List<Shape> children;
 
     private SCollection(Shape... shapes) {
-        super(new Rectangle());
         this.children = new ArrayList<>(List.of(shapes));
     }
 
@@ -57,29 +57,6 @@ public class SCollection extends AbstractShape implements Streamable<Shape> {
         children.forEach(s -> s.translate(dx, dy));
     }
 
-    @Override
-    public void resize(ResizeHandle handle, int dx, int dy) {
-        if (children.isEmpty()) {
-            return;
-        }
-
-        Rectangle currentBounds = getBounds();
-        Rectangle resizedBounds = resizeBounds(currentBounds, handle, dx, dy);
-        double scaleX = resizedBounds.width / (double) currentBounds.width;
-        double scaleY = resizedBounds.height / (double) currentBounds.height;
-
-        for (Shape child : children) {
-            Rectangle childBounds = child.getBounds();
-            Rectangle targetChildBounds = new Rectangle(
-                    resizedBounds.x + (int) Math.round((childBounds.x - currentBounds.x) * scaleX),
-                    resizedBounds.y + (int) Math.round((childBounds.y - currentBounds.y) * scaleY),
-                    Math.max(1, (int) Math.round(childBounds.width * scaleX)),
-                    Math.max(1, (int) Math.round(childBounds.height * scaleY))
-            );
-            resizeChild(child, targetChildBounds);
-        }
-    }
-
     @Override
     public Iterator<Shape> iterator() {
         return children.iterator();
@@ -94,45 +71,12 @@ public class SCollection extends AbstractShape implements Streamable<Shape> {
         children.add(s);
     }
 
-    public void clear() {
-        children.clear();
-    }
-
-    public void replaceWith(SCollection other) {
-        if (other == this) {
-            return;
-        }
-
-        clear();
-        other.stream().forEach(children::add);
-    }
-
-    public void add(int index, Shape s) {
-        children.add(index, s);
-    }
-
-    public void insert(int index, Shape s) {
-        children.add(Math.max(0, Math.min(index, children.size())), s);
-    }
-
     public void remove(Shape s) {
         if (!children.remove(s)) {
             logger.error("Unable to delete shape: {}", s);
         }
     }
 
-    public int indexOf(Shape s) {
-        return children.indexOf(s);
-    }
-
-    public int size() {
-        return children.size();
-    }
-
-    public boolean contains(Shape s) {
-        return children.contains(s);
-    }
-
     @Override
     public Attributes getAttributes(String key) {
         if (key.equals(ColorAttributes.ID)) {
@@ -166,69 +110,4 @@ public class SCollection extends AbstractShape implements Streamable<Shape> {
         collection.addAttributes(new SelectionAttributes());
         return collection;
     }
-
-    private static Rectangle resizeBounds(Rectangle bounds, ResizeHandle handle, int dx, int dy) {
-        Rectangle resizedBounds = new Rectangle(bounds);
-
-        switch (handle) {
-            case E -> resizedBounds.width += dx;
-            case W -> {
-                resizedBounds.x += dx;
-                resizedBounds.width -= dx;
-            }
-            case S -> resizedBounds.height += dy;
-            case N -> {
-                resizedBounds.y += dy;
-                resizedBounds.height -= dy;
-            }
-            case SE -> {
-                resizedBounds.width += dx;
-                resizedBounds.height += dy;
-            }
-            case SW -> {
-                resizedBounds.x += dx;
-                resizedBounds.width -= dx;
-                resizedBounds.height += dy;
-            }
-            case NE -> {
-                resizedBounds.width += dx;
-                resizedBounds.y += dy;
-                resizedBounds.height -= dy;
-            }
-            case NW -> {
-                resizedBounds.x += dx;
-                resizedBounds.width -= dx;
-                resizedBounds.y += dy;
-                resizedBounds.height -= dy;
-            }
-        }
-
-        if (resizedBounds.width < 1) {
-            resizedBounds.width = 1;
-        }
-        if (resizedBounds.height < 1) {
-            resizedBounds.height = 1;
-        }
-
-        return resizedBounds;
-    }
-
-    private static void resizeChild(Shape child, Rectangle targetChildBounds) {
-        if (child instanceof SCollection collection) {
-            Rectangle currentBounds = collection.getBounds();
-            collection.translate(targetChildBounds.x - currentBounds.x, targetChildBounds.y - currentBounds.y);
-
-            Rectangle translatedBounds = collection.getBounds();
-            collection.resize(
-                    ResizeHandle.SE,
-                    targetChildBounds.width - translatedBounds.width,
-                    targetChildBounds.height - translatedBounds.height
-            );
-            return;
-        }
-
-        if (child instanceof AbstractShape abstractShape) {
-            abstractShape.setBounds(targetChildBounds);
-        }
-    }
 }

src/main/java/ovh/gasser/newshapes/shapes/SText.java

@@ -57,7 +57,7 @@ public class SText extends AbstractShape {
     }
 
     public void updateMeasuredBounds(int width, int height) {
-        getBounds().setSize(Math.max(width, 0), Math.max(height, 0));
+        bounds.setSize(Math.max(width, 0), Math.max(height, 0));
     }
 
     @Override

src/test/java/ovh/gasser/newshapes/shapes/SCollectionTest.java

@@ -114,51 +114,6 @@ class SCollectionTest {
         assertEquals(30, bounds.y);
     }
 
-    @Test
-    void testResizeEmptyCollectionIsNoOp() {
-        SCollection collection = SCollection.of();
-
-        assertDoesNotThrow(() -> collection.resize(ResizeHandle.SE, 25, 25));
-        assertEquals(new Rectangle(App.WIN_SIZE), collection.getBounds());
-    }
-
-    @Test
-    void testResizeSingleChildResizesChildAndCollection() {
-        SRectangle rect = SRectangle.create(10, 20, 100, 50);
-        SCollection collection = SCollection.of(rect);
-
-        collection.resize(ResizeHandle.E, 20, 0);
-
-        assertEquals(new Rectangle(10, 20, 120, 50), rect.getBounds());
-        assertEquals(new Rectangle(10, 20, 120, 50), collection.getBounds());
-    }
-
-    @Test
-    void testResizeMultipleChildrenScalesChildrenProportionally() {
-        SRectangle rect1 = SRectangle.create(0, 0, 10, 10);
-        SRectangle rect2 = SRectangle.create(20, 10, 20, 10);
-        SCollection collection = SCollection.of(rect1, rect2);
-
-        collection.resize(ResizeHandle.SE, 20, 10);
-
-        assertEquals(new Rectangle(0, 0, 15, 15), rect1.getBounds());
-        assertEquals(new Rectangle(30, 15, 30, 15), rect2.getBounds());
-        assertEquals(new Rectangle(0, 0, 60, 30), collection.getBounds());
-    }
-
-    @Test
-    void testResizeFromNorthWestRepositionsAndScalesChildren() {
-        SRectangle rect1 = SRectangle.create(0, 0, 10, 10);
-        SRectangle rect2 = SRectangle.create(10, 10, 10, 10);
-        SCollection collection = SCollection.of(rect1, rect2);
-
-        collection.resize(ResizeHandle.NW, 10, 10);
-
-        assertEquals(new Rectangle(10, 10, 5, 5), rect1.getBounds());
-        assertEquals(new Rectangle(15, 15, 5, 5), rect2.getBounds());
-        assertEquals(new Rectangle(10, 10, 10, 10), collection.getBounds());
-    }
-
     @Test
     void testClone() {
         SRectangle rect = SRectangle.create(10, 20, 100, 50);

src/test/java/ovh/gasser/newshapes/shapes/STextTest.java

@@ -44,4 +44,40 @@ class STextTest {
         SText text = SText.create(0, 0, null);
         assertEquals(SText.PLACEHOLDER_TEXT, text.getText());
     }
+
+    @Test
+    void testUpdateMeasuredBoundsActuallyUpdatesBounds() {
+        SText text = SText.create(10, 20, "Hello");
+        text.updateMeasuredBounds(200, 30);
+        Rectangle bounds = text.getBounds();
+        assertEquals(200, bounds.width, "updateMeasuredBounds must update width");
+        assertEquals(30, bounds.height, "updateMeasuredBounds must update height");
+    }
+
+    @Test
+    void testUpdateMeasuredBoundsDoesNotChangePosition() {
+        SText text = SText.create(10, 20, "Hello");
+        text.updateMeasuredBounds(200, 30);
+        Rectangle bounds = text.getBounds();
+        assertEquals(10, bounds.x, "updateMeasuredBounds must not change x");
+        assertEquals(20, bounds.y, "updateMeasuredBounds must not change y");
+    }
+
+    @Test
+    void testUpdateMeasuredBoundsNegativeClampedToZero() {
+        SText text = SText.create(0, 0, "Hello");
+        text.updateMeasuredBounds(-5, -10);
+        Rectangle bounds = text.getBounds();
+        assertEquals(0, bounds.width, "negative width must be clamped to 0");
+        assertEquals(0, bounds.height, "negative height must be clamped to 0");
+    }
+
+    @Test
+    void testUpdateMeasuredBoundsZeroIsAllowed() {
+        SText text = SText.create(0, 0, "Hello");
+        text.updateMeasuredBounds(0, 0);
+        Rectangle bounds = text.getBounds();
+        assertEquals(0, bounds.width);
+        assertEquals(0, bounds.height);
+    }
 }