search

danfis / libccd

Library for collision detection between two convex shapes
Other
505 stars 111 forks source link

ccd_pt_vertex_t.edges is incorrectly assigned #46

Closed hongkai-dai closed 5 years ago

hongkai-dai commented 6 years ago

In the struct ccd_pt_vertex_t, it has a field edges. I assume this field means all the edges connecting to this vertex.

The edges of a vertex is changed in function ccdPtAddEdge(ccd_pt_t* p, ccd_pt_vertex_t* v1, ccd_pt_vertex* v2). Here edge->vertex_list[0] is appended to v1->edges, and edge->vertex_list[1] is appended to v2->edges.

I am confused by these two lines, for two reasons

  1. I think we should append the newly created edge to v1->edges and v2->edges.
  2. Neither edge->vertex_list[0] nor edge->vertex_list[1] were properly initialized, since the creation of edge , so edge->vertex_list[0] and edge->vertex_list[1] just contain garbage values. Why we want to append garbage values to v1->edges and v2->edges?

It is worth noting that ccd_pt_vertex_t.edges is not really used in the code, so it doesn't cause any damage, but still I think it is better to assign meaningful values to these variables.

I can submit a PR for this issue.

danfis commented 5 years ago

This is not how ccd_list works and if you have tried to run tests that are there, you would find that your changes would cause segfaults.

But you are maybe right that ccd_pt_vertex_t.edges is not used for anything meaningful. It is used just for checking whether we can delete a vertex and, I think, we could do this more efficiently...