Skip to content
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

Inconsistencies between Contains/Covers functions #1048

Open
DmytroMuravskyi opened this issue Oct 30, 2023 · 0 comments
Open

Inconsistencies between Contains/Covers functions #1048

DmytroMuravskyi opened this issue Oct 30, 2023 · 0 comments

Comments

@DmytroMuravskyi
Copy link
Contributor

Describe the bug
When working with issue #767 I found that it's hard categorize Contains/Covers functions or understand what it does from signature.
All Covers function say that they return true when "is within this Polygon or coincident with an edge" while Contains - "without coincidence with an edge or vertex". But Contains3D (and a function calling it) says "is within this Polygon or coincident with an edge." and returns accordingly breaking understanding of difference between the two.

It's hard to understand if function expects 3d or 2d geometry. Many functions say "when compared on a shared plane." but some of them, work on XY plane, like bool Contains(Polygon polygon) where Clipper with 2d IntPoint is used or bool Covers(Polygon polygon) where Intersects2d is called. This fails if Polygon is on YZ plane, for example.

Also some functions use Clipper lib: bool Contains(Polygon polygon) and bool Covers(Vector3 vector) while other use custom implementation. Ideally all functions should rely on same implementation since they are similar.

Expected behavior

  • Clear difference between Covers (inside or on boundary) and Contains (only inside).
  • One internal implementation for Point-Polygon and Polygon-Polygon overload of Covers and Contains.
  • Clear separation between 3d and 2d only functions. If only Contains3d is the function that works with 3d objects this should be correctly reflected in function documentation.
    In my opinion, there should be one private implementation (based on Clipper or custom implementation) for Point-Polygon containment and one for Polygon-Polygon containment that works efficiently on XY plane and different public function can be built on top of it depending on how they treat CoincidesAtEdge/Vertex case, do they expect 2d or 3d input. And if 3d input is expected then shared plane is first converted to XY plane.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant