Sketcher: Bezier curves

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: Bezier curves

Post by abdullah »

Is it intended behaviour that an exception text message is modified to start with "FreeCAD exception thrown ("?

I mean, I understand that such a text is used when outputting the message, but I find it strange that the message itself is modified.
User avatar
microelly2
Veteran
Posts: 4688
Joined: Tue Nov 12, 2013 4:06 pm
Contact:

Re: Sketcher: Bezier curves

Post by microelly2 »

Is it possiible to display the length of the spline during edit mode (like the curvature)?
I can ask the sketch.Shape.Edge.Length but I need always a recompute the document to get the value.
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: Sketcher: Bezier curves

Post by abdullah »

microelly2 wrote:Is it possiible to display the length of the spline during edit mode (like the curvature)?
I can ask the sketch.Shape.Edge.Length but I need always a recompute the document to get the value.
Does this suit your use case?

App.ActiveDocument.Sketch.Geometry[22].length()

The function is applicable to any curve.
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: Sketcher: Bezier curves

Post by abdullah »

I have made a PR:
https://github.com/FreeCAD/FreeCAD/pull/664

It is not necessarily closed.

I have finish with the list of todos, but the issue with the text of the pop-up dialog includes "FreeCAD exception thrown (xxxx).". See:
https://forum.freecadweb.org/viewtopic. ... 70#p168075

I am not happy with that. However, I do not know if the solution is to change the behaviour of the code that prepends that "FreeCAD exception thrown", to extend Exception to have another field, so that one member has "A prepend" and the other just the message, plus a method to grab only the message,... or any other solution.

Let me know any issue you may encounter :)
wmayer
Founder
Posts: 20309
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: Sketcher: Bezier curves

Post by wmayer »

I am not sure I am understanding you well. I understand that you are pointing out a potential problem (not being able to assign the tag id) to make me aware (thanks!). I perceive that in principle, if there is not a problem today AND if after thinking there is not a perceived need for it, you would prefer not to allow assigning tag ids. Right?
What I basically want to achieve are two things:
1. The various methods to create a copy (you distinguish between close and copy but for me it's actually the same) should be consistent.
2. The API shouldn't allow it to be used in an unintended way.

Now am not sure where at the moment for the sketcher we currently use the clone() method and where it's important that the tag id doesn't change. When checking the code this is the case in PropertyGeometryList, Sketch::getPyGeometry(), SketchObject::toggleConstruction and SketchObject::setConstruction (not sure about other locations).

Now if this requirement is only given in a few situations we can say the clone() method creates a new tag id and if we need the same tag id a new method like assignTagId(Part::Geometry*) can be implemented which then assigns the tag id from the one to the other geometry and also checks that both geometries are of the same type.

If you say there are much more locations where we need the tag id of a clone to be the same we can follow your suggestion to keep the tag id in the clone() method identical and then add a method createNewTagId() which creates a new random tag id and assigns it when needed. In this case we can't fullfil point 1.) but then it should be clearly documented and that the programmer has to use createNewTagId if a different geometry is desired.

To point 2.): it must not be allowed to directly set a certain tag id because otherwise it happens that the same value is used again and again.
wmayer
Founder
Posts: 20309
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: Sketcher: Bezier curves

Post by wmayer »

abdullah wrote:Is it intended behaviour that an exception text message is modified to start with "FreeCAD exception thrown ("?

I mean, I understand that such a text is used when outputting the message, but I find it strange that the message itself is modified.
This is done in the generated *Py.cpp files and by adding the prefix it's easier to determine that the originally raised exception was of type Base::Exception which simplifies the search of the root cause.

Unfortunately from a Python method we cannot raise C++ exceptions through all calling instances up to the caller that should handle it because this would break a lot of Python internals. Instead we must convert it to a Python exception by using the PyErr_SetString (or friends) and later convert back to a C++ exception.

Although not ideal the best we can do at the moment is to check for the prefix and remove it when not desired.
wmayer
Founder
Posts: 20309
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: Sketcher: Bezier curves

Post by wmayer »

1. Fix curvature comb as indicated by Werner
2. Implement decrease knot multiplicity
3. Review return types in functions (I have some boolean returning functions that should probably better off by returning nothing and raising an exception if something happens)
4. Ask for icons if icons are delivered before merge, integrate icons, otherwise they will be merged to master when ready.
5. Fix annoying bug that crashes FC when undo is effected after a multiplicity change.
6. Do something so that FC compiled against OCC<6.9.0 does not crash.
7. Check Werner's solver test case issue.
The curvature comb where two adjacent curves are made C2-continuous is fixed whereas moving a single poles still affect the curvature comb of the whole curve.

The missing icons are:
Cannot find icon: Sketcher_BSplineKnotMultiplicity
Cannot find icon: Sketcher_BSplineIncreaseKnotMultiplicity
Cannot find icon: Sketcher_BSplineDecreaseKnotMultiplicity

Points to discuss:
https://github.com/FreeCAD/FreeCAD/pull ... 5e7e9426c3
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: Sketcher: Bezier curves

Post by abdullah »

[Deleted] Duplicated
Last edited by abdullah on Sat Apr 08, 2017 1:50 pm, edited 1 time in total.
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: Sketcher: Bezier curves

Post by abdullah »

wmayer wrote:
I am not sure I am understanding you well. I understand that you are pointing out a potential problem (not being able to assign the tag id) to make me aware (thanks!). I perceive that in principle, if there is not a problem today AND if after thinking there is not a perceived need for it, you would prefer not to allow assigning tag ids. Right?
What I basically want to achieve are two things:
1. The various methods to create a copy (you distinguish between close and copy but for me it's actually the same) should be consistent.
2. The API shouldn't allow it to be used in an unintended way.

Now am not sure where at the moment for the sketcher we currently use the clone() method and where it's important that the tag id doesn't change. When checking the code this is the case in PropertyGeometryList, Sketch::getPyGeometry(), SketchObject::toggleConstruction and SketchObject::setConstruction (not sure about other locations).

Now if this requirement is only given in a few situations we can say the clone() method creates a new tag id and if we need the same tag id a new method like assignTagId(Part::Geometry*) can be implemented which then assigns the tag id from the one to the other geometry and also checks that both geometries are of the same type.

If you say there are much more locations where we need the tag id of a clone to be the same we can follow your suggestion to keep the tag id in the clone() method identical and then add a method createNewTagId() which creates a new random tag id and assigns it when needed. In this case we can't fullfil point 1.) but then it should be clearly documented and that the programmer has to use createNewTagId if a different geometry is desired.

To point 2.): it must not be allowed to directly set a certain tag id because otherwise it happens that the same value is used again and again.
Arguments:
It is still not absolutely clear what you mean by consistent. However, it may not be important as you have detailed two implementations and I understand that.

I do not favour the second implementation (createNewTagId), if only, because it implies (constructor assigning a new tagid + clone assigning a tag id + createNewTagId assigning a new tagid), unless there is no other way.

I think there is an intermediate solution, mostly your described solution number 1, with some tweaks for consistent terminology, that will also simplify the coding and using these functions. With your solution 1, there is an inconsistency to call clone something that produces an object that is not exactly the same. The thing is that before my changes the clone function was producing a clone, if we implement our solution 1, clone will not produce a clone, if only, because the tag is different. I think this is confusing and should be avoided.

I was referring to clone and copy as concepts more than member functions. As a concept, what I perceive semantically from a clone is that it is indistinguishable from the original, whereas a copy leaves room for differences. For example, well known high quality and usually expensive products in the market have lower quality and/or less-known and/or cheaper copies. Such terminology is consistent with Sketcher Constraints.

In the sketcher I need copies (different tagid) and clones (same tagid). Also, while doing further coding these days, I came to the conclusion that I would love to have a uniform way of creating copies and clones, so that my mind does not need to think: how do I handle now that tag? It should be uniform. Already as part of Carbon Copy, I have a separate commit with a copy() member in Geometry, like clone() but without assigning the tag. This is used in carbon copy, but will be used for example in array, symmetric element.

Proposed alternative solution:
Having two separate well-documented copy methods, clone() and copy(), and in addition your "assigntagid (geometry *) for the sethandle and handle constructors".

I will prepare the changes.

Question: Do you have anything against me adding the commits of carbon copy on this pull request? That it will avoid that I have to make adaptations to both branches that will finally cancel out, as I have geometry changes in both.
abdullah
Veteran
Posts: 4935
Joined: Sun May 04, 2014 3:16 pm
Contact:

Re: Sketcher: Bezier curves

Post by abdullah »

wmayer wrote:
abdullah wrote:Is it intended behaviour that an exception text message is modified to start with "FreeCAD exception thrown ("?

I mean, I understand that such a text is used when outputting the message, but I find it strange that the message itself is modified.
This is done in the generated *Py.cpp files and by adding the prefix it's easier to determine that the originally raised exception was of type Base::Exception which simplifies the search of the root cause.

Unfortunately from a Python method we cannot raise C++ exceptions through all calling instances up to the caller that should handle it because this would break a lot of Python internals. Instead we must convert it to a Python exception by using the PyErr_SetString (or friends) and later convert back to a C++ exception.

Although not ideal the best we can do at the moment is to check for the prefix and remove it when not desired.
Thanks for the explanation. I am going to give it a go to an alternative idea. If I can not come with anything better, then I will strip the prefix as suggested.
Post Reply