Skip to content

Conversation

@adagolodjo
Copy link
Contributor

This commit addresses a bug in the SurfacePressureConstraintResolution where the force was incorrectly calculated due to a flawed if/if/else control flow. The logic has been corrected to a proper if/else if/else structure.

Additionally, the clamping logic in VolumeGrowthConstraintResolution has been refactored to use std::min and std::max, improving code clarity and conciseness.

This commit addresses a bug in the `SurfacePressureConstraintResolution` where the force was incorrectly calculated due to a flawed `if/if/else` control flow. The logic has been corrected to a proper `if/else if/else` structure.

Additionally, the clamping logic in `VolumeGrowthConstraintResolution` has been refactored to use `std::min` and `std::max`, improving code clarity and conciseness.
Refactors the `getCavityVolume` function in `SurfacePressureModel` to improve readability and reduce code duplication.

The volume calculation for a single tetrahedron has been extracted into a new private helper function, `getTetrahedronVolume`. This change makes the overall volume computation logic easier to follow without altering the underlying algorithm.

void drawValue(const sofa::core::visual::VisualParams* vparams);
void computeEdges();
Real getTetrahedronVolume(const Coord& p0, const Coord& p1, const Coord& p2);
Copy link
Member

Choose a reason for hiding this comment

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

I would rename this function. Here are some propositions:

  • getTriangleContributionToVolume
  • getSurfaceIntegralOfTriangle
  • anything expressing that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. I will do that.

@EulalieCoevoet
Copy link
Member

I've merged with master to fix the failing tests on the CI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants