Cylinder-Wall Spheropolyhedron Support #1984
Conversation
Added cylinder-spheropolyhedra support
Added cylinder-spheropolyhedra overlap method
| = cross(shifted_pos, | ||
| wall.orientation); // find the component of the shifted position that is | ||
| // perpendicular to the normalized orientation vector | ||
| Scalar max_dist = sqrt(dot(dist_vec, dist_vec)); |
There was a problem hiding this comment.
This value is only ever used squared.
Remove the sqrt and rename to max_dist_squared. Remove the multiplications that square it.
| vec3<Scalar> dist_vec | ||
| = cross(shifted_pos, | ||
| wall.orientation); // find the component of the shifted position that is | ||
| // perpendicular to the normalized orientation vector |
There was a problem hiding this comment.
Comments are easier to read at the same level as the code. You can also use English language rules when writing comments.
| vec3<Scalar> dist_vec | |
| = cross(shifted_pos, | |
| wall.orientation); // find the component of the shifted position that is | |
| // perpendicular to the normalized orientation vector | |
| // Find the component of the shifted position that is | |
| // perpendicular to the normalized orientation vector. | |
| vec3<Scalar> dist_vec | |
| = cross(shifted_pos, | |
| wall.orientation); | |
| wall.orientation); // find the component of the shifted position that is | ||
| // perpendicular to the normalized orientation vector | ||
| Scalar max_dist = sqrt(dot(dist_vec, dist_vec)); | ||
| if (wall.inside) |
There was a problem hiding this comment.
A comment would be helpful to understand what the code is doing here.
| if (wall.inside) | |
| // Check if the shape's bounding sphere intersects the wall. | |
| if (wall.inside) |
Also, consider rewriting this so that it evaluates the <= or > inside the if. The branch is repeated again with a ternary operator on the next line which is very confusing. Better yet, combine the content of the if check_verts -> if shape.verts.N -> branches here as well. Bringing all the "inside" and "outside" code together will make it much easier to understand.
| } | ||
| // Edge case; pure sphere. In this case, check_verts implies that the | ||
| // sphere will be outside. | ||
| // Note from gabs: I don't fully understand this edge case |
There was a problem hiding this comment.
When shape.verts.N is zero, it is assume to have a single vertex at 0,0,0 - making it a sphere. In that case the bounding sphere check above is the only check that is necessary. I agree that this is really confusing. This case should be handled FIRST, not last. It would be more readable moved up above and handled with its own return false branch.
Description
I added in spheropolyhedron support for cylindrical walls. I changed one user seen python file and the ExternalWall.h file.
Motivation and context
It's useful to be able to scan through convex polyhedra, convex spheropolyhedra, and spheres within cylindrical walls.
How has this been tested?
NEED TO ADD
Documentation builds and displays correctly.
Checklist:
sphinx-doc/credits.rst) in the pull request source branch.CHANGELOG.rstfollowing the established format.