Skip to content

WireRestShape::initTopology corrupts topology for N≥3 sections #225

@lkarstensen

Description

@lkarstensen

Summary

WireRestShape::initTopology builds the visual EdgeSetTopologyContainer by iterating over section materials and placing points along the wire's curvilinear abscissa. Two accumulator variables are updated via assignment instead of accumulation:

  • prev_length = length (should be prev_length += length)
  • prev_edges = nbrVisuEdges (should be prev_edges += nbrVisuEdges)

For any wire with three or more sections this collapses point coordinates from section 2 onward and corrupts edge indices, producing degenerate edges and a scene-init failure (or silently malformed visual topology).

Two-section wires are unaffected because after the first iteration the assignment and accumulation forms produce the same value.

Affected component

  • File: src/BeamAdapter/component/engine/WireRestShape.inl
  • Method: WireRestShape<DataTypes>::initTopology
  • Confirmed in: release-v24.12 and master

The bug

Real prev_length = 0.0;
int prev_edges = 0;
int startPtId = 0;
for (sofa::Size i = 0; i < l_sectionMaterials.size(); ++i)
{
    int nbrVisuEdges = l_sectionMaterials.get(i)->getNbVisualEdges();
    Real length = fabs(keyPts[i + 1] - keyPts[i]);
    Real dx = length / nbrVisuEdges;

    for (int i = startPtId; i < nbrVisuEdges + 1; i++) {
        l_topology->addPoint(prev_length + i * dx, 0, 0);
    }
    for (int i = prev_edges; i < prev_edges + nbrVisuEdges; i++) {
        l_topology->addEdge(i, i + 1);
    }

    prev_length = length;        // BUG: should be prev_length += length
    prev_edges = nbrVisuEdges;   // BUG: should be prev_edges += nbrVisuEdges
    startPtId = 1;
}
  • length is the local length of the current section (fabs(keyPts[i+1] - keyPts[i])).
  • prev_length is intended to be the global cumulative offset for point placement — assigning resets it to just the last section length.
  • prev_edges is intended to be the global edge index offset — assigning resets it to just the last section's count, causing edges for section 2+ to overwrite indices already used by earlier sections.

Why N=2 works

After iteration 0, both prev_length = length_0 and prev_length += length_0 yield the same value (starting from 0). The discrepancy only appears from iteration 2 onward.

Worked example (3 sections of 100, 50, 25 mm, 10/5/4 edges each)

Iter length prev_length on entry (buggy) Point range (buggy) Point range (correct)
0 100 0 0 → 100 ✓ 0 → 100
1 50 100 100 → 150 ✓ 100 → 150
2 25 50 (bug) 50 → 75 ✗ 150 → 175

Iteration 2 places its points at curvilinear abscissa 50–75 mm, overlapping section 1. Additionally, edges for section 2 are added at global indices 5–8 instead of 15–18, corrupting the topology.

keyPoints is correct

WireRestShape::initLengths uses proper accumulation:

keyPointList[i + 1] = keyPointList[i] + rodSection->getLength();

Mechanical sampling paths (getMechanicalSampling, getCollisionSampling, getRestTransformOnX, getBeamSectionAtX) all consult keyPts directly and are unaffected. Only initTopology, which builds the visual edge set, is broken.

Proposed fix

Two one-line changes:

prev_length += length;       // was: prev_length = length
prev_edges += nbrVisuEdges;  // was: prev_edges = nbrVisuEdges

Zero effect on N≤2 wires (already produces identical output).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions