-
Notifications
You must be signed in to change notification settings - Fork 311
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.
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
[SofaBaseMechanics] Add topological change in barycentric mapping #1203
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your PR @courtecuisse 👍
Is there any reason why you put all your code in headers?
If no, could you please move it to .cpp files?
void processTopologicalChanges(const typename Out::VecCoord& out, const typename In::VecCoord& in, core::topology::Topology* t) { | ||
using sofa::core::behavior::MechanicalState; | ||
|
||
if (t != this->m_toTopology) return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace the "this" with using Inherit1::m_topology
std::list<const core::topology::TopologyChange *>::const_iterator itBegin = this->m_toTopology->beginChange(); | ||
std::list<const core::topology::TopologyChange *>::const_iterator itEnd = this->m_toTopology->endChange(); | ||
|
||
helper::vector<MappingData>& vectorData = *(d_map.beginEdit()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use WriteAccessor instead of beginEdit()/endEdit() this is shorter to write and more safer with respect to security.
@@ -68,6 +68,107 @@ class BarycentricMapperTetrahedronSetTopology : public BarycentricMapperTopology | |||
void computeDistance(double& d, const Vector3& v) override; | |||
void addPointInElement(const int elementIndex, const SReal* baryCoords) override; | |||
|
|||
//handle topology changes depending on the topology |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No code in .h this increase code coupling and thus compilation time.
if ( this->m_toTopology->beginChange() == this->m_toTopology->endChange() ) | ||
return; | ||
|
||
std::list<const core::topology::TopologyChange *>::const_iterator itBegin = this->m_toTopology->beginChange(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case the use of "auto" is recommanded.
If you have no time I will do the polishing of this PR. |
…eader to inl file.
@damienmarchal @courtecuisse I have tested the mapping and done the change requested but I'm not able to push the change in mimesis-inria/sofa , any idea why? |
@epernod Your commits are pushed 👍 |
[ci-build][with-all-tests] |
Modify the mapper in tetra barycentric mapping in order to listen for topological changes and only compute the added points. Previously the behaviour was to call reinit and recompute all the barycentric coordinates for the entire mesh at each topological modification.
This PR:
Reviewers will merge only if all these checks are true.