From 2d23b878e7974f9ebe1bcf891a3f33294332ec9e Mon Sep 17 00:00:00 2001 From: David Montero Date: Tue, 16 Jun 2026 05:25:31 +0200 Subject: [PATCH] fix(canvas): rotated-component pin positions in 3 paths (fixes #230, #231, #232) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All three bugs are rotated components whose pin geometry is computed in a path that ignores the rotation, so pins/wire-starts land tens of pixels off the visual pin tips. The live rotate action already recalculates correctly; these are the paths that didn't. #231 (context-menu 'Tap a pin to wire'): both onPinSelect handlers in SimulatorCanvas computed the wire start as getBoundingClientRect().left + pin.x — adding the UNROTATED pin offset to the ROTATED bounding-box corner. On a 90-deg HC-SR04 that put the start ~70-100px off (measured). Replaced with calculatePinPosition(id, x+6, y+6, rotation), the same rotation-aware helper wires and the pin overlay use. #232 (rotate -> delete -> undo): recordRemoveComponent's undo restored the component + wires but never recalculated wire endpoints, so a rotated part's wires kept the unrotated coords captured at delete time. Added a requestAnimationFrame updateWirePositions(id) after restore. #230 + #232 (pin boxes wrong after import / undo / load, 'fixes if rotated again'): PinOverlay captured the wrapper's layout box (the rotation pivot) once at mount. On import/undo/load the component mounts already-rotated and its wokwi-element may not be sized on the mount tick, baking a wrong pivot that only refreshed when rotation changed. PinOverlay now re-measures after layout (rAF) and whenever it is about to become visible (showPins dep). --- .../src/components/simulator/PinOverlay.tsx | 24 +++++-- .../components/simulator/SimulatorCanvas.tsx | 68 +++++++------------ frontend/src/store/useSimulatorStore.ts | 13 +++- 3 files changed, 55 insertions(+), 50 deletions(-) diff --git a/frontend/src/components/simulator/PinOverlay.tsx b/frontend/src/components/simulator/PinOverlay.tsx index 7b40d138..217ed16e 100644 --- a/frontend/src/components/simulator/PinOverlay.tsx +++ b/frontend/src/components/simulator/PinOverlay.tsx @@ -68,6 +68,8 @@ export const PinOverlay: React.FC = ({ const isCoarse = useIsCoarsePointer(); useEffect(() => { + let raf = 0; + let timer: ReturnType | undefined; const tryRead = () => { const element = document.getElementById(componentId); if (element && (element as any).pinInfo) { @@ -84,12 +86,22 @@ export const PinOverlay: React.FC = ({ } return false; }; - if (!tryRead()) { - // Retry once after a tick in case the element sets pinInfo asynchronously (e.g. via useEffect) - const t = setTimeout(tryRead, 50); - return () => clearTimeout(t); - } - }, [componentId, rotation]); + // Read immediately (correct when the element is already laid out), then + // re-measure after layout. On import / undo / project load the component + // mounts ALREADY rotated and its wokwi-element may not have its final size + // on the mount tick — reading offsetWidth then bakes a wrong rotation pivot + // that previously only refreshed when the user rotated again (issues #230, + // #232). Re-running on `showPins` also re-measures right before the overlay + // becomes visible, by which point the element is laid out. + tryRead(); + raf = requestAnimationFrame(() => { + if (!tryRead()) timer = setTimeout(tryRead, 50); + }); + return () => { + cancelAnimationFrame(raf); + if (timer) clearTimeout(timer); + }; + }, [componentId, rotation, showPins]); if (!showPins || pins.length === 0) { return null; diff --git a/frontend/src/components/simulator/SimulatorCanvas.tsx b/frontend/src/components/simulator/SimulatorCanvas.tsx index 0ad774b6..476995c1 100644 --- a/frontend/src/components/simulator/SimulatorCanvas.tsx +++ b/frontend/src/components/simulator/SimulatorCanvas.tsx @@ -26,6 +26,7 @@ import { PROPERTY_CHANGE_EVENT, type PropertyChangeDetail } from '../../simulati import { mountDigitalGateEngine } from '../../simulation/digital/digitalGateController'; import { isSpiceMapped } from '../../simulation/spice/componentToSpice'; import { PinOverlay } from './PinOverlay'; +import { calculatePinPosition } from '../../utils/pinPositionCalculator'; import { isBoardComponent, boardPinToNumber } from '../../utils/boardPinMapping'; import { autoWireColor, WIRE_KEY_COLORS } from '../../utils/wireUtils'; import { @@ -2714,30 +2715,24 @@ export const SimulatorCanvas = ({ headerSlot }: SimulatorCanvasProps = {}) => { setPinPicker(null); return; } - // Resolve world coords from the rendered element rect — this - // accounts for wrapper offsets, rotation, and current zoom. - const compEl = document.getElementById(targetId); - const canvasRect = canvasRef.current?.getBoundingClientRect(); - const compRect = compEl?.getBoundingClientRect(); + // Resolve world coords the SAME way wires + the pin overlay do: + // calculatePinPosition rotates the pin around the wrapper centre + // for the component's rotation. The old getBoundingClientRect path + // added the unrotated pin offset to the ROTATED bounding-box corner, + // landing the wire start ~70-100px off on a rotated part (issue #231). let worldX: number; let worldY: number; - if (canvasRect && compRect) { - const screenX = compRect.left + pin.x * zoomRef.current; - const screenY = compRect.top + pin.y * zoomRef.current; - const w = toWorld(screenX, screenY); - worldX = w.x; - worldY = w.y; + if (pinPicker.kind === 'board') { + const b = boards.find((x) => x.id === targetId); + const pos = calculatePinPosition(targetId, pinName, b?.x ?? 0, b?.y ?? 0, 0); + worldX = pos?.x ?? (b?.x ?? 0) + pin.x; + worldY = pos?.y ?? (b?.y ?? 0) + pin.y; } else { - // Fallback: approximate from the stored x/y of the target. - if (pinPicker.kind === 'board') { - const b = boards.find((x) => x.id === targetId); - worldX = (b?.x ?? 0) + pin.x; - worldY = (b?.y ?? 0) + pin.y; - } else { - const c = components.find((x) => x.id === targetId); - worldX = (c?.x ?? 0) + pin.x; - worldY = (c?.y ?? 0) + pin.y; - } + const c = components.find((x) => x.id === targetId); + const rot = c ? Number(c.properties?.rotation) || 0 : 0; + const pos = calculatePinPosition(targetId, pinName, (c?.x ?? 0) + 6, (c?.y ?? 0) + 6, rot); + worldX = pos?.x ?? (c?.x ?? 0) + pin.x; + worldY = pos?.y ?? (c?.y ?? 0) + pin.y; } setPinPicker(null); handlePinClick(targetId, pinName, worldX, worldY); @@ -2787,30 +2782,19 @@ export const SimulatorCanvas = ({ headerSlot }: SimulatorCanvasProps = {}) => { } }} onPinSelect={(id, pinName) => { - // Pick world coords from the live element rect when possible — - // it accounts for the wokwi-element wrapper offset and the - // current rotation. Falls back to component origin + pin - // offset if the rect can't be read. - const compEl = document.getElementById(id); + // Resolve world coords the SAME way wires + the pin overlay do, + // via calculatePinPosition, which rotates the pin around the + // wrapper centre for the component's rotation. The old + // getBoundingClientRect path added the unrotated pin offset to the + // ROTATED bounding-box corner, so on a rotated part the wire start + // landed ~70-100px away from the pin (issue #231). const pin = (pinInfo || []).find((p: { name: string }) => p.name === pinName); const c = components.find((x) => x.id === id); if (!c || !pin) return; - const canvasRect = canvasRef.current?.getBoundingClientRect(); - const compRect = compEl?.getBoundingClientRect(); - let worldX: number; - let worldY: number; - if (canvasRect && compRect) { - // Convert pin's CSS-pixel offset (relative to component) into - // world coords via the canvas pan/zoom transform. - const screenX = compRect.left + pin.x * zoomRef.current; - const screenY = compRect.top + pin.y * zoomRef.current; - const w = toWorld(screenX, screenY); - worldX = w.x; - worldY = w.y; - } else { - worldX = c.x + pin.x; - worldY = c.y + pin.y; - } + const rot = Number(c.properties?.rotation) || 0; + const pos = calculatePinPosition(id, pinName, c.x + 6, c.y + 6, rot); + const worldX = pos?.x ?? c.x + pin.x; + const worldY = pos?.y ?? c.y + pin.y; setShowPropertyDialog(false); handlePinClick(id, pinName, worldX, worldY); }} diff --git a/frontend/src/store/useSimulatorStore.ts b/frontend/src/store/useSimulatorStore.ts index ebc88606..d220f757 100644 --- a/frontend/src/store/useSimulatorStore.ts +++ b/frontend/src/store/useSimulatorStore.ts @@ -2623,11 +2623,20 @@ export const useSimulatorStore = create((set, get) => { (w) => w.start.componentId !== id && w.end.componentId !== id, ), })), - undo: () => + undo: () => { set((s) => ({ components: [...s.components, removed], wires: [...s.wires, ...removedWires], - })), + })); + // Recalc this part's wire endpoints once it re-mounts. Without this + // a rotated component restored via Ctrl+Z keeps the unrotated wire + // coords captured at delete time, so its wires sit off the pins + // until the user rotates again (issue #232). rAF waits for the DOM + // node so calculatePinPosition can read the wrapper geometry. + const recalc = () => get().updateWirePositions(id); + if (typeof requestAnimationFrame === 'function') requestAnimationFrame(recalc); + else recalc(); + }, }); },