[BUG] Cancel does not work with Datum Planes
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
Be nice to others! Respect the FreeCAD code of conduct!
[BUG] Cancel does not work with Datum Planes
OS: Windows 10 (10.0)
Word size of OS: 64-bit
Word size of FreeCAD: 64-bit
Version: 0.19.24291 (Git)
Build type: Release
Branch: (HEAD detached at 0.19.2)
Hash: 7b5e18a0759de778b74d3a5c17eba9cb815035ac
Python version: 3.8.8
Qt version: 5.12.9
Coin version: 4.0.0
OCC version: 7.4.0
Locale: German/Germany (de_DE)
when i click cancel the datum plane is created..
Word size of OS: 64-bit
Word size of FreeCAD: 64-bit
Version: 0.19.24291 (Git)
Build type: Release
Branch: (HEAD detached at 0.19.2)
Hash: 7b5e18a0759de778b74d3a5c17eba9cb815035ac
Python version: 3.8.8
Qt version: 5.12.9
Coin version: 4.0.0
OCC version: 7.4.0
Locale: German/Germany (de_DE)
when i click cancel the datum plane is created..
Re: [BUG] Cancel does not work with Datum Planes
It is a bit sophisticated, but it seems that creating and editing are two things. So the DatumPlane is created and then you enter immediately editing mode. You can cancel the latter, and no editing will be done, which means in your case that the DatumPlane remains attached to the face which was selected when creating the DatumPlane.
You can after canceling use undo to make the creation undone as well.
You can after canceling use undo to make the creation undone as well.
A Sketcher Lecture with in-depth information is available in English, auf Deutsch, en français, en español.
Re: [BUG] Cancel does not work with Datum Planes
It is particularly odd to see this behavior only for datum objects. I observed this too (mistakenly re-reported here) and took a look at the source a couple days ago. From what I can tell, the setEdit() command is also used for the solid features like pad or pocket, but they are destroyed when the edit is canceled...chrisb wrote: ↑Mon May 03, 2021 6:01 pm It is a bit sophisticated, but it seems that creating and editing are two things. So the DatunmPlane is created and then you enter immediately editing mode. You can cancel the latter, and no editing will be done, which means in your case that the DatumPlane remains attached to the face which was selected when creating the DatumPlane.
You can after canceling use undo to make the creation undone as well.
My latest (or last) project: B-spline Construction Project.
Re: [BUG] Cancel does not work with Datum Planes
I had this tab opened for a pretty long time.
I had a quick look now and absolutely can't find what commits the undo transaction in the case of the Datum objects but not in the case of a Pad/Pocket/...
Will try to have another look later on.
I had a quick look now and absolutely can't find what commits the undo transaction in the case of the Datum objects but not in the case of a Pad/Pocket/...
Will try to have another look later on.
Re: [BUG] Cancel does not work with Datum Planes
Recently used Datum planes for a model and came across this issue.
My understanding after reviewing the code is there are two commands being added to the undo list when a DatumPlane is created:
- Create DatumPlane
- Edit Attachment
When the user presses the Cancel button (linked to the reject method in the code) only the currently active command "Edit Attachment" is aborted.
The "Create DatumPlane" had already been committed so it is left untouched, thus the object remains in the tree.
As chrisb suggests a manual undo on the GUI will then remove the DatumPlane object.
Note: this applies to any of the Datum objects not just planes.
To address this an additional undo command can be used to clean up the object iff the parameter editor was opened when creating a new object vs editing an existing object .
I have created a branch that demonstrates this does fix the issue.
Not sure if it's the right approach thus haven't submitted a PR yet.
Rather I suspect it would be better to redirect the context menu / double click events to the UnifiedDatumCommand() method that distinguishes between the create and edit cases. (eg "Edit DatumPlane" vs "Create DatumPlane" undo entry).
Have a look at the branch, if there is general agreement that it's acceptable as is I will clean it up and submit a PR.
Otherwise I can look at using UnifiedDatumCommand when I get some more time.
My understanding after reviewing the code is there are two commands being added to the undo list when a DatumPlane is created:
- Create DatumPlane
- Edit Attachment
When the user presses the Cancel button (linked to the reject method in the code) only the currently active command "Edit Attachment" is aborted.
The "Create DatumPlane" had already been committed so it is left untouched, thus the object remains in the tree.
As chrisb suggests a manual undo on the GUI will then remove the DatumPlane object.
Note: this applies to any of the Datum objects not just planes.
To address this an additional undo command can be used to clean up the object iff the parameter editor was opened when creating a new object vs editing an existing object .
I have created a branch that demonstrates this does fix the issue.
Not sure if it's the right approach thus haven't submitted a PR yet.
Rather I suspect it would be better to redirect the context menu / double click events to the UnifiedDatumCommand() method that distinguishes between the create and edit cases. (eg "Edit DatumPlane" vs "Create DatumPlane" undo entry).
Have a look at the branch, if there is general agreement that it's acceptable as is I will clean it up and submit a PR.
Otherwise I can look at using UnifiedDatumCommand when I get some more time.
Re: [BUG] Cancel does not work with Datum Planes
There is a general agreement (with myself ) that your branch looks like an horrible dirty patch.
We discussed a global solution already on this thread : https://forum.freecadweb.org/viewtopic.php?f=10&t=62063
I'm currently listing the impacted classes before doing a draft PR : the PR will be very fast, but testing for regression everywhere (and eventually fixing) will be a huge task -- and I'll probably request for some help from the community --.
As would say GitHub, "You can safely delete your branch"
Re: [BUG] Cancel does not work with Datum Planes
For the record the now deleted branch was definitely a "horrible dirty patch" intended to get the discussion going again.
At least it wasn't a complete waste of time as there is a link to the PR and other discussion thread now.
At least it wasn't a complete waste of time as there is a link to the PR and other discussion thread now.
Re: [BUG] Cancel does not work with Datum Planes
Sure, and maybe you'll be interested in testing regressions in the coming PR.
Just so you see amount of work, all classes in the picture below linked to the right of ViewProviderGeometryObject (grey box) shall be checked for regression (and eventually fixed).