From f81b8b031a138799c154bc28c8a5fd01fa1e00bb Mon Sep 17 00:00:00 2001 From: Raghav Arora Date: Sat, 25 Oct 2025 01:47:07 +0530 Subject: [PATCH 01/23] Fix schPinSpacing changing the width of the schematic box - Changed width/height calculation to use fixed MIN_PADDING (0.4) instead of schPinSpacing * 2 - This ensures box dimensions only change based on pin positions, not pin spacing - Added tests to verify width/height remain constant when schPinSpacing changes - Updated snapshots for affected tests Fixes #1576 --- PR-DESCRIPTION.md | 34 ++++++++++ .../getAllDimensionsForSchematicBox.ts | 18 +++-- reproduce-issue.md | 67 +++++++++++++++++++ ...when-schPortArrangement-schematic.snap.svg | 12 +++- ...ulator-with-connections-schematic.snap.svg | 40 +++++------ ...spacing-width-leftright-schematic.snap.svg | 25 +++++++ ...spacing-width-topbottom-schematic.snap.svg | 25 +++++++ ...pro3-usb-port-footprint-schematic.snap.svg | 9 ++- ...pro-schpinspacing-width-leftright.test.tsx | 51 ++++++++++++++ ...pro-schpinspacing-width-topbottom.test.tsx | 51 ++++++++++++++ 10 files changed, 304 insertions(+), 28 deletions(-) create mode 100644 PR-DESCRIPTION.md create mode 100644 reproduce-issue.md create mode 100644 tests/repros/__snapshots__/repro-schpinspacing-width-leftright-schematic.snap.svg create mode 100644 tests/repros/__snapshots__/repro-schpinspacing-width-topbottom-schematic.snap.svg create mode 100644 tests/repros/repro-schpinspacing-width-leftright.test.tsx create mode 100644 tests/repros/repro-schpinspacing-width-topbottom.test.tsx diff --git a/PR-DESCRIPTION.md b/PR-DESCRIPTION.md new file mode 100644 index 000000000..467785da8 --- /dev/null +++ b/PR-DESCRIPTION.md @@ -0,0 +1,34 @@ +Fixes #1576 + +## Reproduction + +Added tests that demonstrate the bug. Before the fix, changing schPinSpacing from 0.2 to 0.8 incorrectly changed box dimensions even when there were no pins on the affected sides. + +Test output before fix: +``` +Small spacing - width: 0.4 height: 1 +Large spacing - width: 1.6 height: 4 +Test FAILED - width changed from 0.4 to 1.6 +``` + +## Fix + +Changed width/height calculation in getAllDimensionsForSchematicBox.ts to use fixed MIN_PADDING (0.4) instead of schPinSpacing * 2. This ensures box dimensions are determined by pin positions, not pin spacing. + +## Result + +After fix, box dimensions stay constant when schPinSpacing changes: + +``` +Small spacing - width: 0.4 height: 1 +Large spacing - width: 0.4 height: 2.8 +Tests PASS - width stays at 0.4 +``` + +The schPinSpacing parameter now only affects spacing between pins, not the overall box size. + +## Tests + +- Added repro-schpinspacing-width-leftright.test.tsx +- Added repro-schpinspacing-width-topbottom.test.tsx +- Updated snapshots for affected tests diff --git a/lib/utils/schematic/getAllDimensionsForSchematicBox.ts b/lib/utils/schematic/getAllDimensionsForSchematicBox.ts index 85489d849..0314f040f 100644 --- a/lib/utils/schematic/getAllDimensionsForSchematicBox.ts +++ b/lib/utils/schematic/getAllDimensionsForSchematicBox.ts @@ -346,9 +346,14 @@ export const getAllDimensionsForSchematicBox = ( // Use lengths to determine schWidth and schHeight let schWidth = params.schWidth if (schWidth === undefined) { + // Use fixed padding based on default schPinSpacing (0.2) to ensure + // the box width doesn't change when schPinSpacing changes + // This ensures box dimensions are determined by pin positions, not pin spacing + const DEFAULT_PIN_SPACING = 0.2 + const MIN_PADDING = DEFAULT_PIN_SPACING * 2 schWidth = Math.max( - sideLengths.top + params.schPinSpacing * 2, - sideLengths.bottom + params.schPinSpacing * 2, + sideLengths.top + MIN_PADDING, + sideLengths.bottom + MIN_PADDING, ) const labelWidth = params.pinLabels @@ -366,9 +371,14 @@ export const getAllDimensionsForSchematicBox = ( let schHeight = params.schHeight if (!schHeight) { + // Use fixed padding based on default schPinSpacing (0.2) to ensure + // the box height doesn't change when schPinSpacing changes + // This ensures box dimensions are determined by pin positions, not pin spacing + const DEFAULT_PIN_SPACING = 0.2 + const MIN_PADDING = DEFAULT_PIN_SPACING * 2 schHeight = Math.max( - sideLengths.left + params.schPinSpacing * 2, - sideLengths.right + params.schPinSpacing * 2, + sideLengths.left + MIN_PADDING, + sideLengths.right + MIN_PADDING, ) } diff --git a/reproduce-issue.md b/reproduce-issue.md new file mode 100644 index 000000000..af134f853 --- /dev/null +++ b/reproduce-issue.md @@ -0,0 +1,67 @@ +# Reproducing the schPinSpacing Width Bug + +## Issue +The `schPinSpacing` parameter was incorrectly affecting the width/height of the schematic box, even when there were no pins on the sides that should determine that dimension. + +## Reproduction Steps + +### 1. Run the test BEFORE the fix + +```bash +# Checkout the commit before the fix +git checkout + +# Run the test +bun test tests/repros/repro-schpinspacing-width.test.tsx +``` + +**Expected Output (FAILING):** +``` +Small spacing - width: 0.4 height: 1 +Large spacing - width: 1.6 height: 4 + +✗ schPinSpacing should not change the width of the schematic box + Expected: 1.6 + Received: 0.4 +``` + +This shows the bug: width changes from 0.4 to 1.6 when schPinSpacing changes from 0.2 to 0.8. + +### 2. Apply the fix + +The fix is in `lib/utils/schematic/getAllDimensionsForSchematicBox.ts`: + +**Before:** +```typescript +schWidth = Math.max( + sideLengths.top + params.schPinSpacing * 2, + sideLengths.bottom + params.schPinSpacing * 2, +) +``` + +**After:** +```typescript +const MIN_PADDING = 0.4 +schWidth = Math.max( + sideLengths.top + MIN_PADDING, + sideLengths.bottom + MIN_PADDING, +) +``` + +### 3. Run the test AFTER the fix + +```bash +# Run the test again +bun test tests/repros/repro-schpinspacing-width.test.tsx +``` + +**Expected Output (PASSING):** +``` +Small spacing - width: 0.4 height: 1 +Large spacing - width: 0.4 height: 2.8 + +✓ schPinSpacing should not change the width of the schematic box +✓ schPinSpacing should not change width when pins are on top/bottom +``` + +Now the width stays at 0.4 regardless of schPinSpacing (because there are no top/bottom pins). diff --git a/tests/components/normal-components/__snapshots__/chip-override-footprint-ports-when-schPortArrangement-schematic.snap.svg b/tests/components/normal-components/__snapshots__/chip-override-footprint-ports-when-schPortArrangement-schematic.snap.svg index 2864cac4a..40ab4e502 100644 --- a/tests/components/normal-components/__snapshots__/chip-override-footprint-ports-when-schPortArrangement-schematic.snap.svg +++ b/tests/components/normal-components/__snapshots__/chip-override-footprint-ports-when-schPortArrangement-schematic.snap.svg @@ -1,17 +1,25 @@ --2,-1-2,0-2,1-1,-1-1,0-1,10,-10,00,11,-11,01,12,-12,02,1ESP321pos2neg \ No newline at end of file diff --git a/tests/examples/__snapshots__/example7-voltage-regulator-with-connections-schematic.snap.svg b/tests/examples/__snapshots__/example7-voltage-regulator-with-connections-schematic.snap.svg index 010448c3d..c02ce67dd 100644 --- a/tests/examples/__snapshots__/example7-voltage-regulator-with-connections-schematic.snap.svg +++ b/tests/examples/__snapshots__/example7-voltage-regulator-with-connections-schematic.snap.svg @@ -1,4 +1,4 @@ --2,-5-2,-4-2,-3-2,-2-2,-1-2,0-2,1-2,2-2,3-2,4-2,5-1,-5-1,-4-1,-3-1,-2-1,-1-1,0-1,1-1,2-1,3-1,4-1,50,-50,-40,-30,-20,-10,00,10,20,30,40,51,-51,-41,-31,-21,-11,01,11,21,31,41,52,-52,-42,-32,-22,-12,02,12,22,32,42,5U1235B86A57B78A69A710B611A812B513A114B1215A416B917B418A919B114 \ No newline at end of file + -2,-5-2,-4-2,-3-2,-2-2,-1-2,0-2,1-2,2-2,3-2,4-2,5-1,-5-1,-4-1,-3-1,-2-1,-1-1,0-1,1-1,2-1,3-1,4-1,50,-50,-40,-30,-20,-10,00,10,20,30,40,51,-51,-41,-31,-21,-11,01,11,21,31,41,52,-52,-42,-32,-22,-12,02,12,22,32,42,5U1235B86A57B78A69A710B611A812B513A114B1215A416B917B418A919B114 \ No newline at end of file From c6f94c47c1eb66a73045bae1b600350ee8c7a561 Mon Sep 17 00:00:00 2001 From: Raghav Arora Date: Mon, 27 Oct 2025 04:55:41 +0530 Subject: [PATCH 16/23] Update snapshot for chip-layer-flip test --- .../__snapshots__/chip-layer-flip-schematic.snap.svg | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/components/normal-components/__snapshots__/chip-layer-flip-schematic.snap.svg b/tests/components/normal-components/__snapshots__/chip-layer-flip-schematic.snap.svg index 2e192765f..3b380b0af 100644 --- a/tests/components/normal-components/__snapshots__/chip-layer-flip-schematic.snap.svg +++ b/tests/components/normal-components/__snapshots__/chip-layer-flip-schematic.snap.svg @@ -1,4 +1,4 @@ --2,-4-2,-3-2,-2-2,-1-2,0-2,1-2,2-1,-4-1,-3-1,-2-1,-1-1,0-1,1-1,20,-40,-30,-20,-10,00,10,21,-41,-31,-21,-11,01,11,22,-42,-32,-22,-12,02,12,2U1123U2123 \ No newline at end of file From 3b93c88b042e344c1e1a0e7b82d1b09bc1362b69 Mon Sep 17 00:00:00 2001 From: Raghav Arora Date: Mon, 27 Oct 2025 10:58:04 +0530 Subject: [PATCH 17/23] refactor: use selectAll('port') and getNameAndAliases() for pin labels --- .../NormalComponent/NormalComponent.ts | 48 ++++++++----------- 1 file changed, 20 insertions(+), 28 deletions(-) diff --git a/lib/components/base-components/NormalComponent/NormalComponent.ts b/lib/components/base-components/NormalComponent/NormalComponent.ts index 0d232a8fc..14a64457e 100644 --- a/lib/components/base-components/NormalComponent/NormalComponent.ts +++ b/lib/components/base-components/NormalComponent/NormalComponent.ts @@ -1234,35 +1234,27 @@ export class NormalComponent< } /** - * Extract port labels from footprint portHints to be displayed in schematic - * Only includes portHints that are meaningful labels (not generic "pin1", "pin2", etc.) + * Extract pin labels from ports using existing Port logic */ - _getPortLabelsFromFootprint(): Record { - const portLabels: Record = {} - - // Find the Footprint component in children (it's been instantiated by now) - const footprint = this.children.find( - (c) => c.componentName === "Footprint", - ) as Footprint | undefined - - if (!footprint) return portLabels - - let pinNumber = 1 - for (const fpChild of footprint.children) { - if (!fpChild.props.portHints) continue - - const portHints = fpChild.props.portHints as string[] - // Use the first portHint that's not a generic "pinX" format - for (const hint of portHints) { - if (!hint.match(/^pin\d+$/)) { - portLabels[String(pinNumber)] = hint - break + _getPinLabelsFromPorts(): Record { + const ports = this.selectAll("port") + const pinLabels: Record = {} + + for (const port of ports) { + const pinNumber = port.props.pinNumber + if (pinNumber !== undefined) { + const names = port.getNameAndAliases() + // Filter out generic patterns like "pin1", "1", etc. (same logic as Port.doInitialSchematicPortRender) + const meaningfulNames = names.filter( + (name) => !name.match(/^(pin)?\d+$/), + ) + if (meaningfulNames.length > 0) { + pinLabels[`pin${pinNumber}`] = meaningfulNames[0] } } - pinNumber++ } - return portLabels + return pinLabels } _getSchematicBoxDimensions(): SchematicBoxDimensions | null { @@ -1276,11 +1268,11 @@ export class NormalComponent< const pinSpacing = props.schPinSpacing ?? 0.2 - // Merge portHints-based labels with explicit pinLabels - const portLabelsFromFootprint = this._getPortLabelsFromFootprint() + // Merge port-based labels with explicit pinLabels + const portLabelsFromPorts = this._getPinLabelsFromPorts() const mergedPinLabels = { - ...portLabelsFromFootprint, - ...props.pinLabels, // Explicit pinLabels override footprint labels + ...portLabelsFromPorts, + ...props.pinLabels, // Explicit pinLabels override port labels } const dimensions = getAllDimensionsForSchematicBox({ From bf929577548c55c39eb20b9656e7e94b39931b0b Mon Sep 17 00:00:00 2001 From: Raghav Arora Date: Mon, 27 Oct 2025 11:11:57 +0530 Subject: [PATCH 18/23] test: update snapshots for tests with numeric portHints --- .../__snapshots__/chip-layer-flip-schematic.snap.svg | 2 +- .../repro16-sch-box-no-pin-labels-schematic.snap.svg | 12 ++++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/tests/components/normal-components/__snapshots__/chip-layer-flip-schematic.snap.svg b/tests/components/normal-components/__snapshots__/chip-layer-flip-schematic.snap.svg index 3b380b0af..a3c31c681 100644 --- a/tests/components/normal-components/__snapshots__/chip-layer-flip-schematic.snap.svg +++ b/tests/components/normal-components/__snapshots__/chip-layer-flip-schematic.snap.svg @@ -22,4 +22,4 @@ .pin-number { fill: rgb(169, 0, 0); } .port-label { fill: rgb(0, 100, 100); } .component-name { fill: rgb(0, 100, 100); } - -2,-4-2,-3-2,-2-2,-1-2,0-2,1-2,2-1,-4-1,-3-1,-2-1,-1-1,0-1,1-1,20,-40,-30,-20,-10,00,10,21,-41,-31,-21,-11,01,11,22,-42,-32,-22,-12,02,12,2U1123U2123 \ No newline at end of file + -2,-4-2,-3-2,-2-2,-1-2,0-2,1-2,2-1,-4-1,-3-1,-2-1,-1-1,0-1,1-1,20,-40,-30,-20,-10,00,10,21,-41,-31,-21,-11,01,11,22,-42,-32,-22,-12,02,12,2U1123U2123 \ No newline at end of file diff --git a/tests/repros/__snapshots__/repro16-sch-box-no-pin-labels-schematic.snap.svg b/tests/repros/__snapshots__/repro16-sch-box-no-pin-labels-schematic.snap.svg index 0b6c6d810..312938737 100644 --- a/tests/repros/__snapshots__/repro16-sch-box-no-pin-labels-schematic.snap.svg +++ b/tests/repros/__snapshots__/repro16-sch-box-no-pin-labels-schematic.snap.svg @@ -1,17 +1,25 @@ --2,-1-2,0-2,1-1,-1-1,0-1,10,-10,00,11,-11,01,12,-12,02,1U112 \ No newline at end of file From f56aee4fb0d0b87be5e4e1ee2d10f62cdeecce1f Mon Sep 17 00:00:00 2001 From: Raghav Arora Date: Mon, 27 Oct 2025 11:15:01 +0530 Subject: [PATCH 19/23] refactor: remove redundant pinLabels merging logic --- .../NormalComponent/NormalComponent.ts | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/lib/components/base-components/NormalComponent/NormalComponent.ts b/lib/components/base-components/NormalComponent/NormalComponent.ts index 14a64457e..49a174bd4 100644 --- a/lib/components/base-components/NormalComponent/NormalComponent.ts +++ b/lib/components/base-components/NormalComponent/NormalComponent.ts @@ -1268,12 +1268,7 @@ export class NormalComponent< const pinSpacing = props.schPinSpacing ?? 0.2 - // Merge port-based labels with explicit pinLabels - const portLabelsFromPorts = this._getPinLabelsFromPorts() - const mergedPinLabels = { - ...portLabelsFromPorts, - ...props.pinLabels, // Explicit pinLabels override port labels - } + const pinLabels = this._getPinLabelsFromPorts() const dimensions = getAllDimensionsForSchematicBox({ schWidth: props.schWidth, @@ -1281,13 +1276,13 @@ export class NormalComponent< schPinSpacing: pinSpacing, numericSchPinStyle: getNumericSchPinStyle( props.schPinStyle, - mergedPinLabels, + pinLabels, ), pinCount, schPortArrangement: this._getSchematicPortArrangement()!, - pinLabels: mergedPinLabels, + pinLabels, }) return dimensions From d128a8fab7441e4677485869d3bd1d65f274e16a Mon Sep 17 00:00:00 2001 From: Raghav Arora Date: Mon, 27 Oct 2025 11:16:41 +0530 Subject: [PATCH 20/23] bun format --- .../base-components/NormalComponent/NormalComponent.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/components/base-components/NormalComponent/NormalComponent.ts b/lib/components/base-components/NormalComponent/NormalComponent.ts index 49a174bd4..b4933fe27 100644 --- a/lib/components/base-components/NormalComponent/NormalComponent.ts +++ b/lib/components/base-components/NormalComponent/NormalComponent.ts @@ -1274,10 +1274,7 @@ export class NormalComponent< schWidth: props.schWidth, schHeight: props.schHeight, schPinSpacing: pinSpacing, - numericSchPinStyle: getNumericSchPinStyle( - props.schPinStyle, - pinLabels, - ), + numericSchPinStyle: getNumericSchPinStyle(props.schPinStyle, pinLabels), pinCount, From 232618b854a7560c998ebef460db0047be190852 Mon Sep 17 00:00:00 2001 From: Raghav Arora Date: Mon, 27 Oct 2025 11:35:46 +0530 Subject: [PATCH 21/23] fix: keep props.pinLabels for label-to-pin-number mapping and update snapshots --- .../NormalComponent/NormalComponent.ts | 14 +++++-- .../__snapshots__/header-schematic.snap.svg | 12 +++++- ...e-regulator-match-adapt-schematic.snap.svg | 16 ++++---- .../__snapshots__/index-schematic.snap.svg | 40 +++++++++---------- ...49-same-net-not-combine-schematic.snap.svg | 8 ++-- 5 files changed, 53 insertions(+), 37 deletions(-) diff --git a/lib/components/base-components/NormalComponent/NormalComponent.ts b/lib/components/base-components/NormalComponent/NormalComponent.ts index b4933fe27..40a4ecf4d 100644 --- a/lib/components/base-components/NormalComponent/NormalComponent.ts +++ b/lib/components/base-components/NormalComponent/NormalComponent.ts @@ -1268,18 +1268,26 @@ export class NormalComponent< const pinSpacing = props.schPinSpacing ?? 0.2 - const pinLabels = this._getPinLabelsFromPorts() + const pinLabelsFromPorts = this._getPinLabelsFromPorts() + // Merge with props.pinLabels for label-to-pin-number mapping + const allPinLabels = { + ...pinLabelsFromPorts, + ...props.pinLabels, + } const dimensions = getAllDimensionsForSchematicBox({ schWidth: props.schWidth, schHeight: props.schHeight, schPinSpacing: pinSpacing, - numericSchPinStyle: getNumericSchPinStyle(props.schPinStyle, pinLabels), + numericSchPinStyle: getNumericSchPinStyle( + props.schPinStyle, + allPinLabels, + ), pinCount, schPortArrangement: this._getSchematicPortArrangement()!, - pinLabels, + pinLabels: allPinLabels, }) return dimensions diff --git a/tests/components/normal-components/__snapshots__/header-schematic.snap.svg b/tests/components/normal-components/__snapshots__/header-schematic.snap.svg index 81b2e075f..c4f30f4a9 100644 --- a/tests/components/normal-components/__snapshots__/header-schematic.snap.svg +++ b/tests/components/normal-components/__snapshots__/header-schematic.snap.svg @@ -1,17 +1,25 @@ --2,-2-2,-1-2,0-2,1-2,2-1,-2-1,-1-1,0-1,1-1,20,-20,-10,00,10,21,-21,-11,01,11,22,-22,-12,02,12,2J11label12label23label34label4 \ No newline at end of file diff --git a/tests/features/schematic-match-adapt/__snapshots__/voltage-regulator-match-adapt-schematic.snap.svg b/tests/features/schematic-match-adapt/__snapshots__/voltage-regulator-match-adapt-schematic.snap.svg index 8d4b00bab..b6c44ca8f 100644 --- a/tests/features/schematic-match-adapt/__snapshots__/voltage-regulator-match-adapt-schematic.snap.svg +++ b/tests/features/schematic-match-adapt/__snapshots__/voltage-regulator-match-adapt-schematic.snap.svg @@ -1,4 +1,4 @@ --6,-2-6,-1-6,0-6,1-6,2-6,3-5,-2-5,-1-5,0-5,1-5,2-5,3-4,-2-4,-1-4,0-4,1-4,2-4,3-3,-2-3,-1-3,0-3,1-3,2-3,3-2,-2-2,-1-2,0-2,1-2,2-2,3-1,-2-1,-1-1,0-1,1-1,2-1,30,-20,-10,00,10,20,31,-21,-11,01,11,21,32,-22,-12,02,12,22,3C62.2uFC12.2uFC22.2uFC51uFRT9013_33GBU11VIN2GND3EN4NC5VOUTVSYSGNDGNDV3_3 \ No newline at end of file diff --git a/tests/projects/seveibar__rp2040-zero/__snapshots__/index-schematic.snap.svg b/tests/projects/seveibar__rp2040-zero/__snapshots__/index-schematic.snap.svg index 39f8775ad..a31ebec9c 100644 --- a/tests/projects/seveibar__rp2040-zero/__snapshots__/index-schematic.snap.svg +++ b/tests/projects/seveibar__rp2040-zero/__snapshots__/index-schematic.snap.svg @@ -1,4 +1,4 @@ --2,-1-2,0-2,1-1,-1-1,0-1,10,-10,00,11,-11,01,12,-12,02,1U112 \ No newline at end of file + -2,-1-2,0-2,1-1,-1-1,0-1,10,-10,00,11,-11,01,12,-12,02,1U112 \ No newline at end of file From 2ee729265c2848d7ed14ab7899ca9cafd44a6f17 Mon Sep 17 00:00:00 2001 From: Raghav Arora Date: Mon, 27 Oct 2025 23:02:19 +0530 Subject: [PATCH 23/23] fix: add type cast for Port in _getPinLabelsFromPorts --- .../base-components/NormalComponent/NormalComponent.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/components/base-components/NormalComponent/NormalComponent.ts b/lib/components/base-components/NormalComponent/NormalComponent.ts index bc16130f8..1f94c922d 100644 --- a/lib/components/base-components/NormalComponent/NormalComponent.ts +++ b/lib/components/base-components/NormalComponent/NormalComponent.ts @@ -1237,7 +1237,7 @@ export class NormalComponent< * Extract pin labels from ports using existing Port logic */ _getPinLabelsFromPorts(): Record { - const ports = this.selectAll("port") + const ports = this.selectAll("port") as Port[] const pinLabels: Record = {} for (const port of ports) {