Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<group pcbX={16}>`
// 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),
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
// `<group pcbX={16}>` 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(
Expand Down
100 changes: 100 additions & 0 deletions tests/pcb-packing/manual-edits-position-no-double-count.test.tsx
Original file line number Diff line number Diff line change
@@ -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
// `<group pcbX={16}>` 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(
<board width="40mm" height="20mm" manualEdits={manualEdits}>
<group name="region" pcbX={16} pcbY={0}>
<chip
name="U1"
footprint="soic8"
pinLabels={{ pin1: "VCC", pin8: "GND" }}
/>
<capacitor
name="C1"
capacitance="100nF"
footprint="0402"
connections={{ pin1: "U1.VCC", pin2: "U1.GND" }}
/>
</group>
</board>,
)
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(
<board width="20mm" height="10mm" manualEdits={manualEdits}>
<group name="region" pcbX={0} pcbY={0}>
<chip
name="U1"
footprint="soic8"
pinLabels={{ pin1: "VCC", pin8: "GND" }}
/>
<capacitor
name="C1"
capacitance="100nF"
footprint="0402"
connections={{ pin1: "U1.VCC", pin2: "U1.GND" }}
/>
</group>
</board>,
)
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)
})
91 changes: 91 additions & 0 deletions tests/pcb-packing/manual-edits-respected-by-packer.test.tsx
Original file line number Diff line number Diff line change
@@ -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(
<board width="20mm" height="10mm" manualEdits={manualEdits}>
<group name="region" pcbX={0} pcbY={0}>
<chip
name="U1"
footprint="soic8"
pinLabels={{ pin1: "VCC", pin8: "GND" }}
/>
<capacitor
name="C1"
capacitance="100nF"
footprint="0402"
connections={{ pin1: "U1.VCC", pin2: "U1.GND" }}
/>
<capacitor
name="C2"
capacitance="100nF"
footprint="0402"
connections={{ pin1: "U1.VCC", pin2: "U1.GND" }}
/>
<capacitor
name="C3"
capacitance="100nF"
footprint="0402"
connections={{ pin1: "U1.VCC", pin2: "U1.GND" }}
/>
<capacitor
name="C4"
capacitance="100nF"
footprint="0402"
connections={{ pin1: "U1.VCC", pin2: "U1.GND" }}
/>
</group>
</board>,
)
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)
})
Loading