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

In certain lathe groups, triangle normals get flipped #240

Closed
whitequark opened this issue Apr 20, 2017 · 8 comments
Closed

In certain lathe groups, triangle normals get flipped #240

whitequark opened this issue Apr 20, 2017 · 8 comments
Labels
Milestone

Comments

@whitequark
Copy link
Contributor

whitequark commented Apr 20, 2017

See this file: what.zip. If you drag the arc changing its diameter, sometimes the solid will become red. If you enable view of triangle mesh, it can be seen that all triangles get flipped.

@phkahler
Copy link
Member

I was going to open an issue that I think is the same as this. When implementing revolve I find that dragging points around in a sketch can cause the loop normals to flip which causes odd thing to happen when revolving. There is code in extrude and lathe to compensate, but I removed it from my revolve function because it causes other problems. Now my surfaces just turn red instead of the geometry flipping. This is what the "fix" in extrude and lathe prevent from happening, but I'm not sure why it doesn't work in the file you included. If we comment this out:

axis = axis.ScaledBy(-1);

from inside SShell::MakeFromRevolutionOf() your file just has a consistently red surface. Making that line unconditional will probably fix the red all the time (and brake everything else).

The root cause seems to be that loops and loop sets have their own normal that is not necessarily in the same direction as the underlying sketch normal. I'm actively looking into how this is determined and what might be done to fix it. Then these corrections in other places can probably be removed. I'll keep this file as a test case because it isn't even handled correctly now. The issue is that loops use their direction around the normal to indicate the loop normal, and hence the swept surface normal when revolved or extruded. Loop normals need to take a preferred direction from the underlying sketch normal to fix everything. I think.

@phkahler
Copy link
Member

I'm thinking your attached file fails because the sketch contain no entities with starting points off of the axis of revolution. The code at the top of MakeFromRevolutionOf() takes the furthest point from the axis, projects it on that axis to get a vector. In this case that vector is essentially zero length except for rounding errors. The result is randomly flipping the normal and bunch of errors from trying to normalize a zero vector.

@whitequark
Copy link
Contributor Author

@phkahler Thank you for the investigation!

@phkahler
Copy link
Member

@whitequark No problem! Just tried something to confirm. if you increase the angle of the arc, Solvespace will generate more than one curve for it. The point that connects the two curves will be off of the axis and this problem will go away.

I was going to say the solution to this bug lies in GetPlaneContainingBeziers() which is only checking end points of curves and not all the control points. I thought I saw code that made this error but I can't find it now. It's making me wonder if somewhere the degree of a curve is being assigned the wrong value. That might explains some other bugs, but it seems like that would cause a lot more problems. Please tag me if you find the solution to this one.

@whitequark
Copy link
Contributor Author

Please tag me if you find the solution to this one.

Computer graphics is (of the areas used in SolveSpace) my weakest point, by far. I'll happily handle whatever ridiculous things are needed to be able to use GTK or OpenGL or whatever else but I can't really work on the core of it, sadly.

@phkahler
Copy link
Member

@whitequark

Computer graphics is (of the areas used in SolveSpace) my weakest point, by far.

Oh, I like vector math quite a bit. SolveSpace is IMHO a gem in the open source world and I just want it to have some more stuff. Stuff that I may be able to implement, so I'm giving it a try. We shall see what happens.

@phkahler
Copy link
Member

        // Choose the point farthest from the axis; we'll get garbage
        // if we choose a point that lies on the axis, for example.
        // (And our surface will be self-intersecting if the sketch
        // spans the axis, so don't worry about that.)
        for(i = 0; i <= sb->deg; i++) {
          Vector p = sb->ctrl[i];
          double d = p.DistanceToLine(pt, axis);
          if(d > md) {
              md = d;
              pto = p;
          }
       }

This differs by 2 lines and a curly brace from the same code in MakeFromRevolutionOf and fixes this issue.

@whitequark
Copy link
Contributor Author

Fixed in fa66229, thank you!

@whitequark whitequark added this to the 3.0 milestone May 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants