feat: provide a shapeDistance function
#1328
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Deploying with
|
| Latest commit: |
68d086a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0670bc37.penrose-72l.pages.dev |
| Branch Preview URL: | https://shape-distance.penrose-72l.pages.dev |
± Registry diff📊 PerformanceKeyNote that each bar component rounds up to the nearest 100ms, so each full bar is an overestimate by up to 400ms. Data |
Codecov Report
@@ Coverage Diff @@
## main #1328 +/- ##
==========================================
+ Coverage 61.40% 61.55% +0.14%
==========================================
Files 65 65
Lines 8056 8055 -1
Branches 1888 1884 -4
==========================================
+ Hits 4947 4958 +11
+ Misses 3001 2989 -12
Partials 108 108
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
samestep
requested review from
wodeni,
jiriminarcik,
keenancrane and
joshsunshine
|
@samestep, please describe what shapes this works on, what approximations (if any) are used, etc. in the PR description. |
|
@joshsunshine done! |
liangyiliang
approved these changes
Mar 2, 2023
joshsunshine
approved these changes
Mar 2, 2023
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR adds a new function called
shapeDistancewhich computes the signed distance from the origin on the Minkowski sum of the first shape and the negative of the second shape.This works on all shapes. It uses the following approximations:
Circle, we are exactRectlike, we are exactTextorEquationand the other isLine, we use the bounding circle approximation added by @wodeni in feat: improveeuclidean.sty#1117Polygonlike, we use the intersection-of-half-planes approximation added by @jiriminarcik in feat: Line polygon constraints #810Rectlikeand the other isCircle, we are exactPolygonlikeand the other isEllipse, I'm not exactly sure what we do; @jiriminarcik do you happen to know if this is an approximation?Circleand the other isLine, I believe we are exactImplementation strategy and design decisions
I factored out most of the existing
overlappingconstraint. Also, for better typechecking, I added aShapeTupletype representing the pair format expected by objectives and constraints, and ashapeTupleToShapefunction that converts this to just aShape. In the same file, I also addedRectlikeandPolygonliketypes along withisRectlikeandisPolygonliketype predicates to go along with those.This patch made the
overlappingconstraint more correct for the case where one shape is a circle or an ellipse and the other is a rectangle, so I corrected the tests for that case.Checklist
Open questions
I didn't factor out the cases where one shape is an ellipse and the other is either a circle or an ellipse, because I don't know how to use our existing code to actually calculate the distance there.