Currently, when creating a polygon shape via b2PolygonShape::Set, box2d will perform vertex welding for vertices that are closer than 0.5 * b2_linearSlop, and assert that the number of vertices is >= 3, and that edge lengths are > b2_epsilon. There is currently no way for the user code to determine if a set of vertices will be changed and/or asserted on by box2d without duplicating the entire logic in b2PolygonShape::Set. Note that vertex welding might make a polygon invalid that wasn't before.
This is problematic in my case, because my game generates polygons dynamically, so degenerate polygons might occur during normal execution. If they occur, I want my game to know about them and handle the error. I do not want box2d to assert or silently fail.
Silently modifying the polygon might also lead to other issues, such as an independent rendering representation not matching the actual polygon in box2d, because box2d performed vertex welding. To prevent this, one would need to read back the vertices after calling b2PolygonShape::Set from the b2PoygonShape, which seems strange.
In general, I think it would be helpful to make changes and errors of polygon creation visible and obvious to the caller. To avoid having to replicate (and track changes to) the logic inside b2PolygonShape::Set, I propose extracting vertex welding and all checks from b2PolygonShape::Set into their own function, which takes vertices as input and returns the vertices after welding, or an error, if the polygon was degenerate. The caller would then be able to take the modified vertices and know that box2d will not alter them or fail when calling b2PolygonShape::Set, or handle the error on their side, with more context.
For backwards compatibility and to prevent unnecessary work, one could add a parameter like "bool verticesAreGuaranteedToBeValid" to b2PolygonShape::Set, which, if true, would skip the check inside b2PolygonShape::Set. That way, people who are interested in handling the transformations/errors on their side can do so and set this bool to true, and all others set it to false and still just assert, when the check fails in b2PolygonShape::Set.
erincatto
commented
1 year ago
Currently, when creating a polygon shape via b2PolygonShape::Set, box2d will perform vertex welding for vertices that are closer than 0.5 * b2_linearSlop, and assert that the number of vertices is >= 3, and that edge lengths are > b2_epsilon. There is currently no way for the user code to determine if a set of vertices will be changed and/or asserted on by box2d without duplicating the entire logic in b2PolygonShape::Set. Note that vertex welding might make a polygon invalid that wasn't before.
This is problematic in my case, because my game generates polygons dynamically, so degenerate polygons might occur during normal execution. If they occur, I want my game to know about them and handle the error. I do not want box2d to assert or silently fail.
Silently modifying the polygon might also lead to other issues, such as an independent rendering representation not matching the actual polygon in box2d, because box2d performed vertex welding. To prevent this, one would need to read back the vertices after calling b2PolygonShape::Set from the b2PoygonShape, which seems strange.
In general, I think it would be helpful to make changes and errors of polygon creation visible and obvious to the caller. To avoid having to replicate (and track changes to) the logic inside b2PolygonShape::Set, I propose extracting vertex welding and all checks from b2PolygonShape::Set into their own function, which takes vertices as input and returns the vertices after welding, or an error, if the polygon was degenerate. The caller would then be able to take the modified vertices and know that box2d will not alter them or fail when calling b2PolygonShape::Set, or handle the error on their side, with more context.
For backwards compatibility and to prevent unnecessary work, one could add a parameter like "bool verticesAreGuaranteedToBeValid" to b2PolygonShape::Set, which, if true, would skip the check inside b2PolygonShape::Set. That way, people who are interested in handling the transformations/errors on their side can do so and set this bool to true, and all others set it to false and still just assert, when the check fails in b2PolygonShape::Set.