Skip to content

Conversation

@phkahler
Copy link
Member

…oes not flip when the revolve angle changes direction. Not sure how to fix that.

…oes not flip when the revolve angle changes direction. Not sure how to fix that.
@phkahler
Copy link
Member Author

I tried to do this years ago when I implemented revolve and it didn't quite work. Now I thought we can just swap the handles for the end points if the angle is negative, but that doesn't work.

@jwesthues Do you remember how regeneration works? Are entities created via copy types (extrusion lines for ex) only created the first time, or are they discarded and recreated when the group is generated? Obviously if an underlying sketch has new elements then regeneration has to create new one in an extrusion, but how are existing ones handled?

@jwesthues
Copy link
Member

All entities get discarded and recreated with each regeneration. It feels wasteful, but the workload is very small compared to the NURBS or mesh Booleans. A line or curve sketched directly by the user is preserved only because it's stored as a Request.

@phkahler
Copy link
Member Author

phkahler commented Oct 23, 2025

All entities get discarded and recreated with each regeneration. It feels wasteful, but the workload is very small compared to the NURBS or mesh Booleans. A line or curve sketched directly by the user is preserved only because it's stored as a Request.

@jwesthues OK, that's what I thought. That also explains why generated entities can't have custom styles - they'd revert on regeneration. But in this PR I tried 2 ways to swap the arc endpoints based on the direction of the revolve (sign of the angle) and neither of them work. If you swap them unconditionally then the arcs go "the other way around" as expected, but it's still independent of which way you drag.

So when a group gets generated, the group parameters are set to defaults - in this case the revolve angle is set to a default 120 degrees. Perhaps this default is being used in the code to flip the ends. But that begs the question, how do those parameters get back to their previously solved values? If an extrude were reset to defaults you couldn't apply a length constraint on the opposite side of the default or it would flip when regenerated. I need to print the angle where I'm checking it...

Where should I be checking the sign of the revolve (parameter 3 of the group) and flipping these endpoints? Or how can I check the parameter value of the previous solve?

@phkahler
Copy link
Member Author

Unrelated to this PR, I think it would be good not to delete and recreate entities during regeneration. For one, that would allow custom styles for those entities. For two, when I get around to completing a chamfer/fillet tool I'd like to map shell edges to their corresponding sketch entities so we can assign a fillet radius or chamfer depth to the entities and have the shell update/modify with those features. This information (one double) would need to survive regeneration and file save/load.

@jwesthues
Copy link
Member

The parameters actually do get saved, to preserve the information in the initial point location dragged by the user. Entity and parameter numbering is consistent across regeneration, even when entities end up created or deleted, so the parameters still end up associated with the right entities.

That said (but without ever having looked closely at the <360 degree lathe feature), I'm surprised that swap would be required. An arc entity should always run counterclockwise from its start point to its end point when viewed with its normal facing us. This defines any arc angle in [0, 360) degrees, which seems like what you want. Perhaps the trouble is in the normal? The points alone aren't enough to define the arc, since they fail to define a plane when they're collinear and since they never define the direction of travel. I think you probably have to create a new normal for this purpose, since we can e.g. lathe about a line that's not parallel to any existing normal.

Preserving the Entity objects sounds painful to me, very easy to make mistakes when stuff gets deleted. I'd use a different object for the persistent state, associated with the desired hEntity by consistent handle numbering, as for a Request or Param now. If I did want to use the same object, then I'd preserve by caching the old entities, deleting and regenerating, and copying from the cache to the survivors.

@phkahler
Copy link
Member Author

That said (but without ever having looked closely at the <360 degree lathe feature), I'm surprised that swap would be required. An arc entity should always run counterclockwise from its start point to its end point when viewed with its normal facing us. This defines any arc angle in [0, 360) degrees, which seems like what you want. Perhaps the trouble is in the normal?

@jwesthues The revolve group can be dragged either side of the originating sketch plane. So +270 degrees is not the same as -90 degrees, so the valid range is +/-360. The angle can technically go way beyond that and is also used in the Helix groups where it can cover many turns with +/- pitch. Anyway, I tried swapping the point handles and also tried pointing the normal in the opposite direction - both based on the sign of the angle - and neither works here. When I originally made the revolve group I concluded that we'd need an arc entity defined in a different way to create these things, but now I see it's just a matter of getting the existing ones to "go the other way" when needed. The points will not actually be swapped, just their handles withing the arc entity.

@jwesthues
Copy link
Member

My inclination would have been to keep the point mapping in the arc constant and handle positive vs. negative angle by flipping the normal, thus keeping the semantics that the arc always begins at the original point. Either way seems like it should work though, and your implementation of the normal flip looks correct to me. (That doesn't handle the helix case, but the curve isn't an arc in that case.) The logical next debugging step seems like it's indeed to check whether you're actually getting the correct angle, then either fix that or look for a non-fundamental issue in the arc generation.

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