Fix SchematicBox control-point geometry on placement and drag

Two related bugs made box pins detach from the box edge:

1. SchematicBox.setControlPoint(pt, 0) called updateControlPoints(), which
   rebuilt the whole control-point array. The presenter's drag loop moves
   every control point by the same delta one at a time, so once index 0
   was set the other pins were recomputed and then had the delta added a
   second time - pins flew off by 2x the drag, and by different amounts
   depending on the (HashSet) index order. setControlPoint now just moves
   the requested point; updateControlPoints only runs when the node lists
   change, mutating points in place when the count is unchanged.

2. SchematicBuilder.moveAnchorTo() read getControlPoint(i) inside its loop
   after setting index 0, so components that recompute on set (SchematicBox)
   got a double offset during initial placement - this is why initial wires
   converged on one node and only snapped to the right pins after a manual
   move. moveAnchorTo now snapshots all target positions before mutating.

Adds SchematicBox drag/translation tests.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FWD3QJxbr7RuEHxyFEYeEY
This commit is contained in:
Bane Stojkovic
2026-09-03 11:13:50 +02:00
parent 20116ec98c
commit 5997fcfe9c
4 changed files with 98 additions and 14 deletions
@@ -133,17 +133,25 @@ public class SchematicBox extends AbstractTransparentComponent<String> {
}
/**
* Rebuilds the control point array so that index 0 keeps its location and every other point is
* placed relative to it. Ordering: left side (top to bottom), right side (top to bottom), top side
* (left to right), bottom side (left to right).
* Recomputes the pin geometry from the anchor (control point 0) and the configured node counts.
* Ordering: left side (top to bottom), right side (top to bottom), top side (left to right),
* bottom side (left to right).
*
* <p>
* This is called on construction and whenever the node lists change — <b>not</b> from
* {@link #setControlPoint}. During a drag the presenter translates every control point by the
* same delta (because {@link #canPointMoveFreely} is {@code false}), which already keeps the pins
* on the box edge, so recomputing there would double-apply the delta. When the pin count is
* unchanged the existing {@link Point2D} objects are mutated in place so references held by the
* drawing/selection code stay valid.
* </p>
*/
private void updateControlPoints() {
nodeLabels = null;
double spacing = pinSpacing();
double leadLength = spacing;
Point2D anchor = controlPoints[0];
double firstPinX = anchor.getX();
double firstPinY = anchor.getY();
double firstPinX = controlPoints[0].getX();
double firstPinY = controlPoints[0].getY();
double bodyX = firstPinX + leadLength;
double bodyY = firstPinY - spacing;
@@ -172,7 +180,14 @@ public class SchematicBox extends AbstractTransparentComponent<String> {
}
// keep index 0 exactly where it was
points.set(0, new Point2D.Double(firstPinX, firstPinY));
controlPoints = points.toArray(new Point2D[0]);
if (controlPoints.length == points.size()) {
for (int i = 0; i < points.size(); i++) {
controlPoints[i].setLocation(points.get(i));
}
} else {
controlPoints = points.toArray(new Point2D[0]);
}
}
private List<String> getNodeLabels() {
@@ -312,10 +327,9 @@ public class SchematicBox extends AbstractTransparentComponent<String> {
@Override
public void setControlPoint(Point2D point, int index) {
// Move only the requested point. The presenter drags every control point of the box together
// (canPointMoveFreely == false), so the pins stay on the edge without a recompute here.
controlPoints[index].setLocation(point);
if (index == 0) {
updateControlPoints();
}
}
@Override
@@ -285,15 +285,23 @@ public class SchematicBuilder {
/** Translates every control point of the component so that control point 0 lands on the target. */
static void moveAnchorTo(IDIYComponent<?> component, double targetX, double targetY) {
if (component.getControlPointCount() == 0) {
int count = component.getControlPointCount();
if (count == 0) {
return;
}
Point2D anchor = component.getControlPoint(0);
double dx = targetX - anchor.getX();
double dy = targetY - anchor.getY();
for (int i = 0; i < component.getControlPointCount(); i++) {
// Snapshot the target positions before mutating: some components (e.g. SchematicBox) recompute
// all of their control points when control point 0 is set, so reading getControlPoint(i) inside
// the loop after that would double-apply the offset.
Point2D[] targets = new Point2D[count];
for (int i = 0; i < count; i++) {
Point2D p = component.getControlPoint(i);
component.setControlPoint(new Point2D.Double(p.getX() + dx, p.getY() + dy), i);
targets[i] = new Point2D.Double(p.getX() + dx, p.getY() + dy);
}
for (int i = 0; i < count; i++) {
component.setControlPoint(targets[i], i);
}
}
@@ -28,6 +28,7 @@ import java.util.List;
import java.util.UUID;
import org.diylc.components.passive.ResistorSymbol;
import org.diylc.components.schematic.SchematicBox;
import org.diylc.components.schematic.SchematicWire;
import org.diylc.core.IDIYComponent;
import org.junit.Test;
@@ -73,6 +74,67 @@ public class SchematicRerouteTest {
assertTrue(!SchematicBuilder.rerouteWires(components));
}
@Test
public void moveAnchorToTranslatesEverySchematicBoxPinExactlyOnce() {
SchematicBox box = new SchematicBox();
box.setLeftNodes("A,B");
box.setRightNodes("C,D");
box.setTopNodes("");
box.setBottomNodes("");
int n = box.getControlPointCount();
double ax = box.getControlPoint(0).getX();
double ay = box.getControlPoint(0).getY();
double[] relX = new double[n];
double[] relY = new double[n];
for (int i = 0; i < n; i++) {
relX[i] = box.getControlPoint(i).getX() - ax;
relY[i] = box.getControlPoint(i).getY() - ay;
}
SchematicBuilder.moveAnchorTo(box, 500, 300);
assertEquals(500.0, box.getControlPoint(0).getX(), 0.001);
assertEquals(300.0, box.getControlPoint(0).getY(), 0.001);
for (int i = 0; i < n; i++) {
assertEquals("pin " + i + " x", 500 + relX[i], box.getControlPoint(i).getX(), 0.001);
assertEquals("pin " + i + " y", 300 + relY[i], box.getControlPoint(i).getY(), 0.001);
}
}
@Test
public void draggingSchematicBoxTranslatesEveryPinByTheSameDelta() {
// the presenter drags a box by calling setControlPoint(old + delta, i) for every index, in an
// arbitrary (HashSet) order; every pin must end up translated by exactly the delta
int[][] orders = {{0, 1, 2, 3, 4, 5, 6}, {6, 5, 4, 3, 2, 1, 0}, {3, 0, 5, 1, 6, 2, 4}};
for (int[] order : orders) {
SchematicBox box = new SchematicBox();
box.setLeftNodes("A,B,C");
box.setRightNodes("D,E");
box.setTopNodes("F");
box.setBottomNodes("G");
int n = box.getControlPointCount();
assertEquals(7, n);
Point2D[] before = new Point2D[n];
for (int i = 0; i < n; i++) {
before[i] = new Point2D.Double(box.getControlPoint(i).getX(), box.getControlPoint(i).getY());
}
double dx = 137;
double dy = -84;
for (int idx : order) {
Point2D old = box.getControlPoint(idx);
box.setControlPoint(new Point2D.Double(old.getX() + dx, old.getY() + dy), idx);
}
for (int i = 0; i < n; i++) {
assertEquals("pin " + i + " x", before[i].getX() + dx, box.getControlPoint(i).getX(), 0.001);
assertEquals("pin " + i + " y", before[i].getY() + dy, box.getControlPoint(i).getY(), 0.001);
}
}
}
private static int last(SchematicWire wire) {
return wire.getRoutePoints().size() - 1;
}
+1 -1
View File
@@ -433,7 +433,7 @@ The schematic uses the **existing `DrawingManager` + `Presenter`** rendering pip
- `INetlistProcessor.getContinuityAreas()` (implemented on `Presenter`) so the schematic can be built outside the presenter.
**diylc-library**
- `SchematicBox` component (`org.diylc.components.schematic`) — generic box with configurable left/right/top/bottom comma-separated terminal lists, NAME/VALUE/BOTH/NONE display.
- `SchematicBox` component (`org.diylc.components.schematic`) — generic box with configurable left/right/top/bottom comma-separated terminal lists, NAME/VALUE/BOTH/NONE display. `setControlPoint` moves only the requested point (the presenter drags all of them together because `canPointMoveFreely` is false); the pin geometry is recomputed only when the node lists change, and then in place when the pin count is unchanged so held references stay valid.
- `SchematicWire` component — auto-routed Manhattan connection; implements `IContinuity`; stores source/target component id + pin index and the calculated `routePoints`; deep-cloned.
- `ManhattanRouter` — straight / L / HVH / VHV routing with pin exit-direction preference and overlap/crossing cost scoring against existing segments.
- `GenericBoxSchematicFactory` (default fallback) + `SchematicBuilder` (filter, symbol creation, connection-density grid placement, per-net wire chaining, canvas auto-resize) + `SchematicSynchronizer` (add/remove/keep-position incremental sync, full wire rebuild). Switches (`ISwitch`, which extends `IContinuity`) are treated as regular symbols — the generic box fallback until a dedicated factory is added — since the netlist is built with `includeSwitches=false`.