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

increase GetNormal precision #6

Merged
merged 4 commits into from
Feb 8, 2024

Conversation

skemaikin
Copy link
Contributor

No description provided.

@skemaikin
Copy link
Contributor Author

There is an old test - DoNotGetNegativeNormal (NormalTests.cs) - that will break any PR.

@bertt
Copy link
Owner

bertt commented Feb 7, 2024

looks like a pretty normal polygon in that testcase. I'm wondering what could be the issue with it. Maybe something with exterior ring order.

POLYGON Z ((-75.51889015799998 39.15624977500005 17.71,-75.51880883299998 39.15605855000007 17.71,-75.51882902999995 39.156053351000025 17.71,-75.51874978099994 39.155858297000066 17.71,-75.51882395499996 39.155839280000066 17.71,-75.51889671399994 39.15601041700006 17.71,-75.51894566199996 39.15599799200004 17.71,-75.51903664399998 39.15621200700008 17.71,-75.51889015799998 39.15624977500005 17.71))

image

@skemaikin
Copy link
Contributor Author

looks like a pretty normal polygon in that testcase. I'm wondering what could be the issue with it. Maybe something with exterior ring order.

POLYGON Z ((-75.51889015799998 39.15624977500005 17.71,-75.51880883299998 39.15605855000007 17.71,-75.51882902999995 39.156053351000025 17.71,-75.51874978099994 39.155858297000066 17.71,-75.51882395499996 39.155839280000066 17.71,-75.51889671399994 39.15601041700006 17.71,-75.51894566199996 39.15599799200004 17.71,-75.51903664399998 39.15621200700008 17.71,-75.51889015799998 39.15624977500005 17.71))

The normal is (0 -0 -1 ). Is it incorrect?

@skemaikin
Copy link
Contributor Author

skemaikin commented Feb 7, 2024

The best vectors on polygon are 0-1 and 0-6.
image

@skemaikin
Copy link
Contributor Author

skemaikin commented Feb 7, 2024

Oh... sorry! Just stupid evening ))
The correct code is the:

public void DoNotGetNegativeNormal()
{
    // arrange
    var wkt = "POLYGON Z ((-75.51889015799998 39.15624977500005 17.71,-75.51880883299998 39.15605855000007 17.71,-75.51882902999995 39.156053351000025 17.71,-75.51874978099994 39.155858297000066 17.71,-75.51882395499996 39.155839280000066 17.71,-75.51889671399994 39.15601041700006 17.71,-75.51894566199996 39.15599799200004 17.71,-75.51903664399998 39.15621200700008 17.71,-75.51889015799998 39.15624977500005 17.71))";
    var geom = (Polygon)Geometry.Deserialize<WktSerializer>(Encoding.UTF8.GetBytes(wkt));

    // act
    var normal = geom.GetNormal();

    // assert
    Assert.That(normal.X==0 && normal.Y==0 && normal.Z == 0, Is.False);
}

The problems are in the serialization and in Assert.That method.

@bertt
Copy link
Owner

bertt commented Feb 8, 2024

Have you a sample wkt where this method gives better results?

@skemaikin
Copy link
Contributor Author

Have you a sample wkt where this method gives better results?

No, only screenshots that demonstrate an increase in accuracy and image quality when calculating models with very small details.

@bertt
Copy link
Owner

bertt commented Feb 8, 2024

I just check NettopologySuite, there is an existing normal calculation that averages and uses double precision, maybe we can get some inspiration from there

https://github.com/NetTopologySuite/NetTopologySuite/blob/develop/src/NetTopologySuite/Operation/Distance3D/PlanarPolygon3D.cs#L66

        var wkt1 = "POLYGON Z((-75.52562443799997 39.155855306000035 0, -75.52571984999997 39.15583353900007 0, -75.52575546999998 39.155928182000025 0, -75.52566005799997 39.155949949000046 0, -75.52562443799997 39.155855306000035 0))";
        var reader = new NetTopologySuite.IO.WKTReader();
        var geometry = (Polygon)reader.Read(wkt1);
        var planar3d = new PlanarPolygon3D(geometry);

image

@skemaikin
Copy link
Contributor Author

skemaikin commented Feb 8, 2024

I just check NettopologySuite, there is an existing normal calculation that averages and uses double precision, maybe we can get some inspiration from there

https://github.com/NetTopologySuite/NetTopologySuite/blob/develop/src/NetTopologySuite/Operation/Distance3D/PlanarPolygon3D.cs#L66

        var wkt1 = "POLYGON Z((-75.52562443799997 39.155855306000035 0, -75.52571984999997 39.15583353900007 0, -75.52575546999998 39.155928182000025 0, -75.52566005799997 39.155949949000046 0, -75.52562443799997 39.155855306000035 0))";
        var reader = new NetTopologySuite.IO.WKTReader();
        var geometry = (Polygon)reader.Read(wkt1);
        var planar3d = new PlanarPolygon3D(geometry);

It seems to me that the same polygon but with Z-coordinate <> 0 (f.e. z=10) will not give such a result. sum.X will accumulate the value.

Yes, it's more elegant.

@bertt
Copy link
Owner

bertt commented Feb 8, 2024

looks nice this way, I've fixed the tests.

have to do some testing in my tools, does it give good results for you?

@skemaikin
Copy link
Contributor Author

looks nice this way, I've fixed the tests.

have to do some testing in my tools, does it give good results for you?

Yes, everything is working fine now. No any problems with calculation accuracy.

@bertt bertt merged commit 5418019 into bertt:master Feb 8, 2024
1 check passed
@bertt
Copy link
Owner

bertt commented Feb 8, 2024

Thanks I've published 1.3.1 with these changes

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.

2 participants