avaxman / Directional

A library for Directional Field Synthesis, Design, and Processing.
179 stars 30 forks source link

Bug: Inconsistency of innerEdges(init in dual_cycles.h) and isBoundaryEdge(init in trimesh.h) #78

Open zxhcho opened 3 days ago

zxhcho commented 3 days ago

For mesh with boundary, we only define angles on inner/non-boundary edges when calling functions in index_prescription.h , so we need to map all inner edges into a new array.

in dual_cycles.h, line 187, you write:

    for (int i=0;i<EV.rows();i++)
      if (!((isBoundary(EV(i,0)))&&(isBoundary(EV(i,1)))))
        innerEdgesList.push_back(i);

in TriMesh.h, line 81, you write:

            std::vector<int> innerEdgesList, boundEdgesList;
            isBoundaryVertex=Eigen::VectorXi::Zero(V.size());
            isBoundaryEdge=Eigen::VectorXi::Zero(EV.size());
            for (int i = 0; i < EF.rows(); i++) {
                if ((EF(i, 1) == -1) || (EF(i, 0) == -1)) {
                    boundEdgesList.push_back(i);
                    isBoundaryEdge(i)=1;
                    isBoundaryVertex(EV(i,0))=1;
                    isBoundaryVertex(EV(i,1))=1;
                }else
                    innerEdgesList.push_back(i);

            }

But in such mesh, these two definitions are inconsistent, an edge connecting 2 boundary vertices is not a boundary edge, more reasonable one is in TriMesh.h image

avaxman commented 3 days ago

Yes it's right, care to publish a pull request?

On Wed, Oct 9, 2024, 08:03 Shelf_Zhang @.***> wrote:

For mesh with boundary, we only define angles on inner/non-boundary edges when calling functions in index_prescription.h , so we need to map all inner edges into a new array.

in dual_cycles.h, line 187, you write:

for (int i=0;i<EV.rows();i++)
  if (!((isBoundary(EV(i,0)))&&(isBoundary(EV(i,1)))))
    innerEdgesList.push_back(i);

in TriMesh.h, line 81, you write:

        std::vector<int> innerEdgesList, boundEdgesList;
        isBoundaryVertex=Eigen::VectorXi::Zero(V.size());
        isBoundaryEdge=Eigen::VectorXi::Zero(EV.size());
        for (int i = 0; i < EF.rows(); i++) {
            if ((EF(i, 1) == -1) || (EF(i, 0) == -1)) {
                boundEdgesList.push_back(i);
                isBoundaryEdge(i)=1;
                isBoundaryVertex(EV(i,0))=1;
                isBoundaryVertex(EV(i,1))=1;
            }else
                innerEdgesList.push_back(i);

        }

But in such mesh, these two definitions are inconsistent, an edge connecting 2 boundary vertices is not a boundary edge, more reasonable one is in TriMesh.h image.png (view on web) https://github.com/user-attachments/assets/b064293d-b9ef-4d66-8937-78f18b0210b0

— Reply to this email directly, view it on GitHub https://github.com/avaxman/Directional/issues/78, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACERY6GM3XXDZYQSROOJBETZ2TIKZAVCNFSM6AAAAABPT2DPZCVHI2DSMVQWIX3LMV43ASLTON2WKOZSGU3TIOJYG4ZDOMI . You are receiving this because you are subscribed to this thread.Message ID: @.***>

zxhcho commented 3 days ago

It's OK, I'll raise a PR soon.