Sketcher: Ellipse support

[abdullah]

Edited: I was wrong, the foci have to move at dragging at the minor axis.

Not only that Ulrich. When dragging the point at the end of the minor axis, it is not as simple as saying "please reduce b", if b is fixed and you drag the point you still want to drag the ellipse, so for example rotate it. That is the magic of the solver (when it works properly), it does know what can be moved. Fixing an equation is not the way. Eventually we will make it work properly...

Edit: Because DeepSOIC has tested the partials, this must be solved by looking into what we do in the solver (including also what DeepSOIC pointed out about the reconstruction of the ellipse). If the solver gets stuck, the solution is to improve it... we, together, will manage...

[abdullah]

This is the first n rows of the Jacobian (missing the length constraints):

I have to admit, that I am not familiar with matrix calculations. I only remember a statement from I think it was shoogen or ickby, that the matrix should not contain values of two or above. This matrix has 2 and -2 for the PoA. It contains even a value of 3.73205, is this a problem?

Ickby pointed out to this issue some 20-30 posts ago. He referred to "high values" and "non-metric system of equations". He also pointed out that we can not normalize partials. My limited understanding of his words is that high values are undesirable as they slow down convergence, the reasons behind this are unknown to me. However, 2 and 3 maybe are not that "high".

Sorry, I can not explain what I can not understand myself.

[DeepSOIC]

Because DeepSOIC has tested the partials,

I have to admit I have not tested arc angles related partials. It was kinda this: I opened constraints.cpp, found those partial, looked at the error function and ... oh, goodness... what the hell atan() is doing there I don't understand it.... My response to that was to completely avoid using those angle-related constraints. Sorry , I haven't tested those arc's angle related partials yet.

EDIT: I have missed them during my last testing when I had reported all derivatives are correct. Sorry guys . I'll do a test this evening.

[abdullah]

Abdullah, check this out! it fixes the jump-backs on button release.git commit ed4a7dc5529e38da576d24ef7d7b92c8ef2cbe01
BTW. Ellipse arcs are extremely slow to move now...

I have integrated this commit into my sketcher_testing branch. I want to further study what is happening with this, so I will dedicate some time to debug which values the solver is feeding to OCC...

I also want to modify the internal representation of ellipse and arcofellipse, so that the focus point coordinates are not a point, but two doubles, so that the internal solver point array is a one-to-one match to the visual one, and I can "undo" the patch I submitted yesterday. I think it is a much more cleaner way to proceed...

[DeepSOIC]

I want to further study what is happening with this, so I will dedicate some time to debug which values the solver is feeding to OCC...

Here is a quick rip from opencascade documentation:

void Geom2d_Ellipse::SetMajorRadius ( const Standard_Real MajorRadius )


Assigns a value to the major radius of this ellipse. <br>

Exceptions
Standard_ConstructionError if:

•the major radius of this ellipse becomes less than
the minor radius, or

•MinorRadius is less than 0.

Got this from here: http://dev.opencascade.org/doc/refman/h ... lipse.html

[abdullah]

Because DeepSOIC has tested the partials,

I have to admit I have not tested arc angles related partials. It was kinda this: I opened constraints.cpp, found those partial, looked at the error function and ... oh, goodness... what the hell atan() is doing there I don't understand it.... My response to that was to completely avoid using those angle-related constraints. Sorry , I haven't tested those arc's angle related partials yet.

EDIT: I have missed them during my last testing when I had reported all derivatives are correct. Sorry guys . I'll do a test this evening.

Oh well! I have time. After having to change branch to my todays pull_request, now I have to compile the sketch_testing again.

Atan is a way of looking for errors in angle in a "comfortable way". I explain myself. If you usually have expressions with cost, sint, to calculate an error would mean doing arcsin,arccos, and then, you might still have problems because of different quadrants. So I, inspired by the normal arc and after "reverse engineered" it, decided to use the tan of the difference:

tan(alpha-beta)=(sinacosb-cosasinb)/(cosacosb+sinasinb)

then of course you need one arc, atan of that inside. Atan also has a very nice derivative for partial calculation. Everything is my sage worksheet that you can find some posts ago...

[abdullah]

I want to further study what is happening with this, so I will dedicate some time to debug which values the solver is feeding to OCC...

Here is a quick rip from opencascade documentation:
Got this from here: http://dev.opencascade.org/doc/refman/h ... lipse.html

Yes, that I know. I had to look into it when building the support for GeomArcofEllipse. I know an expection is thrown. What I mean is which values the solver is passing to OCC, how this values are calculated, can we do something to steer the solver to improve convergence (avoid divergence). In any case, it pretends to be complementary to your patch, which addresses a totally different situation (i.e. when the values that the ellipse had before solving colide with those after solving).

Thanks for that patch. I was not seeing it at all...

https://github.com/abdullahtahiriyo/Fre ... er_testing

[abdullah]

Abdullah, check this out! it fixes the jump-backs on button release.git commit ed4a7dc5529e38da576d24ef7d7b92c8ef2cbe01
BTW. Ellipse arcs are extremely slow to move now...

Ok. I have just tested your code:
1) solves the problem with the problematic sketch from Ulrich (the one jumping back)
2) it is not slow at all.

2) might be related to a bug fix I did. When setting constraints in CGS.cpp, you can do this by referring to other constraints like in:

Code: Select all

int System::addConstraintInternalAlignmentEllipseFocus1(ArcOfEllipse &a, Point &p1, int tagId)
{
           addConstraintEqual(a.focus1_X, p1.x, tagId);
    return addConstraintEqual(a.focus1_Y, p1.y, tagId);
}

Here it is very easy to miss the last parameter (tagID) that is optional and defaults to zero. Because it is optional, the compiler won't complain. This is a "priority" setting in the solver, which is known (from some comments in the code) to generate problems, if zero (still is needed in some situations). Generally, if you see some addConstraintXXX(aa,bb,cc) without having the last parameter as tagId, there is a high chance that it is generating a problem.

Latest code at:

https://github.com/abdullahtahiriyo/Fre ... master.git
6454374..116f7d8 sketcher_testing

[DeepSOIC]

Can you please also check out this one: git commit f41f0ac49a566d0525cd147486919ef300144019?
I got rid of arc's angle tracking constraints completely, which is beneficial in that there are less solver parameters required for ellipse arc. The same thing can be done for circular arcs and I think it is worth doing. Sorry for duplicating my own self
A code cleanup will be required after the commit, since ArcOfEllipse.****angle are not used anymore; the new constraint with that atan is not needed too...

[abdullah]

Can you please also check out this one: git commit f41f0ac49a566d0525cd147486919ef300144019?
I got rid of arc's angle tracking constraints completely, which is beneficial in that there are less solver parameters required for ellipse arc. The same thing can be done for circular arcs and I think it is worth doing. Sorry for duplicating my own self
A code cleanup will be required after the commit, since ArcOfEllipse.****angle are not used anymore; the new constraint with that atan is not needed too...

I took a look at that commit and I understand the change in the code. However, I am still not convinced that the change would bring any satisfactory result. Unless the change, for example would solve Ulrich frozen sketch or any other problem... could you check this?

I explain where I stand:
1) When I code, I try to replicate the structures already in place, so that the code is uniform.
2) The arc of circle is coded in this way. Probably there was a reason to do it so. It is not that I think that what it is now is coded by powerful beings that know better (though probably it is neverthess true ) and that that code can not be changed, but I would like to have a sound reason to depart from their approach.
3) In principle, removing two parameters with a simple parameter dependency (they merely touch two columns) should not bring any advantage to the solver (unless the constraint is wrongly coded) . That is really peanuts for such a solver running in a modern computer.

Nevertheless, I have not tried the commit yet, can you tell me if you can appreciate any advantage in general? have you tried the problematic sketches Ulrich submitted with it, any improvement? If for example it fixes the frozen sketch of Ulrich , I integrate it right away

BTW, I still had something for you from another post, about Ctrl-A used for alignment constraint. The shortcuts of the sketcher are getting complicated (mostly thanks to me). So after this project is finished, I plan to dedicate so time to try to improve the situation...