Part Line: Infinit/Finite 2d vs 3d

[ickby]

Hello Werner,

I did follow your effort to add 2d geometry ot the part workbench and highly appretiate this effort! In light of the done changes I like to discuss the topic of the freecad line definition.

In the already available Part::Line (3d Line geometry) two point lead to a trimmed line, basically a segment. This is IMHO confusing, a line should be infinite, and a segment should be called segment. However, it is only a slight inconvienience. Now the 2d coutnerpart Part::Line2d has been added. For this the constructor with 2 points lead to an infinite line. I think this is a bit strange, as the two line definitions work differently which is not ideal. Of course the simple fix would be to make a segment in 2d too, but while working on this, maybe it is also a good time to somehow change the 3d behavior?

I must however admit that I don't know how to do it with out large problems in backward compatibility. Maybe introducing a 3d/2d Part::Segment type and adding a depricated/"change behavior" notification to Part::Line for the next release and change it for 0.18? Some kind of deprication workflow for the API will most likely be needed in the future anyway and this may be a first good example.

[wmayer]

This is IMHO confusing, a line should be infinite

That is true.

This is IMHO confusing, a line should be infinite, and a segment should be called segment.

Just segment or line segment? About segment I am not sure that this implies that a line is meant.

Now the 2d coutnerpart Part::Line2d has been added. For this the constructor with 2 points lead to an infinite line.

You recently wrote somewhere else about the strangeness that line in FreeCAD is a actually a line segment and for the 2d part I wanted to avoid this from the beginning on and made it closer to the OCC meaning.

Of course the simple fix would be to make a segment in 2d too, but while working on this, maybe it is also a good time to somehow change the 3d behavior?

In 2d there is the class Line2dSegment. To be consistent the plan would be to change the 3d stuff and add there the class LineSegment, too. The only problem what I am not sure about is whether this causes regressions in existing scripts if a line now is infinite.

I must however admit that I don't know how to do it with out large problems in backward compatibility. Maybe introducing a 3d/2d Part::Segment type and adding a depricated/"change behavior" notification to Part::Line for the next release and change it for 0.18? Some kind of deprication workflow for the API will most likely be needed in the future anyway and this may be a first good example.

What can the problems be when making a line infinite?

1. You may get additional solutions when checking for intersections with e.g. a sphere, cone, torus, ... But I guess that the majority of scripts won't suffer from this and we just let it happen.

2. Now it is possible to create directly a valid edge from the line. If it's infinite then the edge won't be something that can be displayed any more and many scripts would be affected by such a change. So, to avoid this we can overload the toShape method and create a valid edge (and therefore we have to save the input points). The overloaded toShape method then should raise a deprecation warning to use the line segment class instead.

[ickby]

Just segment or line segment? About segment I am not sure that this implies that a line is meant.

Yes, LineSegment is better.

What can the problems be when making a line infinite?

1. You may get additional solutions when checking for intersections with e.g. a sphere, cone, torus, ... But I guess that the majority of scripts won't suffer from this and we just let it happen.

2. Now it is possible to create directly a valid edge from the line. If it's infinite then the edge won't be something that can be displayed any more and many scripts would be affected by such a change. So, to avoid this we can overload the toShape method and create a valid edge (and therefore we have to save the input points). The overloaded toShape method then should raise a deprecation warning to use the line segment class instead.

Yes that is what I expect too. But I think simply changing without any notification is problematic, this is a hell to debug if you use this in a script. Maybe we could add a warning message about changed behavior when the constructor is used and add a global property to omit the warning? That would than at least require some attention and manual work by the developer to handle this correctly? Like

Code: Select all

l = Part.Line()
>>> Warning: Changed behavior in Version 0.17. You created an infinite line. To supress this warning see "link"
>>> <Line (0,0,0) (0,0,1) >
p = App.ParamGet("User parameter:BaseApp/Preferences/DepricationWarning")
p.SetBool("Line", True)
l = Part.Line(...)
>>> <Line (0,0,0) (0,0,1) >

[ickby]

Just segment or line segment? About segment I am not sure that this implies that a line is meant.

Yes, LineSegment is better.

What can the problems be when making a line infinite?

1. You may get additional solutions when checking for intersections with e.g. a sphere, cone, torus, ... But I guess that the majority of scripts won't suffer from this and we just let it happen.

2. Now it is possible to create directly a valid edge from the line. If it's infinite then the edge won't be something that can be displayed any more and many scripts would be affected by such a change. So, to avoid this we can overload the toShape method and create a valid edge (and therefore we have to save the input points). The overloaded toShape method then should raise a deprecation warning to use the line segment class instead.

Yes that is what I expect too. But I think simply changing without any notification is problematic, this is a hell to debug if you use this in a script. Maybe we could add a warning message about changed behavior when the constructor is used and add a global property to omit the warning? That would than at least require some attention and manual work by the developer to handle this correctly? Like

Code: Select all

l = Part.Line()
>>> Warning: Changed behavior in Version 0.17. You created an infinite line. To supress this warning see "link"
>>> <Line (0,0,0) (0,0,1) >
p = App.ParamGet("User parameter:BaseApp/Preferences/DepricationWarning")
p.SetBool("Line", True)
l = Part.Line(...)
>>> <Line (0,0,0) (0,0,1) >

and then the console output should most likely be changed to <Line Point(0,0,0), Direction(0,0,1) > or so.

[yorik]

This doesn't work anymore:

Code: Select all

obj = FreeCAD.ActiveDocument.addObject("Sketcher::SketchObject", "Sketch")
line = Part.LineSegment(App.Vector(0,0,0),App.Vector(1,1,0)).toShape()
obj.addGeometry(line.Curve)

Traceback (most recent call last):
File "<input>", line 1, in <module>
TypeError: Unsupported geometry type: Part::GeomLine

In Sketcher/App/SketchObjectPyImp.cpp L67, PyObject* SketchObjectPy::addGeometry expects a GeomLineSegment... Not sure which is wrong, line or linesegment. I'm afraid I have to leave it for you Werner

[wmayer]

I have moved your post to the right thread.

The old class Part.Line actually is a line segment while from mathematical point of view a real line is infinite. OCC does it the right way but our Python binding isn't really correct. Since I recently also started the Geom2d Python binding where I avoided this inconsistency ickby asked me to also fix it in the 3d part.

Therefore I introduced the new class Part.LineSegment which does the same as the old class Part.Line. The new class Part.Line will be infinite in the future.
To be backward compatible for some time I added an option (by default on) to show the old behaviour.

But then there is a second change which is causing the trouble. When you have e.g. a face and call its Surface attribute then the returned geometry is always the full and unlimited geometry. While for faces nothing has actually changed there is a slight change for edges. If an edge has a line inside then edge.Curve no longer returns a line segment but an infinite line.

So your example code doesn't work any more because the edge internally has a line. See comments below:

Code: Select all

obj = FreeCAD.ActiveDocument.addObject("Sketcher::SketchObject", "Sketch")
# in OCC a line segment is a trimmed curve which internally has an infinite line as basis curve
line = Part.LineSegment(App.Vector(0,0,0),App.Vector(1,1,0))
# when creating the edge then the algorithm uses the boundaries of the trimmed curve to create a limited edge but the underlying line as geometry 
edge = line.toShape()
# the Curve is the geometry of the edge, i..e the infinite line
obj.addGeometry(edge.Curve)

In the future you should adjust your code to be:

Code: Select all

obj = FreeCAD.ActiveDocument.addObject("Sketcher::SketchObject", "Sketch")
line = Part.LineSegment(App.Vector(0,0,0),App.Vector(1,1,0))
obj.addGeometry(line)

But I will use the above option to also temporarily restore the old behaviour of "Curve".

BTW, I realized that I broke something in Arch and Draft which is hopefully fixed by the above planned change. When running their unit tests you will get three failures.

Code: Select all

r = Draft.makeRectangle(4,2)
r2 = Draft.offset(r,FreeCAD.Vector(-1,-1,0),copy=True)

Code: Select all

r = Draft.makeRectangle(4,2)
newwire = DraftGeomUtils.offsetWire(r.Shape,FreeCAD.Vector(-1,-1,0))
DraftGeomUtils.hasCurves(newwire)

I will work on it tomorrow since it's quite late already.

[yorik]

Great! Thanks for the explanation. Indeed it makes total sense.. It will behave like the arc of circle edge, where the underlying curve is the whole circle. Don't worry about possible broken things in arch and draft, now that i understand better the concept it should be no problem to fix things as they come.

[wmayer]

I have restored the old behaviour of Curve again git commit 846f062. So, this code works again

Code: Select all

obj = FreeCAD.ActiveDocument.addObject("Sketcher::SketchObject", "Sketch")
line = Part.LineSegment(App.Vector(0,0,0),App.Vector(1,1,0)).toShape()
obj.addGeometry(line.Curve)

but you will be spammed with a deprecation warning.

You can switch to the future mode be setting the user parameter BaseApp/Preferences/Mod/Part/General/LineOld = false.

For new code you can write

Code: Select all

obj = FreeCAD.ActiveDocument.addObject("Sketcher::SketchObject", "Sketch")
line = Part.LineSegment(App.Vector(0,0,0),App.Vector(1,1,0))
obj.addGeometry(line)

or

Code: Select all

obj = FreeCAD.ActiveDocument.addObject("Sketcher::SketchObject", "Sketch")
line = Part.LineSegment(App.Vector(0,0,0),App.Vector(1,1,0)).toShape()
obj.addGeometry(Part.LineSegment(line.Curve,line.FirstParameter,line.LastParameter))

These two examples work if LineOld is on or off but if it's on you still get the deprecation warning. So, it's best to switch it off and use the new style everywhere.

[mlampert]

With this change some parts in Path are broken - I can fix them but need some clarification.

In DraftGeomUtils.geomType(...) it checks for the type LineSegment - but does return the string 'Line' (might be worth to clean up as well).
However, we still have Part.Line curves in the edges.

The edge list to work with gets created by TechDraw.findShapeOutline(...). Haven't figured out yet where the lines come from.

Recommendations for the fix?

[mlampert]

BTW - pulling the latest line restores Path's functionality and creates a slew of deprecation warnings

What would be really great is if the deprecation warning would include where it comes from, but I guess it's rather tricky to investigate/print the stack trace.