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

Improve gentrap documentation and fix typos #1205

Merged
merged 6 commits into from
Apr 26, 2024

Conversation

mrguilima
Copy link
Contributor

Since the input lower points can be ordered in both CW or CCW orderings, the sense needs to take into account the sign of the plane offset with respect to the plane normal.

An extra test was added for opposite ordering of points (CCW ordering), and the documentation has been adjusted.

See review process for descriptions of the labels and requirements.

The points in upper face should still be provided in the same
corresponding order as in the lower face. Otherwise a standard
trapezoid will have twisted faces, which is still not supported for now.
@mrguilima mrguilima added bug Something isn't working documentation Improvements or additions to documentation, examples, and tests orange Work on ORANGE geometry engine labels Apr 25, 2024
@mrguilima mrguilima self-assigned this Apr 25, 2024
@sethrj sethrj self-requested a review April 25, 2024 19:26
@sethrj sethrj removed the documentation Improvements or additions to documentation, examples, and tests label Apr 25, 2024
@sethrj sethrj changed the title Fix the senses of gentrap side surfaces *[orange, bug, documentation]* Fix the senses of gentrap side surfaces Apr 25, 2024
src/orange/orangeinp/ConvexRegion.cc Outdated Show resolved Hide resolved
@sethrj
Copy link
Member

sethrj commented Apr 25, 2024

@mrguilima Maybe some additional restrictions are needed on the points input to the gentrap. For example, are there restrictions about the top and bottom points not being rotated 180 degrees?
Screenshot 2024-04-25 at 18 49 23

@sethrj
Copy link
Member

sethrj commented Apr 25, 2024

@mrguilima Actually it looks like the problem is that the normal calculation assumes the points are CCW ... which is not the case.

@sethrj sethrj changed the title Fix the senses of gentrap side surfaces Improve gentrap documentation and fix typos Apr 26, 2024
@sethrj sethrj added documentation Improvements or additions to documentation, examples, and tests and removed bug Something isn't working labels Apr 26, 2024
@sethrj sethrj enabled auto-merge (squash) April 26, 2024 01:15
@sethrj sethrj mentioned this pull request Apr 26, 2024
@sethrj sethrj merged commit a016078 into celeritas-project:develop Apr 26, 2024
27 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation, examples, and tests orange Work on ORANGE geometry engine
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants