Sketcher: Ellipse support

Here's the place for discussion related to coding in FreeCAD, C++ or Python. Design, interfaces and structures.
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: Sketcher: Ellipse support

Post by abdullah »

DeepSOIC wrote:Sketch.cpp, line 2337
if (pos == mid | pos == none) {
should be boolean or "||", not bitwise or "|". Thanks goes to compiler warning. This doesn't actually affect anything ;)
Thanks!!
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: Sketcher: Ellipse support

Post by abdullah »

marktaff wrote: The only commit after that turns off the autoconstraint from suggested constraint for the ellipse center for my construction method. It is a temp hack. With it on, my method exhibits weird behavior where ellipse 1 gets resized/moved when ellipse 2 is constructed on it. I think the root of the issue is the suggested constraint when drawing assumes your construction method (as mine didn't exist when you wrote that code). So that autosuggest code will have to be tweaked to support both methods intelligently. We can add that to the todo list, but I think we should wait until after I refactor to allow axis-switching in my method.
I have included your last commit, so that the testing can go more smoothly.
marktaff wrote:Next up for me is the refactor to allow axis-switching, then more digging into the solve() issues. I don't think we need to wait on a fix for the solve() issues to commit to master though, as those issues are more general than just the ellipses. But hey, maybe that solution will be trivial. :-)
Nice!!
User avatar
DeepSOIC
Veteran
Posts: 7896
Joined: Fri Aug 29, 2014 12:45 am
Location: used to be Saint-Petersburg, Russia

Re: Sketcher: Ellipse support

Post by DeepSOIC »

OK, my tangency via point is ready for first testing. Use my EllipseTangencyViaPoint branch.
Features and benefits:
* tangency via point for any combination of: Circle/Arc, Ellipse/ArcOfEllipse
* tangency is created where the user asked for (where the point was), not where it was guessed by some "smart" code
* two ellipses can be easily made tangent in two places at the same time (see teaser 1).
* the old tangency coexists and can be tested in the same sketch
* all works through a single constraint formula in Constraints.cpp
Instructions:
* Select two curves and a point where you want the tangency, and click the tangency constraint on toolbar (point can be an endpoint of an arc, or whatever point you choose (it will fail of course if the point is the center/focus of one of curves being constrained, because it's impossible).
* to connect two arcs by their endpoints, constrain those endpoints coincident and apply the tangency-via-point. Delete a resulting redundant point-on-curve.
Known issues/todos
* partials are numeric, so expect problems with very large and very small stuff.
* tangency via point to lines is not implemented
* autoconstraints can cause the point that was thrown in to become constrained onto one of the curves. In this case, tangency via point creates an extra (redundant) point-on-object constraint that has to be manually deleted. Similar problem will happen if the point of tangency is already constrained onto one of the curves (there is a check only for endpoints now).
* tangency icon in 3d view is not placed properly
User avatar
DeepSOIC
Veteran
Posts: 7896
Joined: Fri Aug 29, 2014 12:45 am
Location: used to be Saint-Petersburg, Russia

Re: Sketcher: Ellipse support

Post by DeepSOIC »

Please don't integrate my changes now since I plan a major rework to convert the tangency-via-point constraint into an angle constraint.
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: Sketcher: Ellipse support

Post by abdullah »

DeepSOIC wrote: partials are numeric, so expect problems with very large and very small stuff.
But I guess we could make it symbolic, couldn't we? I mean, if it can be done then it will be much more robust.

Anyway, I let you rework everything, then give me the math expression and I will use sage to generate the partials...
User avatar
DeepSOIC
Veteran
Posts: 7896
Joined: Fri Aug 29, 2014 12:45 am
Location: used to be Saint-Petersburg, Russia

Re: Sketcher: Ellipse support

Post by DeepSOIC »

I've spent the whole day fighting with C++ :ugeek: , but I've done the generalization I wanted. I hope to get a working code tomorrow.
The main aim was to avoid stupid if-elseif (a is line, b is ellipse etc...). If it will work, adding angle-via-point support for hyperbola and parabola will be almost trivial (just type some math). The changes required came out bigger than I anticipated.
User avatar
DeepSOIC
Veteran
Posts: 7896
Joined: Fri Aug 29, 2014 12:45 am
Location: used to be Saint-Petersburg, Russia

Re: Sketcher: Ellipse support

Post by DeepSOIC »

WooHoo!
I have some working code of angle-via-point. EllipseAngleViaPoint branch.
Features and benefits:
* angle via point works for all combinations of edges: line, circle, arc, ellipse, arc of ellipse.
* tangency-via-point constraint can be achieved by setting an angle to zero or 180 degrees.
* when used as tangency, it locks down the type of tangency (internal tangency will never swap to external). Angle of 0 corresponds to internal tangency, angle of 180 - external tangency.
* can be used as perpendicular constraint between anything, just as for tangency.
* old constraints are still there.
* no switches like if a is arc and b is ellipse. The appropriate formulas are picked automatically thanks to polymorphism (that's what my fight with c++ was about).
Instructions:
* Select two curves and a point where you want the angle, and click the angle constraint on toolbar (point can be an endpoint of an arc, or whatever point you choose.) The angle is counted from first selected edge to second selected edge in ccw direction.
Known issues/todos
* [solved] angle is not precalculated basing on existing geometry, and angle of zero constraint is applied immediately, often causing unsolvable situations. I'll address this asap.
* [fixed]partials are numeric, so expect problems with very large and very small stuff.
* [fixed] autoconstraints can cause the point that was thrown in to become constrained onto one of the curves. In this case, angle via point creates an extra (redundant) point-on-object constraint that has to be manually deleted. Similar problem will happen if the point of angle is already constrained onto one of the curves (there is a check only for endpoints now).
* angle icon in 3d view is not placed properly.
Last edited by DeepSOIC on Mon Nov 03, 2014 1:46 am, edited 2 times in total.
User avatar
DeepSOIC
Veteran
Posts: 7896
Joined: Fri Aug 29, 2014 12:45 am
Location: used to be Saint-Petersburg, Russia

Re: Sketcher: Ellipse support

Post by DeepSOIC »

Precalculation of angle now works, and I kindly ask for feedback.
Not only on how it works, but also on code side of things. The changes I've done are significant (fundamental, in Geo.h), but all the old code is untouched and should be still working.
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: Sketcher: Ellipse support

Post by abdullah »

DeepSOIC wrote:Precalculation of angle now works, and I kindly ask for feedback.
Not only on how it works, but also on code side of things. The changes I've done are significant (fundamental, in Geo.h), but all the old code is untouched and should be still working.
I promise to have a look as soon as I have time... hopefully this week.
User avatar
DeepSOIC
Veteran
Posts: 7896
Joined: Fri Aug 29, 2014 12:45 am
Location: used to be Saint-Petersburg, Russia

Re: Sketcher: Ellipse support

Post by DeepSOIC »

I've made the automatic point-on-curve additions as smart as I can think of (and very simple by the way - it simply tests if the point is geometrically on the curve, and if not so, adds point-on-object constraint).
There still are some ways to trick it to not create necessary point-on-object constraints (I know at least one), but they are very unlikely to be encountered, and can be resolved manually.
Post Reply