Skip to content

[algorithm] Add some checks for null BaseProximity::SPtr #65

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
epernod merged 8 commits into from
Aug 18, 2025
Merged

[algorithm] Add some checks for null BaseProximity::SPtr #65

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a5d6c2d
[algorithm] shaftProx in if check and normalize
th-skam Aug 8, 2025
fb4e2c7
[algorithm] check for tipProx etc.
th-skam Aug 8, 2025
c8bd748
[algorithm] edgeProx in if check
th-skam Aug 8, 2025
8132f04
Merge branch 'master' into add-checks
epernod Aug 13, 2025
04a5255
[algorithm] Added comments
th-skam Aug 13, 2025
0fe1f15
Merge branch 'master' into add-checks
th-skam Aug 13, 2025
68e2a6c
[algorithm] More compact syntax for shaftProx check in loop
th-skam Aug 14, 2025
2b56205
[algorithm] Added a final check for coupling points removal
th-skam Aug 13, 2025
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
41 changes: 33 additions & 8 deletions src/sofa/collisionAlgorithm/algorithm/InsertionAlgorithm.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ class InsertionAlgorithm : public BaseAlgorithm
auto createTipProximity =
Operations::CreateCenterProximity::Operation::get(itTip->getTypeInfo());
const BaseProximity::SPtr tipProx = createTipProximity(itTip->element());
if (!tipProx) return;

// 2.1 Check whether coupling point should be added
const type::Vec3 tip2Pt = m_couplingPts.back()->getPosition() - tipProx->getPosition();
Expand Down Expand Up @@ -251,15 +252,32 @@ class InsertionAlgorithm : public BaseAlgorithm
auto createShaftProximity =
Operations::CreateCenterProximity::Operation::get(itShaft->getTypeInfo());
const BaseProximity::SPtr shaftProx = createShaftProximity(itShaft->element());
const EdgeProximity::SPtr edgeProx = dynamic_pointer_cast<EdgeProximity>(shaftProx);
const type::Vec3 normal = (edgeProx->element()->getP1()->getPosition() -
edgeProx->element()->getP0()->getPosition())
.normalized();
// If the coupling point lies ahead of the tip (positive dot product),
// the needle is retreating. The last coupling point is removed
if (dot(tip2Pt, normal) > 0_sreal)
if (shaftProx)
{
m_couplingPts.pop_back();
const EdgeProximity::SPtr edgeProx =
dynamic_pointer_cast<EdgeProximity>(shaftProx);
if (edgeProx)
{
const type::Vec3 normal = (edgeProx->element()->getP1()->getPosition() -
edgeProx->element()->getP0()->getPosition())
.normalized();
// If the (last) coupling point lies ahead of the tip (positive dot product),
// the needle is retreating. Thus, that point is removed.
if (dot(tip2Pt, normal) > 0_sreal)
{
m_couplingPts.pop_back();
}
}
else
{
msg_warning() << "shaftGeom: " << l_shaftGeom->getName()
<< " is not an EdgeGeometry. Point removal is disabled";
}
}
Copy link
Contributor

@epernod epernod Aug 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if edgeProx is null? should we sent a warning? or exit the loop?

Copy link
Collaborator Author

@th-skam th-skam Aug 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I placed 2 warnings eventually.

  1. Notifies when shaftProx is null. It's unlikely because the CreateCenterProximity operations are straightforward.
  2. Notifies when edgeProx is null. With a valid shaftProx, this will happen if l_shaftGeom is not an EdgeGeometry.

If the two repos were merged into one, we could use the normal handler from the ConstraintGeometry repo. It could help avoid this downcasting.

else
{
msg_warning() << "Cannot create proximity from shaftGeom: "
<< l_shaftGeom->getName() << " - point removal is disabled";
}
}
}
Expand All @@ -274,8 +292,15 @@ class InsertionAlgorithm : public BaseAlgorithm
{
const BaseProximity::SPtr shaftProx = findClosestProxOnShaft(
m_couplingPts[i], l_shaftGeom.get(), projectOnShaft, getFilterFunc());
if (!shaftProx) continue;
shaftProx->normalize();
insertionOutput.add(shaftProx, m_couplingPts[i]);
}
// This is a final-frontier check: If there are coupling points stored, but the
// findClosestProxOnShaf operation yields no proximities on the shaft, it could be
// because the needle has exited abruptly. Thus, we clear the coupling points.
if (insertionOutput.size() == 0)
m_couplingPts.clear();
}

d_collisionOutput.endEdit();
Expand Down
Loading