Skip to content
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

Adding 3D barycentric coordinates for convex polyhedra #8267

Open
wants to merge 65 commits into
base: master
Choose a base branch
from

Conversation

sloriot
Copy link
Member

@sloriot sloriot commented Jun 7, 2024

Summary of Changes

New package for 3D barycentric coordinates in a convex polyhedron

Release Management

  • Affected package(s):
  • Feature/Small Feature (if any): here
  • Link to compiled documentation (obligatory for small feature) wrong link name to be changed
  • License and copyright ownership:

antoniospg and others added 30 commits June 7, 2024 14:16
…ulate tetrahedra coordinates, as well as utils_3 that do the actual calculation, and one example to test the possible 3 cases of points
…the equations, now I will make my code more organized
…ween 0 and 1. Added utils.h to generate polyhedrons
@sloriot sloriot added Not yet approved The feature or pull-request has not yet been approved. Feature Stalled gsoc gsoc-2021 labels Jun 7, 2024
@sloriot

This comment was marked as outdated.

This comment was marked as outdated.

@sloriot
Copy link
Member Author

sloriot commented Jun 7, 2024

/build:v1

Copy link

github-actions bot commented Jun 7, 2024

The documentation is built. It will be available, after a few minutes, here: https://cgal.github.io/8267/v1/Manual/index.html

@sloriot
Copy link
Member Author

sloriot commented Jun 7, 2024

test/Barycentric_coordinates_3/test_containers fails with

terminate called after throwing an instance of 'CGAL::Assertion_exception'
  what():  CGAL ERROR: assertion violation!
Expr: sum != FT(0)
File: Barycentric_coordinates_3/include/CGAL/Barycentric_coordinates_3/Wachspress_coordinates_3.h
Line: 269
Aborted

I suspect there is a degenerate case not handled as the mesh is cube and the query the centroid of the cube

@sloriot
Copy link
Member Author

sloriot commented Jun 7, 2024

cc @antoniospg :)

@MaelRL MaelRL added this to the 6.1-beta milestone Jun 7, 2024
@danston
Copy link
Contributor

danston commented Jun 15, 2024

@sloriot No, for center query in a cube there are no edge cases. The sum should be strictly positive because all weights are positive. My guess would be that certain weights are positive and certain are negative so they sum up to zero. If this is a case, most likely the orientation of faces is different from what it has been when the test was developed because it did pass when Antonio worked on it. Could that be that this line

CGAL::make_hexahedron(p0, p1, p2, p3, p4, p5, p6, p7, polyhedron, CGAL::parameters::do_not_triangulate_faces(false));

produces different results from this one:

PMP::triangulate_faces(faces(polyhedron), polyhedron);

In any case for such a simple test, I would first check how the cube looks like, orientation of its faces, and values of the weights.

@sloriot
Copy link
Member Author

sloriot commented Jun 18, 2024

What is very strange. If I write the generated mesh into a file, and read it back the test works. address sanitizer does not report anything and CGAL::is_valid_polygon_mesh() does not raise any assertion. It seems that the issue depends on the way elements are stored in the mesh (that is what is the first halfedge visited around a vertex, ...).


// Compute wp coordinates for a given vertex v and a query q
template<typename Vertex>
FT compute_wp_vertex_query(const Vertex& vertex, const Point_3& query){
Copy link
Contributor

@danston danston Jun 19, 2024

Choose a reason for hiding this comment

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

@sloriot The issue is probably coming from this method. There is suspicious negation of the first face normal, which is not coherent with other normals used here. Implementations of other MV and DH coordinates do not have this. My guess would be that this method does not return the same weights for the same mesh but differently oriented faces, which should not be the case. What is important here is to have consistent normals as indicated in the original paper. Maybe just try to remove the negation in line 296 and see what happens?

There are multiple formulas to compute WP coordinates. I do not remember which one Antonio used here. Maybe it is worth using all formulas from the same paper to be consistent. Finally, another test that is easy to do is to change WP to DH coordinates in this test and see if it works. If I remember correctly for such a simple case (center point in a cube) WP and DH should produce exactly the same result.

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.

4 participants