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

Trying to use visitor in split() method #8275

Closed
citystrawman opened this issue Jun 11, 2024 · 14 comments · Fixed by #8276
Closed

Trying to use visitor in split() method #8275

citystrawman opened this issue Jun 11, 2024 · 14 comments · Fixed by #8276
Assignees
Milestone

Comments

@citystrawman
Copy link

citystrawman commented Jun 11, 2024

Please use the following template to help us solving your issue.

Issue Details

I am using split function to intersect two surface meshes and I want to get the newly created points/edges during split so that I can remesh these points. From this link I think I could use visitor to get what I want. the surface meshes, named slope and slide respectively, are shown as follows:

image
企业微信截图_17180771721603

However, I could not get any infomation, before_subface_creations, after_subface_created after_subface_creations all the 3 call back functions do not get called.

/

I hope if anyone could help me solve this. Thank you!

Source Code

#include <CGAL/Exact_predicates_inexact_constructions_kernel.h>
#include <CGAL/Exact_predicates_exact_constructions_kernel.h>
#include <CGAL/Surface_mesh.h>
#include <CGAL/boost/graph/selection.h>
#include <CGAL/Polygon_mesh_processing/remesh.h>

#include <CGAL/Polygon_mesh_processing/corefinement.h>
#include <CGAL/Polygon_mesh_processing/clip.h>
#include <CGAL/Polygon_mesh_processing/IO/polygon_mesh_io.h>

#include <iostream>
#include <string>

typedef CGAL::Exact_predicates_inexact_constructions_kernel K;
typedef CGAL::Exact_predicates_exact_constructions_kernel EK;
typedef CGAL::Surface_mesh<K::Point_3> Mesh;
typedef boost::graph_traits<Mesh>::vertex_descriptor vertex_descriptor;
typedef Mesh::Property_map<vertex_descriptor, EK::Point_3> Exact_point_map;
typedef boost::graph_traits<Mesh>::face_descriptor            face_descriptor;
typedef boost::graph_traits<Mesh>::edge_descriptor            edge_descriptor;
typedef boost::graph_traits<Mesh>::halfedge_descriptor        halfedge_descriptor;


namespace PMP = CGAL::Polygon_mesh_processing;
namespace params = CGAL::parameters;

class Insert_iterator
{
    typedef std::unordered_map<face_descriptor, face_descriptor> Container;
    Container& container;
public:

    Insert_iterator(Container& c)
        : container(c) {}

    Insert_iterator&
        operator=(const std::pair<face_descriptor, face_descriptor>& p)
    {
        container[p.second] = p.first;
        return *this;
    }

    Insert_iterator&
        operator*() { return *this; }

    Insert_iterator
        operator++(int) { return *this; }

};
struct Visitor : public CGAL::Polygon_mesh_processing::Corefinement::Default_visitor<Mesh>
{
    typedef std::unordered_map<face_descriptor, face_descriptor> Container;

    Container& container;
    face_descriptor qfd;

    Visitor(Container& container)
        : container(container)
    {}

    void before_subface_creations(face_descriptor fd)
    {
        std::cout << "split : " << fd << " into:" << std::endl;
        Container::iterator it = container.find(fd);
        qfd = it->second;
        container.erase(it);
    }

    void after_subface_created(face_descriptor fd)
    {
        std::cout << "  " << fd;
        container[fd] = qfd;
    }

    void after_subface_creations()
    {
        std::cout << std::endl;
    }
};

int main(int argc, char* argv[])
{
    const std::string filename1 = (argc > 1) ? argv[1] : CGAL::data_file_path("meshes/slope.off");
    const std::string filename2 = (argc > 2) ? argv[2] : CGAL::data_file_path("meshes/slide.off");

    Mesh mesh1, mesh2;
    if (!PMP::IO::read_polygon_mesh(filename1, mesh1) || !PMP::IO::read_polygon_mesh(filename2, mesh2))
    {
        std::cerr << "Invalid input." << std::endl;
        return 1;
    }

    if (PMP::does_self_intersect(mesh1) || PMP::does_self_intersect(mesh2))
    {
        std::cerr << "self intersect input." << std::endl;
        return 1;
    }

    std::unordered_map<face_descriptor, face_descriptor> t2q;
    Visitor v(t2q);

    PMP::split(mesh1, mesh2, params::do_not_modify(true), params::visitor(v).do_not_modify(true));

    return 0;
}

Environment

  • Operating system (Windows/Mac/Linux, 32/64 bits): Windows 11
  • Compiler: MSVC
  • Release or debug mode:
  • Specific flags used (if any):
  • CGAL version: 5.6.1
  • Boost version:
  • Other libraries versions if used (Eigen, TBB, etc.):
@sloriot
Copy link
Member

sloriot commented Jun 11, 2024

The visitor should be in the first named parameter, not the second one (you have to expand the doc to see the np_tm only). This kind of problem should be fixed when #7966 is merged and used for all functions.

@citystrawman
Copy link
Author

The visitor should be in the first named parameter, not the second one (you have to expand the doc to see the np_tm only). This kind of problem should be fixed when #7966 is merged and used for all functions.

Thank you for your kind remind, but after changing the visitor to the first name parameter, the call backs are still not called, and the visitor v is still empty after running split method.

@sloriot
Copy link
Member

sloriot commented Jun 11, 2024

You cannot put do_not_modify(true) for the first mesh. Then the signature of the visitor functions are incorrect, the last argument is a mesh. container.find(fd); might return container.end() which will lead to segfault if you access the iterator. On CGAL side, you'll need #8276 to get it working after fixing the other issues.

@citystrawman
Copy link
Author

You cannot put do_not_modify(true) for the first mesh. Then the signature of the visitor functions are incorrect, the last argument is a mesh. container.find(fd); might return container.end() which will lead to segfault if you access the iterator. On CGAL side, you'll need #8276 to get it working after fixing the other issues.

Sorry I may not catch your words.
You mean Split() method should be written like this: PMP::split(mesh1, mesh2, params::visitor(v))? (I tried this, and it still does not work.)
Or do you mean that, the visitor is not working correctly until the issue is fixed?

@lrineau lrineau linked a pull request Jun 11, 2024 that will close this issue
@lrineau lrineau added this to the 5.5.5 milestone Jun 11, 2024
@citystrawman
Copy link
Author

citystrawman commented Jun 12, 2024

You cannot put do_not_modify(true) for the first mesh. Then the signature of the visitor functions are incorrect, the last argument is a mesh. container.find(fd); might return container.end() which will lead to segfault if you access the iterator. On CGAL side, you'll need #8276 to get it working after fixing the other issues.

Hi, after reading this post, I changed split to clip method, and now before_subface_creations get called. At first before_subface_creations has code as follows:

void before_subface_creations(face_descriptor fd)
    {
        std::cout << "split : " << fd << " into:" << std::endl;
        Container::iterator it = container.find(fd);
        qfd = it->second;
        container.erase(it);
    }

which is copied from official code example.
However when I run the code, a face_descriptor which has the same idx_(in my example, f1134) run into this callback twice.
I am confused about it because I think face_descriptor should be unique and if a face_descriptor get called, the container will erase it, then why a face_descriptor that has the same idx_ run into the callback twice?

@sloriot
Copy link
Member

sloriot commented Jun 12, 2024

Copy/paste of my answer here: You are mixing things up. The example you are pointing out is for triangulating a mesh, and the visitor must model the concept PMPTriangulateFaceVisitor. If you are using corefinement, the visitor must model the concept PMPCorefinementVisitor. In clip (and corefinement) you have 2 meshes so the same id might appear twice but with different meshes.

@sloriot
Copy link
Member

sloriot commented Jun 12, 2024

Or do you mean that, the visitor is not working correctly until the issue is fixed?

Yes you need the aforementioned patch so that the visitor is correctly forwarded

@citystrawman
Copy link
Author

citystrawman commented Jun 12, 2024

Or do you mean that, the visitor is not working correctly until the issue is fixed?

Yes you need the aforementioned patch so that the visitor is correctly forwarded

Thank you, BTW I see that you just changed few code in #8276 as well as in #7966 , If I want to use this new feature now, can I just use the 5.5.x branch and just add the same code ?

@citystrawman
Copy link
Author

Copy/paste of my answer here: You are mixing things up. The example you are pointing out is for triangulating a mesh, and the visitor must model the concept PMPTriangulateFaceVisitor. If you are using corefinement, the visitor must model the concept PMPCorefinementVisitor. In clip (and corefinement) you have 2 meshes so the same id might appear twice but with different meshes.

OK. So params::visitor(v) actually deals with both the tm as well as clipper/splitter even though it is associated as np_tm?

@lrineau
Copy link
Member

lrineau commented Jun 13, 2024

The PR #8275 is now merged.

@lrineau lrineau closed this as completed Jun 13, 2024
@sloriot
Copy link
Member

sloriot commented Jun 14, 2024

If you are using cmake, it is just a matter of modifying your CGAL_DIR to point to the git checkout.

@citystrawman
Copy link
Author

If you are using cmake, it is just a matter of modifying your CGAL_DIR to point to the git checkout.

OK. Thank you!

@citystrawman
Copy link
Author

citystrawman commented Jun 28, 2024

If you are using cmake, it is just a matter of modifying your CGAL_DIR to point to the git checkout.

Hi, just have another question: I am trying to implement some halfedge callbacks such as after_edge_copy after_edge_duplicated intersection_edge_copy , etc. but all these callbacks are not get called when I use clip funtion. Is that correct ?

update: I added add_retriangulation_edge and edge_split, and these two callbacks are called

@sloriot
Copy link
Member

sloriot commented Jun 28, 2024

if the clip is done with clip_volume=false there is no edge copy/duplication

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

Successfully merging a pull request may close this issue.

4 participants