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

fix: halfPlaneSDF and padding #1360

Merged
merged 1 commit into from May 8, 2023
Merged

fix: halfPlaneSDF and padding #1360

merged 1 commit into from May 8, 2023

Conversation

liangyiliang
Copy link
Contributor

@liangyiliang liangyiliang commented May 5, 2023

Description

Resolves #1359.

This is an attempt to resolve the issue using the fix I talked about in the comment. It might not be correct.

Implementation strategy and design decisions

  • Now, halfPlaneSDF has the effect of adding padding instead of subtracting.
  • With this, a broken test case has been fixed. (Detailed below)
  • Also, containsPolygonCircle now calls containsPolygonPoints with add(padding, s2.r.contents). (Detailed below)

Broken Test Case

The bad test case involved is as follows:

  let point1 = [2, 3];
  let point2 = [1, 2];
  let point3 = [1, 4];
  let point4 = [2, 2];
  let point5 = [0, 0];
  // ...
  test("with padding", async () => {
    let result = halfPlaneSDF([point2, point3], [point2, point4], point5, 10);
    expect(numsOf([result])[0]).toBeCloseTo(-13, 4);
  });

Under the prescribed changes, the halfPlaneSDF function should output 7 instead of -13.

As far as I understand it, checking whether or not a point p is within the halfplane defined with lineSegment and interior vector center is essentially checking whether or not

halfPlaneSDF(lineSegment, -p, center, padding) // note the minus sign before `p`

So, calling

halfPlaneSDF([point2, point3], [point2, point4], point5, 10)

is essentially checking whether or not points [-point2, -point4] is within the half-plane defined using line segment [point2, point3] with interior point point5, under padding 10.

Graphically,
image

Adding instead of subtracting paddings

Originally, the function containsPolygonCircle is defined as follows:

export const containsPolygonCircle = (
  s1: Polygon<ad.Num>,
  s2: Circle<ad.Num>,
  padding: ad.Num = 0
): ad.Num => {
  return containsPolygonPoints(
    s1.points.contents,
    s2.center.contents,
    sub(padding, s2.r.contents) // padding
  );
};

I was confused as to why we subtract the radius from the padding.

Checking whether or not a Polygon contains a circle is essentially checking whether the Polygon contains the center of the circle, with padding r. So the new padding should be add(padding, s2.r.contents). But as I made that change, other parts of the Penrose stopped working. With the other parts fixed (as above), I changed sub to add.

Checklist

  • I have commented my code, particularly in hard-to-understand areas
  • I have reviewed any generated registry diagram changes

Open questions

Questions that require more discussion or to be addressed in future development:

@github-actions
Copy link

github-actions bot commented May 5, 2023

± Registry diff

📊 Performance

Key

Note that each bar component rounds up to the nearest 100ms, so each full bar is an overestimate by up to 400ms.

     0s   1s   2s   3s   4s   5s   6s   7s   8s   9s
     |    |    |    |    |    |    |    |    |    |
name ▝▀▀▀▀▀▀▀▀▀▀▀▚▄▄▄▄▄▄▄▄▄▞▀▀▀▀▀▀▀▀▀▀▀▀▚▄▄▄▄▄▄▄▄▄▖
      compilation labelling optimization rendering

Data

                                                                  0s   1s   2s   3s   4s   5s
                                                                  |    |    |    |    |    |
3d-projection-fake-3d-linear-algebra                              ▝▞▖
acquaintanceship-graph-simple-graph                               ▝▀▀▀▀▀▀▀▀▀▀▀▀▀▀▞▀▀▀▀▀▚
allShapes-allShapes                                               ▝▞▄▄▄▖
angle-equivalence-triangle-mesh-2d                                ▝▀▞▀▖
arrowheads-arrowheads                                             ▝▞▖
caffeine-structural-formula                                       ▝▀▀▞▖
call-directed-graph-directed-multigraph                           ▝▀▀▀▀▀▞▀▚
call-undirected-graph-pseudograph                                 ▝▀▀▀▀▞▖
center-shrink-circle-animation                                    ▝▚▚
circle-example-euclidean                                          ▝▀▞▖
closest-point-test-closest-point                                  ▝▀▀▀▀▀▞▖
collinear-euclidean                                               ▝▚▚
communications-network-one-way-links-graph-simple-directed-graph  ▝▀▀▀▀▞▚
complete-bipartite-graphs-simple-graph                            ▝▀▀▀▀▀▀▀▀▀▀▀▚▀▀▚
complete-graphs-simple-graph                                      ▝▀▀▀▀▀▀▀▀▞▀▖
computer-network-diagnostic-links-graph-pseudograph               ▝▀▀▀▚▚
computer-network-graph-simple-graph                               ▝▀▀▚▚
computer-network-multiple-links-graph-pseudograph                 ▝▀▀▀▚▀▚
computer-network-multiple-one-way-links-graph-directed-multigraph ▝▀▀▀▀▀▚▀▖
concyclic-pair-triangle-mesh-2d                                   ▝▚▚
congruent-triangles-euclidean                                     ▝▀▀▞▖
continuousmap-continuousmap                                       ▝▞▖
cotan-formula-triangle-mesh-2d                                    ▝▀▞▖
cube-graphs-simple-graph                                          ▝▀▀▀▀▀▀▀▀▚▀▖
cubic-bezier-cubic-bezier                                         ▝▀▞▚
cycle-graphs-simple-graph                                         ▝▀▀▀▀▀▞▀▖
glutamine-molecules-basic                                         ▝▀▞▖
half-adder-distinctive-shape                                      ▝▚▚
halfedge-mesh-triangle-mesh-2d                                    ▝▚▚
hybrid-topology-graph-simple-graph                                ▝▀▚▚
hypercube-network-graph-simple-graph                              ▝▀▀▀▚▚
hypergraph-hypergraph                                             ▝▀▀▚▀▀▀▀▀▀▀▖
incenter-triangle-euclidean                                       ▝▀▞▖
influence-graph-simple-directed-graph                             ▝▀▚▚
jobs-trained-matching-graph-simple-graph                          ▝▀▀▚▀▖
jobs-trained-no-matching-graph-simple-graph                       ▝▀▀▚▚
lagrange-bases-lagrange-bases                                     ▝▞▖
linear-array-graph-simple-graph                                   ▝▀▞▖
lines-around-rect-rect-line-dist                                  ▝▚▚
matrix-matrix-addition-matrix-ops                                 ▝▚▚
matrix-matrix-division-elementwise-matrix-ops                     ▝▚▚
matrix-matrix-multiplication-elementwise-matrix-ops               ▝▚▚
matrix-matrix-multiplication-matrix-ops                           ▝▚▚
matrix-matrix-subtraction-matrix-ops                              ▝▚▚
matrix-transpose-matrix-ops                                       ▝▚▚
matrix-vector-left-multiplication-matrix-ops                      ▝▞▖
matrix-vector-right-multiplication-matrix-ops                     ▝▚▚
mesh-network-graph-simple-graph                                   ▝▀▀▀▀▀▀▀▀▀▀▀▀▀▞▀▀▀▀▀▀▀▀▀▖
midsegment-triangles-euclidean                                    ▝▀▞▖
mobius-mobius                                                     ▝▞▖
module-dependency-graph-simple-directed-graph                     ▝▀▀▚▀▖
niche-overlap-graph-simple-graph                                  ▝▀▀▀▀▞▖
nitricacid-lewis                                                  ▝▀▚▀▚
non-convex-non-convex                                             ▝▚▚
one-water-molecule-atoms-and-bonds                                ▝▞▖
parallel-lines-euclidean                                          ▝▚▚
persistent-homology-persistent-homology                           ▝▀▀▞▀▀▀▀▀▚
points-around-line-shape-distance                                 ▝▀▞▖
points-around-polyline-shape-distance                             ▝▀▚▚
points-around-star-shape-distance                                 ▝▀▀▚▚
precedence-graph-simple-directed-graph                            ▝▀▀▞▖
protein-interaction-graph-simple-graph                            ▝▀▀▀▀▀▚▀▀▚
quaternion-group-group-theory-cayley-graph                        ▝▀▞▖
quaternion-group-group-theory-multiplication-table                ▝▀▀▚▞▖
relative-orientation-triangle-mesh-2d                             ▝▚▚
ring-topology-graph-simple-graph                                  ▝▀▚▚
round-robin-graph-simple-directed-graph                           ▝▀▀▀▚▚
scalar-vector-division-matrix-ops                                 ▝▞▖
scalar-vector-left-multiplication-matrix-ops                      ▝▚▚
scalar-vector-right-multiplication-matrix-ops                     ▝▞▖
siggraph-teaser-euclidean-teaser                                  ▝▀▞▖
star-topology-graph-simple-graph                                  ▝▀▞▖
tree-tree                                                         ▝▀▞▖
tree-venn                                                         ▝▀▞▖
tree-venn-3d                                                      ▝▚▚▄▄
triangle-centers-triangle-mesh-2d                                 ▝▚▚
two-triangles-triangle-mesh-3d                                    ▝▚▚
two-vectors-perp-vectors-dashed                                   ▝▞▖
union-graph-simple-graph                                          ▝▀▚▚
vector-vector-addition-matrix-ops                                 ▝▚▚
vector-vector-division-elementwise-matrix-ops                     ▝▞▖
vector-vector-multiplication-elementwise-matrix-ops               ▝▞▖
vector-vector-outerproduct-matrix-ops                             ▝▚▚
vector-vector-subtraction-matrix-ops                              ▝▞▖
vector-wedge-exterior-algebra                                     ▝▞▖
wet-floor-atoms-and-bonds                                         ▝▚▀▖
wheel-graphs-simple-graph                                         ▝▀▀▀▀▀▀▀▚▀▚
word-cloud-example-word-cloud                                     ▝▚▚
wos-laplace-estimator-walk-on-spheres                             ▝▀▞▖
wos-nested-estimator-walk-on-spheres                              ▝▀▚▀▖
wos-offcenter-estimator-walk-on-spheres                           ▝▀▞▚
wos-poisson-estimator-walk-on-spheres                             ▝▀▞▚

@codecov
Copy link

codecov bot commented May 5, 2023

Codecov Report

Merging #1360 (9f721d2) into main (58bd78b) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1360   +/-   ##
=======================================
  Coverage   62.70%   62.70%           
=======================================
  Files          70       70           
  Lines        8738     8738           
  Branches     2110     2110           
=======================================
  Hits         5479     5479           
  Misses       3116     3116           
  Partials      143      143           
Impacted Files Coverage Δ
packages/core/src/contrib/ConstraintsUtils.ts 78.57% <ø> (ø)
packages/core/src/contrib/Minkowski.ts 98.16% <100.00%> (ø)

@liangyiliang liangyiliang marked this pull request as ready for review May 8, 2023 14:48
@liangyiliang liangyiliang requested a review from samestep May 8, 2023 14:55
@liangyiliang liangyiliang merged commit d234a96 into main May 8, 2023
12 checks passed
@samestep samestep deleted the hpsdf-fix branch May 8, 2023 16:07
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

Successfully merging this pull request may close these issues.

containsConvexPolygonPoints with Padding
1 participant