fix centroids in polygons with collinear vertices

[wynged]

BACKGROUND:

• Centroids were returning strange results because vertices that were collinear with other perimeter vertices could move the centroid. This isn't really sensible because a centroid represents a sort of center of mass of the 2D shape, and shouldn't be affected by extra perimeter vertices.

DESCRIPTION:

• Remove collinear vertices before computing the centroid.

TESTING:

• Test passes.

FUTURE WORK:

• Is there any future work needed or anticipated? Does this PR create any obvious technical debt?

REQUIRED:

• All changes are up to date in CHANGELOG.md.

COMMENTS:

• Any other notes.

---

This change is 

[ikeough]

What's the status here? Should we push to get this in?

[andrewheumann]

This is a good candidate for 2.0 since it's a somewhat breaking change. Can we add a test for Centroid that would have failed under the old method?

Reviewed all commit messages.
Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @wynged)

---

CHANGELOG.md line 53 at r1 (raw file):

- Handle quotes in string literals for content catalog code generation by doubling them up.
- Fix `AdaptiveGrid.TryGetVertexIndex` returning `false` for existing vertex if other vertex has similar X or Y coordinate.
- Fix he polygon centroid calculation to remove collinear vertices.

he => the

---

Elements/test/PolygonTests.cs line 146 at r2 (raw file):

            Assert.Equal(x, centroid.X);
            Assert.Equal(y, centroid.Y);
        }

The fact that these identical tests pass for both Center and Centroid means they're not very good test cases. These two tests together are effectively asserting that Centroid+Center have the same behavior, when obviously they should not. Can we add some better test cases to the theory, and maybe separate out expected values for center + centroid? I want to see examples like this:

var pgon = new Polygon((20.710443, 4.926839, 0),
(16.247129, 18.703601, 0),
(20.769954, 26.440012, 0),
(28.179055, 23.52398, 0),
(25.571221, 16.897954, 0),
(30.202424, 13.37738, 0),
(28.565876, 9.003332, 0),
(20.710443, 4.926839, 0));

The area centroid of this polygon should be {22.862188, 15.809823, 0}

and the vertex average should be {24.320872, 16.124728, 0}

---

Elements/test/PolygonTests.cs line 162 at r2 (raw file):

            //The center and centroid methods return different results for polygons with colinear vertices.
            Assert.NotEqual(polygon.Center(), centroid);

I don't really like expressing this test in terms of the behavior of other methods. Can we kill this test and just improve the theories above?

[wynged]

Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @andrewheumann)

---

CHANGELOG.md line 53 at r1 (raw file):

Previously, andrewheumann (Andrew Heumann) wrote…

he => the

Done.

---

Elements/test/PolygonTests.cs line 146 at r2 (raw file):

Previously, andrewheumann (Andrew Heumann) wrote…

The fact that these identical tests pass for both Center and Centroid means they're not very good test cases. These two tests together are effectively asserting that Centroid+Center have the same behavior, when obviously they should not. Can we add some better test cases to the theory, and maybe separate out expected values for center + centroid? I want to see examples like this:

var pgon = new Polygon((20.710443, 4.926839, 0),
(16.247129, 18.703601, 0),
(20.769954, 26.440012, 0),
(28.179055, 23.52398, 0),
(25.571221, 16.897954, 0),
(30.202424, 13.37738, 0),
(28.565876, 9.003332, 0),
(20.710443, 4.926839, 0));

The area centroid of this polygon should be {22.862188, 15.809823, 0}

and the vertex average should be {24.320872, 16.124728, 0}

ok, took a stab at it, consolidating the centroid/center and the collinear vertices tests into one test. Added your polygon and values (thanks for that)

---

Elements/test/PolygonTests.cs line 162 at r2 (raw file):

Previously, andrewheumann (Andrew Heumann) wrote…

I don't really like expressing this test in terms of the behavior of other methods. Can we kill this test and just improve the theories above?

Done.

[andrewheumann]

Reviewed 2 of 3 files at r4, all commit messages.
Reviewable status: 1 change requests, 0 of 1 approvals obtained

---

Elements/test/PolygonTests.cs line 146 at r2 (raw file):

Previously, wynged (Eric Wassail) wrote…

ok, took a stab at it, consolidating the centroid/center and the collinear vertices tests into one test. Added your polygon and values (thanks for that)

The Centroid method still appears to be incorrectly calculating the 3D centroid of a polygon. Here is a failing test:

 var pgon = new Polygon((15.164654, 33.292556, 13.183789),
            (7.462208, 35.568885, 20.632929),
            (8.147323, 22.539536, 33.843797),
            (23.718464, 18.112903, 18.595288),
            (26.914692, 20.349983, 12.062899),
            (24.18474, 34.732703, 0.01945),
            (7.483351, 40.774657, 14.975199),
            (15.164654, 33.292556, 13.183789));
Assert.True(pgon.Centroid().IsAlmostEqualTo(new Vector3(16.95831, 28.253992, 16.325457)));

[andrewheumann]

Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @wynged)

[andrewheumann]

Reviewed 2 of 2 files at r5.
Reviewable status: complete! 1 of 1 approvals obtained

---

Elements/test/PolygonTests.cs line 146 at r2 (raw file):

Previously, andrewheumann (Andrew Heumann) wrote…

The Centroid method still appears to be incorrectly calculating the 3D centroid of a polygon. Here is a failing test:

 var pgon = new Polygon((15.164654, 33.292556, 13.183789),
            (7.462208, 35.568885, 20.632929),
            (8.147323, 22.539536, 33.843797),
            (23.718464, 18.112903, 18.595288),
            (26.914692, 20.349983, 12.062899),
            (24.18474, 34.732703, 0.01945),
            (7.483351, 40.774657, 14.975199),
            (15.164654, 33.292556, 13.183789));
Assert.True(pgon.Centroid().IsAlmostEqualTo(new Vector3(16.95831, 28.253992, 16.325457)));

OK this is fixed now :)