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

Plane.FromPoints returning -1 Z normal for some planar splines causing inverted arcs. #204

Open
ChrisClems opened this issue Dec 18, 2024 · 1 comment
Assignees
Labels
bug Something isn't working enhancement New feature or request

Comments

@ChrisClems
Copy link
Contributor

splineNorms.zip

See the attached drawings in the zip which contain splines that demonstrate the problem.

When adding a plane to a spline using FromPoints, there are situations where a planar flat spline on Z 0 will receive a normal of 0, 0, -1. When importing a DXF with splines and approximating them, this creates Ellipse GeoObjects with a Plane and Normal of 0, 0, -1.

It appears that the symptom of this was noticed and remedied in ExportDxf.cs by inverting the normal when a clockwise ellipse was found. The original symptoms would likely look like an arc with inverted bulge value:

Plane dxfPlane;
if (elli.CounterClockWise) dxfPlane = Import.Plane(Vector3(elli.Center), Vector3(elli.Plane.Normal));
else dxfPlane = Import.Plane(Vector3(elli.Center), Vector3(-elli.Plane.Normal));

This fix caused the resulting arcs to look good in 2D, but the normals of the impacted arcs were inverted to 0, 0, -1. This shouldn't matter for flat objects, but it causes geometry linkage issues in AutoCAD and in CAM software where up matters in 2D. If you attempt to join planar arcs with a -1 Z normal to lines in AutoCAD, they will join into a spline instead of a polyline. I've had this problem previously with geometry exported from Rhino and implemented a similar fix as below in an AutoCAD plugin that swapped the start and end points.

You can demonstrate the issue with this code:

var cadabDxf = new CADability.DXF.Import(dxfFile).Project;
var newProj = Project.CreateSimpleProject();
var newModel = newProj.GetModel(0);
var allEntites = cadabDxf.GetModel(0).AllObjects;
var contours = new GeoObjectList(allEntites
        .Where(ent => ent.Layer.Name == "Outer_Loop" || ent.Layer.Name == "Interior_Loops").ToList());
var compShp = CompoundShape.CreateFromList(contours, 0.001, out Plane plane);
var simpOutline = compShp.SimpleShapes[0].Outline;
var simpHoles = compShp.SimpleShapes[0].Holes;
var simpOutlinePath = simpOutline.AsPath();
var simpOutlineGeo = simpOutlinePath.MakeGeoObject(plane);
newModel.Add(simpOutlineGeo);
foreach (var hole in simpHoles)
    {
    var segments = hole.GetClonedSegments();
    var path = hole.AsPath();
    var pathGeo = path.MakeGeoObject(plane);
    newModel.Add(pathGeo);
    }

var export = new Export(DxfVersion.AutoCad2000);
export.WriteToFile(newProj, $"{outDir}{Path.GetFileNameWithoutExtension(dxfFile)}-2.dxf");

Open the resulting DXF files and check the normal Z value on any arcs generated in the outline border on the simple shape.
 
I've worked around this issue using the changes in this commit. In Plane.FromPoints I'm checking for a < 0 normal Z and 0.0 Z value for all items in the Points array. If this condition arises, the plane is reversed. In ExportEllipse, I've changed the CW/CCW check to swap the start and end points of the ellipse instead of inverting the normal. This so far appears to have resolved the issue in all of my test drawings. I've been unable find any unexpected issues from the plane check change.

My question before I submit this as a PR: Are there any unexpected side effects that I'm not thinking of that could come about by changing the Plane.FromPoints logic as I have? I assume this is some floating point shenanigans in the math that is calculating the plane from points and I'm not sure if there's a better way to address this earlier. I'm also getting some floating point errors in my final geometry but I'm assuming that's a result of not having a way to export chained entities currently so start/end points can be off by miniscule amounts.

@dsn27 dsn27 added bug Something isn't working enhancement New feature or request labels Dec 20, 2024
@SOFAgh
Copy link
Collaborator

SOFAgh commented Dec 22, 2024

Since plane.FromPoints always has two different solutions with the normal pointing in one direction or the opposite direction, and the points in the parameter cannot specify, which direction shoule be preferred, I don't see any problem with always returning the normal vector with z>0. I even don't think it is necessary to check, whether the points are above the XY-plane. This even would make FromPoints more predictable than it is now, where the orientation of the normal is random.

I think there is no clockwise/couterclockwise property for (elliptical) arcs in DXF. This is why the plane is reversed to keep the orientation in DXF. So I think, we shouldn't change the export behaviour.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants