FreeCAD|
|
It appears it also causes some other artifacts like making any other circles added invisible until the fake dimension is moved. |
|
|
|
|
|
Attached a file that could replicate the bug with minimal operations. Run "bug_2735_replicate.FCMacro" after opening a new/blank sketch. The angle constraint remains even after the line is deleted if the second constraint (coincident, line 10) is also applied, but without it the bug does not happen. Used the following version: OS: Ubuntu 16.04.1 LTS Word size of OS: 64-bit Word size of FreeCAD: 64-bit Version: 0.17.8676 (Git) Build type: None Branch: master Hash: b8163a03dc80b64b38744ed1289e8a7bdab0d378 Python version: 2.7.12 Qt version: 4.8.7 Coin version: 4.0.0a OCC version: 6.8.0.oce-0.17 |
|
|
The bug is in method SketchObject::delGeometry() The sketch has three constraints at this point: constraint 1: coincident constraint 2: coincident constraint 3: angle, the values of the indexes are: First=1 (the geo id of the line), Second=-2 (the geo id of the vertical axis), Third=-2000 (undefined) In two iteration steps this method removes the coincident constraints inside delConstraintOnPoint(). After the first iteration the values of the angle constraint are still correct but with the second step First suddenly becomes -1. Afterwards in a second loop all constraints are deleted if either First, Second, or Third has the same value as GeoId. Since for the angle constraint all values are different it won't be deleted. |
|
|
@wmayer: In my case there are 2 constraints, First for the angle constraint is 0 in the beginning, and it seems to change during the call to transferConstraints(). It appears the origin is marked as the start point of the x-axis (GeoId=-1). The implementation raises a question of what does one mean by the FirstPos and SecondPos for an angle constraint, and is there a way to ensure the transferConstraints doesn't change angle constraints unless perhaps there is a line segment parallel to the deleted one. |
|
|
It seems the logic of deletion of geometries and adjusting constraints is not well thought out. There are so many different kind of constraints which needs an explicit handling somewhere but the current implementation probably makes some implicit assumptions about GeoIds and PointPos values. Here is another example that doesn't correctly clear all associated constraints: 1. draw a line 2. add a point-on-curve constraint with the line and the vertical axis 3. with the same point of the line and the root point make a coincidence 4. delete the line => on the left side the filter shows the point-on-curve constraint This kind of problem comes up if you add constraints between the vertical axis and a geometry and a coincidence between a point of the geometry and the root node. Now internally the GeoId of the vertical axis is -2, -1 of the horizontal axis and also -1 of the root point. Due this ambiguity it happens that when deleting the geometry and with it the coincidence that the -1 of the root node gets interpreted as the horizontal axis. |
|
|
> The implementation raises a question of what does one mean by the FirstPos and SecondPos for an angle constraint @ jnxd: As far as I could figure it out the FirstPos and SecondPos values are used to decide where to draw the angle symbol between the two lines. > and is there a way to ensure the transferConstraints doesn't change angle constraints unless perhaps there is a line segment parallel to the deleted one. I don't know if this method should be changed or better delGeometry(). |
|
|
> I don't know if this method should be changed or better delGeometry(). I would also suggest referring to the origin/root with GeoId 0, but it is also sensible to view it as a point on the x/horizontal axis. Also, I do not know if it is even possible since it could well be saved in the current format within FCStd files. Perhaps it would also be a good idea to view each point as a separate entity every time, so transferConstraints() becomes unnecessary. |
|
|
> I would also suggest referring to the origin/root with GeoId 0 That would be an evil thing because it will break all existing project files. I think I will add a new method that deletes at least angle and point-on-curve constraints if one of the associated geometries gets deleted. > Perhaps it would also be a good idea to view each point as a separate entity every time, so transferConstraints() becomes unnecessary. Sorry but can you explain your idea in more details, please? |
|
|
The first idea is purely cosmetic, but I guess it would be better to refer to the origin with a separate Id than the hor. axis (currently both are -1), and the most natural choice is 0. Any new element would then have to be referred to with and Id starting from 1. For the second idea, every time an element A is created, create separate point elements for each new point created (currently there can be up to three new points, B, C, D, one for each pos of an arc or such elements). Let us call them nodes to differentiate them from the elements' pos points. Any point-based constraint, like distance or coincident, uses these nodes. There are perhaps additional constraints for coincidence between the pos points of A and B, C, D. Whenever a point is clicked on, the node is selected. When a coincident constraint is added between separate nodes, one of them is deleted and all constraints move to the surviving one. When a coincidence constraint between two elements is deleted, a new node is created, but we do not allow deletion of the constraint if it leaves the node without any geometry on it. If an element is deleted, any node on it not also coincident on a point of another element is also deleted. Since transferConstraint() is used in delGeometry() to move the constraints from the point to be deleted (since the element it was going to be referred to is deleted), the new method renders it unnecessary since the constraints would already be on the nodes. The deleteConstraintsonPoint() function also reduces to removing precisely one coincidence constraint between the node and the point. |
|
|
> The first idea is purely cosmetic ... In FreeCAD the geo id of an external geometry is always negative. The axes and the root point are also considered as external geometry and thus I think their geo ids should also be negative to be consistent. However, as said before this will invalidate any existing projects where one of these geometries is used because the geo id is written into the project file. Maybe it's best to discuss this topic further in the development forum. |
|
|
> Maybe it's best to discuss this topic further in the development forum. posted to http://forum.freecadweb.org/viewtopic.php?f=10&t=19814 Edit: previous thread I just found https://forum.freecadweb.org/viewtopic.php?f=10&t=18117 |
|
|
FreeCAD-incorrect-angle-sm.jpg (99,315 bytes) |
|
|
https://github.com/FreeCAD/FreeCAD/pull/768 |
|
|
https://github.com/FreeCAD/FreeCAD/commit/1a33f3bf9913ca23effdb1d29bad900d638be0a5 |
- Anonymous
