diff --git a/lib/components/base-components/PrimitiveComponent/PrimitiveComponent.ts b/lib/components/base-components/PrimitiveComponent/PrimitiveComponent.ts index ddbc14bca..ec666d2ff 100644 --- a/lib/components/base-components/PrimitiveComponent/PrimitiveComponent.ts +++ b/lib/components/base-components/PrimitiveComponent/PrimitiveComponent.ts @@ -499,13 +499,19 @@ export abstract class PrimitiveComponent< this.props.pcbX === undefined && this.props.pcbY === undefined ) { + // `manualPlacement` is already in subcircuit-global coordinates — + // `_getPcbManualPlacementForComponent` returns + // applyToPoint(subcircuit.globalTransform, position.center) + // so the value is the absolute (board-frame) position the user + // dragged the component to. Composing it with the parent's + // transform on top of that translates by the parent's anchor a + // second time, double-counting any positioned ancestor group: a + // cap manually placed at (11.4, 4.3) inside `` + // ended up at (27.4, 4.3). Just translate to the absolute pos. const rotation = this._getPcbRotationBeforeLayout() ?? 0 return compose( - this.parent?._computePcbGlobalTransformBeforeLayout() ?? identity(), - compose( - translate(manualPlacement.x, manualPlacement.y), - rotate((rotation * Math.PI) / 180), - ), + translate(manualPlacement.x, manualPlacement.y), + rotate((rotation * Math.PI) / 180), ) } diff --git a/lib/components/primitive-components/Group/Group_doInitialPcbLayoutPack/Group_doInitialPcbLayoutPack.ts b/lib/components/primitive-components/Group/Group_doInitialPcbLayoutPack/Group_doInitialPcbLayoutPack.ts index 48d95a539..1a4211d6a 100644 --- a/lib/components/primitive-components/Group/Group_doInitialPcbLayoutPack/Group_doInitialPcbLayoutPack.ts +++ b/lib/components/primitive-components/Group/Group_doInitialPcbLayoutPack/Group_doInitialPcbLayoutPack.ts @@ -112,6 +112,24 @@ export const Group_doInitialPcbLayoutPack = (group: Group) => { } } + // Also mark any descendant component that has a manualEdits placement + // as static — otherwise the packer would happily reposition it on top + // of the user's pinned location, silently undoing the manual edit. + // Walk every NormalComponent under this group; if its nearest subcircuit + // resolves a manual placement for it, treat it like any other relatively + // positioned child. + const collectManuallyPlacedDescendants = (comp: any) => { + const subcircuit = comp?.getSubcircuit?.() + const manualPlacement = subcircuit?._getPcbManualPlacementForComponent?.( + comp, + ) + if (manualPlacement && comp?.pcb_component_id) { + staticPcbComponentIds.add(comp.pcb_component_id) + } + if (comp?.children) comp.children.forEach(collectManuallyPlacedDescendants) + } + collectManuallyPlacedDescendants(group) + // Keep all circuit elements; static components will remain fixed during packing const filteredCircuitJson = db.toArray() diff --git a/lib/components/primitive-components/Group/Group_doInitialPcbLayoutPack/applyPackOutput.ts b/lib/components/primitive-components/Group/Group_doInitialPcbLayoutPack/applyPackOutput.ts index dac5317c8..10b5114cf 100644 --- a/lib/components/primitive-components/Group/Group_doInitialPcbLayoutPack/applyPackOutput.ts +++ b/lib/components/primitive-components/Group/Group_doInitialPcbLayoutPack/applyPackOutput.ts @@ -106,9 +106,6 @@ export const applyPackOutput = ( const pcbComponent = db.pcb_component.get(componentId) if (pcbComponent) { - db.pcb_component.update(componentId, { - position_mode: "packed", - }) const currentGroupId = group.source_group_id const sourceComponent = db.source_component.get( pcbComponent.source_component_id, @@ -121,6 +118,32 @@ export const applyPackOutput = ( continue } + // If a manual placement positioned this component, skip the + // pack transform — `pcb_component.center` is already at the + // absolute manual position (set by + // `_computePcbGlobalTransformBeforeLayout`), and the static + // pack output's `center` is also in board-absolute coords, so + // composing the group's transform on top would double-count + // the group anchor (a cap manually placed at (11.4, 4.3) inside + // `` would land at (27.4, 4.3)). The child + // pcb primitives were already placed at the absolute position + // during initial render — nothing to do here. + const manualEdits = (group as any).getSubcircuit?.()?._parsedProps + ?.manualEdits + if (manualEdits?.pcb_placements && sourceComponent?.name) { + const hit = manualEdits.pcb_placements.find( + (p: any) => p.selector === sourceComponent.name, + ) + if (hit) { + db.pcb_component.update(componentId, { position_mode: "packed" }) + continue + } + } + + db.pcb_component.update(componentId, { + position_mode: "packed", + }) + const originalCenter = pcbComponent.center const rotationDegrees = ccwRotationDegrees ?? ccwRotationOffset ?? 0 const transformMatrix = compose( diff --git a/tests/pcb-packing/manual-edits-position-no-double-count.test.tsx b/tests/pcb-packing/manual-edits-position-no-double-count.test.tsx new file mode 100644 index 000000000..328eb3e42 --- /dev/null +++ b/tests/pcb-packing/manual-edits-position-no-double-count.test.tsx @@ -0,0 +1,100 @@ +import { expect, test } from "bun:test" +import { getTestFixture } from "tests/fixtures/get-test-fixture" + +// Repro for: manualEdits.pcb_placements double-count the parent group's +// anchor. _computePcbGlobalTransformBeforeLayout composed manualPlacement +// (already absolute in the subcircuit's frame) with the parent's +// transform, so a cap manually placed at (11.4, 4.3) inside +// `` ended up at (27.4, 4.3) — the +16 region anchor +// was added on top of the absolute manual position. +test("manualEdits placement lands at absolute position, not parent-anchor + manual", () => { + const { circuit } = getTestFixture() + + const manualEdits = { + pcb_placements: [ + { + selector: "C1", + center: { x: 11.4, y: 4.3 }, + relative_to: "group_center", + }, + ], + } as any + + circuit.add( + + + + + + , + ) + circuit.render() + + const c1 = (() => { + const src = circuit.db.source_component + .list() + .find((s) => s.name === "C1")! + return circuit.db.pcb_component + .list() + .find((p) => p.source_component_id === src.source_component_id)! + })() + + // Without the fix the cap would land at (11.4 + 16, 4.3) = (27.4, 4.3). + // With the fix it lands at the absolute manual position (11.4, 4.3). + expect(c1.center.x).toBeCloseTo(11.4, 1) + expect(c1.center.y).toBeCloseTo(4.3, 1) +}) + +test("manualEdits placement also works when parent group is at origin (no regression)", () => { + const { circuit } = getTestFixture() + + const manualEdits = { + pcb_placements: [ + { + selector: "C1", + center: { x: 5, y: 2 }, + relative_to: "group_center", + }, + ], + } as any + + circuit.add( + + + + + + , + ) + circuit.render() + + const c1 = (() => { + const src = circuit.db.source_component + .list() + .find((s) => s.name === "C1")! + return circuit.db.pcb_component + .list() + .find((p) => p.source_component_id === src.source_component_id)! + })() + + expect(c1.center.x).toBeCloseTo(5, 1) + expect(c1.center.y).toBeCloseTo(2, 1) +}) diff --git a/tests/pcb-packing/manual-edits-respected-by-packer.test.tsx b/tests/pcb-packing/manual-edits-respected-by-packer.test.tsx new file mode 100644 index 000000000..8968de056 --- /dev/null +++ b/tests/pcb-packing/manual-edits-respected-by-packer.test.tsx @@ -0,0 +1,91 @@ +import { expect, test } from "bun:test" +import { getTestFixture } from "tests/fixtures/get-test-fixture" + +// Repro for: components placed via the board's `manualEdits.pcb_placements` +// were applied during _computePcbGlobalTransformBeforeLayout but then the +// auto-packer re-positioned them, silently undoing the user's pin. +// +// The packer marks a child as "static" (skip-repack) only if +// isRelativelyPositioned() returns true — and that helper checks +// pcbX/pcbY/pcbLeftEdgeX/etc., NOT manualEdits. The fix walks each +// descendant of the group being packed and adds its pcb_component_id +// to staticPcbComponentIds when the nearest subcircuit resolves a +// manual placement for it. +// +// Mirrors the failing case in loco-boardv2: a chip inside an anchored +// region with several auto-placed bottom-side caps. Without the fix the +// inner packer dumps every cap on top of the chip's network-minimum +// point; with manualEdits and the fix, the caps land at the requested +// coordinates instead. +test("manualEdits placements survive the auto-packer (inner-group case)", () => { + const { circuit } = getTestFixture() + + const manualEdits = { + pcb_placements: [ + { selector: "C1", center: { x: 5, y: 3 }, relative_to: "group_center" }, + { selector: "C2", center: { x: -5, y: 3 }, relative_to: "group_center" }, + { selector: "C3", center: { x: 5, y: -3 }, relative_to: "group_center" }, + { selector: "C4", center: { x: -5, y: -3 }, relative_to: "group_center" }, + ], + } as any + + circuit.add( + + + + + + + + + , + ) + circuit.render() + + const named = (name: string) => { + const src = circuit.db.source_component + .list() + .find((s) => s.name === name)! + return circuit.db.pcb_component + .list() + .find((p) => p.source_component_id === src.source_component_id)! + } + + const c1 = named("C1") + const c2 = named("C2") + const c3 = named("C3") + const c4 = named("C4") + + expect(c1.center.x).toBeCloseTo(5, 1) + expect(c1.center.y).toBeCloseTo(3, 1) + expect(c2.center.x).toBeCloseTo(-5, 1) + expect(c2.center.y).toBeCloseTo(3, 1) + expect(c3.center.x).toBeCloseTo(5, 1) + expect(c3.center.y).toBeCloseTo(-3, 1) + expect(c4.center.x).toBeCloseTo(-5, 1) + expect(c4.center.y).toBeCloseTo(-3, 1) +})