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.

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

Jump to bottom

PMP::detect_vertex_incident_patches #8730

Open
yDF8EPJN8 opened this issue Feb 9, 2025 · 1 comment · May be fixed by #8765
Open

PMP::detect_vertex_incident_patches #8730

yDF8EPJN8 opened this issue Feb 9, 2025 · 1 comment · May be fixed by #8765
Assignees
@MaelRL
Labels
Bug Pkg::PMP
Milestone
5.6.3

Comments

@yDF8EPJN8
Copy link

Issue Details

In the PMP::detect_vertex_incident_patches function, the property map is calculated only for vertices incident to feature edges.
But the documentation says: "a property map that will contain the patch ids of all the faces incident to each vertex of pmesh".

I think there's a bug in the condition

    // Look only at feature vertices
    if(!get(edge_is_feature_map, edge(halfedge(vit, pmesh), pmesh)))
      continue;

at line 337, in the file "CGAL/Polygon_mesh_processing/detect_features.h"

Source Code

I have generated a cube in which each face has a point on each face that is not incident to feature edges.
The number of incident patches for these points is zero, which does not correspond to the documentation.

#include <CGAL/Exact_predicates_inexact_constructions_kernel.h>
#include <CGAL/Surface_mesh.h>
#include <CGAL/draw_surface_mesh.h>
#include <CGAL/Polygon_mesh_processing/detect_features.h>

using Kernel = CGAL::Exact_predicates_inexact_constructions_kernel;
using Point_3 = Kernel::Point_3;

using Surface_mesh = CGAL::Surface_mesh<Point_3>;

using vertex_index = Surface_mesh::vertex_index;
using halfedge_index = Surface_mesh::halfedge_index;
using face_index = Surface_mesh::face_index;
using edge_index = Surface_mesh::edge_index;


std::vector<Point_3> points = {
    {0,0,0},
    {0,0.5,0},
    {0,1,0},
    {0.5,0,0},
    {0.5,0.5,0},
    {0.5,1,0},
    {1,0,0},
    {1,0.5,0},
    {1,1,0},
    {0,0,0.5},
    {0,0.5,0.5},
    {0,1,0.5},
    {0.5,0,0.5},
    {0.5,1,0.5},
    {1,0,0.5},
    {1,0.5,0.5},
    {1,1,0.5},
    {0,0,1},
    {0,0.5,1},
    {0,1,1},
    {0.5,0,1},
    {0.5,0.5,1},
    {0.5,1,1},
    {1,0,1},
    {1,0.5,1},
    {1,1,1}
};

std::vector<std::array<int, 3>> faces = {
    {7,4,5},
    {7,6,4},
    {7,5,8},
    {4,2,5},
    {2,4,1},
    {11,5,2},
    {11,13,5},
    {11,2,10},
    {2,1,10},
    {11,10,18},
    {11,18,19},
    {10,17,18},
    {10,9,17},
    {20,18,17},
    {20,21,18},
    {12,20,17},
    {12,14,20},
    {12,17,9},
    {3,12,9},
    {3,6,12},
    {3,9,0},
    {3,0,1},
    {9,1,0},
    {1,9,10},
    {3,1,4},
    {6,3,4},
    {12,6,14},
    {6,7,14},
    {15,14,7},
    {14,15,23},
    {15,7,8},
    {15,8,16},
    {24,15,16},
    {16,8,13},
    {22,16,13},
    {13,8,5},
    {22,25,16},
    {22,13,19},
    {13,11,19},
    {22,19,21},
    {21,24,22},
    {19,18,21},
    {21,23,24},
    {22,24,25},
    {24,16,25},
    {23,21,20},
    {24,23,15},
    {14,23,20}
};

int main()
{
  Surface_mesh mesh;
  std::vector<vertex_index> vertices(points.size());
  for (size_t i = 0; i < points.size(); ++i)
  {
    vertices[i] = mesh.add_vertex(points[i]);
  }

  for (size_t i = 0; i < faces.size(); ++i)
  {
    vertex_index v0 = vertices[faces[i][0]];
    vertex_index v1 = vertices[faces[i][1]];
    vertex_index v2 = vertices[faces[i][2]];
    mesh.add_face(v0, v1, v2);
  }

  CGAL::draw(mesh);

  auto vertexToPatches = mesh.add_property_map<vertex_index, std::set<size_t>>("v:patches").first;
  auto faceToPatch = mesh.add_property_map<face_index, size_t>("f:patch", std::numeric_limits<size_t>::max()).first;
  auto edgeToIsFeature = mesh.add_property_map<edge_index, bool>("e:is_feature", false).first;

  double angle_in_deg = 90;

  CGAL::Polygon_mesh_processing::detect_sharp_edges(mesh, angle_in_deg, edgeToIsFeature);

  const size_t number_of_patches = CGAL::Polygon_mesh_processing::
    connected_components(mesh, faceToPatch,
      CGAL::parameters::edge_is_constrained_map(edgeToIsFeature));

  CGAL::Polygon_mesh_processing::detect_vertex_incident_patches(mesh,
    faceToPatch, vertexToPatches, edgeToIsFeature);


  for (auto v : mesh.vertices())
  {
    std::cout << v << '\t';
    std::cout << vertexToPatches[v].size() << '\n';
  }

  return 0;
}

Environment

  • Operating system (Windows/Mac/Linux, 32/64 bits): Windows 64 bits
  • Compiler: visual c++
  • Release or debug mode:
  • Specific flags used (if any):
  • CGAL version: 6.0.1
  • Boost version: 1.86
  • Other libraries versions if used (Eigen, TBB, etc.):
    Qt 6.7.3
    Everything is installed via vcpkg (classic mode).
@MaelRL MaelRL self-assigned this Feb 13, 2025
Dhruv-Sharma01 added a commit to Dhruv-Sharma01/cgal that referenced this issue Mar 3, 2025
@Dhruv-Sharma01
Update detect_features.h
05bfaa2 
> This commit fixes an issue in the `detect_vertex_incident_patches` function where only vertices incident to feature edges were processed. According to the documentation, every vertex should have a property map entry containing the patch IDs of all incident faces. However, due to the condition:
> 
> ```cpp
> // Look only at feature vertices
> if(!get(edge_is_feature_map, edge(halfedge(vit, pmesh), pmesh)))
>   continue;
> ```
> 
> vertices that are not incident to any feature edges were being skipped, resulting in their patch IDs remaining empty. This patch removes (or comments out) that conditional check, ensuring that every vertex is examined and its corresponding patch IDs are collected. This change aligns the function’s behavior with its documentation.
> 
> **Files Affected:**  
> `Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/detect_features.h`  
> 
> **Impact:**  
> All vertices in the mesh now have the property map populated with the patch IDs of all incident faces, ensuring consistent and correct behavior as expected by users.
> 
> **Issue Reference:**  
> Fixes CGAL#8730
@Dhruv-Sharma01 Dhruv-Sharma01 linked a pull request Mar 3, 2025 that will close this issue
Open
@Dhruv-Sharma01
Copy link

@MaelRL I have opened a PR to address this issue, please review it.

@lrineau lrineau added this to the 5.6.3 milestone Mar 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@MaelRL MaelRL

Labels
Bug Pkg::PMP
Projects
None yet
Milestone
5.6.3
Development

Successfully merging a pull request may close this issue.

Update detect_features.h Dhruv-Sharma01/cgal
4 participants
@MaelRL @lrineau @Dhruv-Sharma01 @yDF8EPJN8