-
Notifications
You must be signed in to change notification settings - Fork 39
fix vertex id assignment #204
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
Conversation
| ++unused_vert_id; | ||
| } else { | ||
| boxes->vertices[i].vertex_ids.z = vertex_boxes[i].vertex_ids[2]; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want you can use the same logic as Scalable CCD:
Vertex Box: (i, -1, -1) -> (i, -i-1, -i-1)
Edge Box: (i, j, -1) -> (i, j, -i - 1)
Triangle Box: (i, j, k) -> (i, j, k)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const auto [vi, vj, vk] = vertex_boxes[i].vertex_ids;
assert(vi >= 0);
boxes->vertices[i].vertex_ids.x = vi;
boxes->vertices[i].vertex_ids.y = vj >= 0 ? vj : (-vi - 1);
boxes->vertices[i].vertex_ids.z = vk >= 0 ? vk : (-vi - 1);There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can vi be 0? Maybe the assert should be assert(vi >= 0);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes you are right
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #204 +/- ##
=======================================
Coverage 97.40% 97.41%
=======================================
Files 149 149
Lines 23748 23748
Branches 797 798 +1
=======================================
+ Hits 23132 23133 +1
+ Misses 616 615 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
Fix vertex id assignment logic in
SweepAndTiniestQueue.Type of change
How Has This Been Tested?
The broadphase unit test now pass.
Test Configuration:
Checklist