Skip to content

[algorithm] Remove projection operation on tip #95

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 2 commits into from
Sep 24, 2025
Merged

[algorithm] Remove projection operation on tip #95

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dde0315
[algorithm] Remove projection operation on tip in the InsertionAlgori…
th-skam Sep 18, 2025
9b6039c
Merge branch 'master' into improve-projective
epernod Sep 22, 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
9 changes: 1 addition & 8 deletions src/CollisionAlgorithm/algorithm/InsertionAlgorithm.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@ class InsertionAlgorithm : public BaseAlgorithm
auto projectOnSurf = Operations::Project::Operation::get(l_surfGeom);
auto projectOnTip = Operations::Project::Operation::get(l_tipGeom);

const bool isProjective = d_projective.getValue();
const SReal punctureForceThreshold = d_punctureForceThreshold.getValue();
for (auto itTip = l_tipGeom->begin(); itTip != l_tipGeom->end(); itTip++)
{
Expand Down Expand Up @@ -167,13 +166,7 @@ class InsertionAlgorithm : public BaseAlgorithm
}
}

// 1.2 If not, create a proximity pair for the tip-surface collision
if (isProjective)
{
tipProx = projectOnTip(surfProx->getPosition(), itTip->element()).prox;
if (!tipProx) continue;
tipProx->normalize();
Copy link
Contributor

@epernod epernod Sep 22, 2025

Choose a reason for hiding this comment

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

are you sure this normalization was never useful for the rest of the algo?

if yes, ok for me. Let's double check ci as well.

Copy link
Collaborator Author

@th-skam th-skam Sep 22, 2025

Choose a reason for hiding this comment

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

Yes, it does nothing. Check the implementation:

CollisionAlgorithm/src/CollisionAlgorithm/proximity/PointProximity.h

Lines 45 to 47 in d758a81

bool isNormalized() const override { return true; }
void normalize() override {}

normalize() is empty and isNormalized() always returns true.

Copy link
Contributor

@epernod epernod Sep 22, 2025

Choose a reason for hiding this comment

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

hum... and is it normal that it is empty?

Copy link
Collaborator Author

@th-skam th-skam Sep 22, 2025

Choose a reason for hiding this comment

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

Yes it should be because the normalization simply corrects the projection by placing the proximity inside the simplex. I guess it's there because the projection can in fact shoot the new point outside the simplex. But the PointProximity is the most trivial case. Projecting on a point simply means "use that point".

Check the EdgeProximity implementation:

CollisionAlgorithm/src/CollisionAlgorithm/proximity/EdgeProximity.h

Lines 66 to 78 in d758a81

bool isNormalized() const override {
// if (m_f0+m_f1 != 1.0) return false;
return m_f0>=0 && m_f0<=1 &&
m_f1>=0 && m_f1<=1;
}
void normalize() override {
if (m_f1<0.0) m_f1 = 0.0;
else if (m_f1>1.0) m_f1 = 1.0;
m_f0 = 1.0-m_f1;
}

m_f0 and m_f1 are the bary coords.

}
// ... if not, create a proximity pair for the tip-surface collision
collisionOutput.add(tipProx, surfProx);
}
}
Expand Down
Loading