Skip to content

Commit 010dc7d

Browse files
committed
fix(pro): review-pass corrections on the constraint/connector fixes
Findings from the adversarial review of the fix series itself: - ProAdvancedTab.buildInput now passes connectors — the 8th hand-built ModelData literal; without it every PRO advanced analysis (modal, buckling, P-Delta, time-history...) silently dropped connector stiffness while the fixed linear solve included it. - migrateConstraint: an unmappable legacy linearMPC term DOF now drops the whole constraint instead of silently rewriting it onto ux (rewriting changes the equation's meaning; consistent with how rigidLink/equalDOF filter and unknown kinds drop). - removeNode cascade is now bulkMutate-safe: the commit phase of bulkMutate overwrites model.constraints/model.loads with its pre-callback buffers, which would resurrect the pruned entries — the cascade prunes the buffers too (latent today: no current caller deletes nodes inside bulkMutate). - Node tool: the cursor node scan is computed once and shared by the auto-split guard and the duplicate-coincident guard (two identical scans that could silently diverge if one threshold is tuned). - Hinge-mode split gets the zero-length-element guard the extracted attemptSplit already had (tParam = NaN otherwise). - linearMPC input: ';' is now the documented term separator (',' still accepted), and a malformed fragment ABORTS the add instead of being dropped — with comma separators, a decimal comma in a coefficient ('-0,5') split as a term boundary and committed a silently-zeroed equation (pre-existing; the comma-tolerant parseNum cannot fix it at this call site because the separator is also a comma).
1 parent 247bb27 commit 010dc7d

4 files changed

Lines changed: 48 additions & 24 deletions

File tree

web/src/components/Viewport.svelte

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1176,7 +1176,8 @@
11761176
if (nearElem) {
11771177
const ni = modelStore.getNode(nearElem.nodeI);
11781178
const nj = modelStore.getNode(nearElem.nodeJ);
1179-
if (ni && nj) {
1179+
if (ni && nj && ((nj.x - ni.x) ** 2 + (nj.y - ni.y) ** 2) > 1e-10) {
1180+
// (zero-length guard: a degenerate element would make t NaN)
11801181
const edx = nj.x - ni.x;
11811182
const edy = nj.y - ni.y;
11821183
const lenSq = edx * edx + edy * edy;
@@ -1228,13 +1229,17 @@
12281229
resultsStore.clear();
12291230
return true;
12301231
};
1232+
// One shared scan: both the auto-split guard and the
1233+
// duplicate-coincident-node guard below answer the same question
1234+
// ('is the cursor on an existing node?') with the same threshold —
1235+
// two separate calls would silently diverge if one threshold is tuned.
1236+
const nodeAtCursor = findNearestNode(world.x, world.y, 0.5);
12311237
if (uiStore.autoSplitOnNodePlace) {
12321238
// Only auto-split when the cursor isn't already targeting an
12331239
// existing node. snapWithMidpoint returns node coords if a node is
12341240
// within nodeThreshold of the raw cursor; we re-check explicitly
12351241
// to keep the guard tight (also handles the same threshold).
1236-
const nearNode = findNearestNode(world.x, world.y, 0.5);
1237-
if (!nearNode) {
1242+
if (!nodeAtCursor) {
12381243
// 1st attempt — cursor-based: use the RAW cursor to find which
12391244
// element the user is pointing at (grid-snap can warp the cursor
12401245
// off the element line), but project the GRID-SNAPPED cursor so
@@ -1266,7 +1271,7 @@
12661271
// placement point `ms`: with grid snap on, `ms` can land exactly
12671272
// on an existing grid-aligned node that is >0.5m from the cursor —
12681273
// creating an exact coincident duplicate.
1269-
const onExisting = findNearestNode(world.x, world.y, 0.5)
1274+
const onExisting = nodeAtCursor
12701275
?? findNearestNode(ms.x, ms.y, 0.01);
12711276
if (onExisting) {
12721277
if (!uiStore.selectedNodes.has(onExisting.id)) {

web/src/components/pro/ProAdvancedTab.svelte

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,8 @@
7474
const input = buildSolverInput3D(
7575
{ nodes: modelStore.nodes, elements: modelStore.elements, supports: modelStore.supports,
7676
loads: modelStore.loads, materials: modelStore.materials, sections: modelStore.sections,
77-
quads: modelStore.quads, plates: modelStore.plates, constraints: modelStore.constraints },
77+
quads: modelStore.quads, plates: modelStore.plates, constraints: modelStore.constraints,
78+
connectors: modelStore.connectors },
7879
uiStore.includeSelfWeight,
7980
);
8081
if (!input) throw new Error(t('advanced.emptyModel'));

web/src/components/pro/ProConstraintsTab.svelte

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -136,20 +136,29 @@
136136
}
137137
138138
function addLinearMpc() {
139-
// Parse terms: "nodeId:dof:coefficient, ..." e.g. "1:ux:1.0, 2:ux:-1.0".
139+
// Parse terms: "nodeId:dof:coefficient; ..." e.g. "1:ux:1.0; 2:ux:-1.0".
140140
// Convert to the shape Rust expects: type discriminator `linearMPC`,
141141
// each term { nodeId, dof: usize-index, coefficient: f64 }. The constraint
142142
// sums to 0 by definition — no `rhs` field exists in LinearMPCConstraint.
143-
const parsed = mpcTerms.split(',').map(s => {
143+
//
144+
// Separator: ';' when present, ',' otherwise (legacy). With comma
145+
// separators a decimal comma in a coefficient ('-0,5') would be split as
146+
// a term boundary and the coefficient silently read as '-0' — so when
147+
// commas separate terms, a malformed fragment must ABORT the add (not be
148+
// dropped) to avoid committing a corrupted equation.
149+
const separator = mpcTerms.includes(';') ? ';' : ',';
150+
const fragments = mpcTerms.split(separator).filter(s => s.trim().length > 0);
151+
const parsed: Array<{ nodeId: number; dof: number; coefficient: number }> = [];
152+
for (const s of fragments) {
144153
const parts = s.trim().split(':');
145-
if (parts.length !== 3) return null;
154+
if (parts.length !== 3) return; // malformed fragment → reject the whole input
146155
const nodeId = parseInt(parts[0]);
147156
const dofName = parts[1].trim();
148157
const coefficient = parseNum(parts[2]);
149158
const dofIdx = dofLabels.indexOf(dofName as typeof dofLabels[number]);
150-
if (isNaN(nodeId) || isNaN(coefficient) || dofIdx < 0) return null;
151-
return { nodeId, dof: dofIdx, coefficient };
152-
}).filter(Boolean) as Array<{ nodeId: number; dof: number; coefficient: number }>;
159+
if (isNaN(nodeId) || isNaN(coefficient) || dofIdx < 0) return;
160+
parsed.push({ nodeId, dof: dofIdx, coefficient });
161+
}
153162
if (parsed.length === 0) return;
154163
modelStore.addConstraint({
155164
type: 'linearMPC',
@@ -403,7 +412,7 @@
403412

404413
{:else if selectedKind === 'linearMPC'}
405414
<div class="pro-cst-row">
406-
<label class="pro-label-wide">{t('pro.terms')}: <input type="text" bind:value={mpcTerms} placeholder="nodo:dof:coeff, ..." class="pro-input-wide" /></label>
415+
<label class="pro-label-wide">{t('pro.terms')}: <input type="text" bind:value={mpcTerms} placeholder="nodo:dof:coef; ... (ej: 1:ux:1; 2:ux:-1)" class="pro-input-wide" /></label>
407416
</div>
408417
<div class="pro-cst-hint">{t('pro.formatHint')}</div>
409418
{/if}

web/src/lib/store/model.svelte.ts

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -511,14 +511,18 @@ function createModelStore() {
511511
return { ...raw, type, dofs: mapDofs(raw.dofs) };
512512
case 'equalDOF':
513513
return { ...raw, type, dofs: mapDofs(raw.dofs) ?? [] };
514-
case 'linearMPC':
515-
return {
516-
...raw, type,
517-
terms: (raw.terms ?? []).map((t: any) => ({
518-
...t,
519-
dof: typeof t.dof === 'string' ? (LEGACY_DOF_NAME_TO_INDEX[t.dof] ?? 0) : t.dof,
520-
})),
521-
};
514+
case 'linearMPC': {
515+
const terms = (raw.terms ?? []).map((t: any) => ({
516+
...t,
517+
dof: typeof t.dof === 'string' ? LEGACY_DOF_NAME_TO_INDEX[t.dof] : t.dof,
518+
}));
519+
// An unmappable term DOF must drop the whole constraint — silently
520+
// rewriting it (e.g. to ux) would change the equation's meaning.
521+
if (terms.length === 0 || terms.some((t: any) => !Number.isInteger(t.dof) || t.dof < 0 || t.dof > 5)) {
522+
return null;
523+
}
524+
return { ...raw, type, terms };
525+
}
522526
case 'diaphragm':
523527
case 'eccentricConnection':
524528
return { ...raw, type };
@@ -1069,9 +1073,10 @@ function createModelStore() {
10691073
}
10701074
}
10711075
model.supports = new Map(model.supports);
1072-
model.loads = model.loads.filter(l =>
1073-
!((l.type === 'nodal' || l.type === 'nodal3d') && l.data.nodeId === id)
1074-
);
1076+
const keepLoad = (l: Load) =>
1077+
!((l.type === 'nodal' || l.type === 'nodal3d') && l.data.nodeId === id);
1078+
model.loads = model.loads.filter(keepLoad);
1079+
if (_bulkLoadBuffer) _bulkLoadBuffer = _bulkLoadBuffer.filter(keepLoad);
10751080
// Cascade to connectors/constraints: a dangling node reference is worse
10761081
// than a missing entity — the engine silently skips it (zero stiffness)
10771082
// while the connectivity preflight keeps crediting it as an edge.
@@ -1083,7 +1088,7 @@ function createModelStore() {
10831088
}
10841089
}
10851090
if (connectorsChanged) model.connectors = new Map(model.connectors);
1086-
model.constraints = model.constraints
1091+
const pruneConstraints = (arr: Constraint3D[]) => arr
10871092
.map((c): Constraint3D | null => {
10881093
if (c.type === 'diaphragm') {
10891094
if (c.masterNode === id) return null;
@@ -1099,6 +1104,10 @@ function createModelStore() {
10991104
return (c.masterNode === id || c.slaveNode === id) ? null : c;
11001105
})
11011106
.filter((c): c is Constraint3D => c !== null);
1107+
model.constraints = pruneConstraints(model.constraints);
1108+
// Inside bulkMutate the commit phase overwrites model.constraints with
1109+
// the buffer — prune the buffer too or dangling constraints resurrect.
1110+
if (_bulkConstraintBuffer) _bulkConstraintBuffer = pruneConstraints(_bulkConstraintBuffer);
11021111
},
11031112

11041113
removeElement(id: number): void {

0 commit comments

Comments
 (0)