All Projects FreeCAD  Addon Manager  Arch  Assembly  Draft  Expressions  FEM  File formats  GCS  OpenSCAD  Part  PartDesign  Path  Project Tools & Websites  Raytracing  Robot  Sketcher  Spreadsheet  TechDraw

View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0004265FreeCADBugpublic2020-02-15 10:382021-02-06 06:49
Reporterwmayer Assigned Torealthunder  
PrioritynormalSeveritymajorReproducibilityalways
Status newResolutionreopened 
Target Version0.20 
Summary0004265: Undo doesn't restore the state as it was before opening a transaction
DescriptionWhen opening a transaction and adding an arbitrary amount of objects to a document before committing it then the undo should always remove these objects again.

In v0.19 this doesn't work any more.
Steps To Reproduce
  1. Go to Part Design
  2. Create a document
  3. Create a sketch, select one of the predefined planes and confirm with OK
  4. Undo now or close the sketch and then undo

=> The body and origin will be removed from the document but the sketch stays

It's possible to edit the sketch (some error messages appear) and as soon as closing the sketch after doing some changes FreeCAD crashes.
Tags#pending-PR, undo
FreeCAD InformationOS: Ubuntu 18.04.4 LTS (XFCE/xubuntu)
Word size of OS: 64-bit
Word size of FreeCAD: 64-bit
Version: 0.19.19593 (Git)
Build type: Unknown
Branch: master
Hash: a10a0d9e0f7f3783ee6728e4df60050aba4306ee
Python version: 3.6.9
Qt version: 5.9.5
Coin version: 4.0.0a
OCC version: 7.3.0
Locale: German/Germany (de_DE)

Activities

wmayer

2020-02-23 11:12

administrator   ~0014177

Last edited: 2020-02-23 11:13

 Another regression:
  1. Create a PD box
  2. Select some edges and create a fillet
  3. Now add or remove an edge. This causes to open a second transaction
  4. Click cancel. The expected behaviour is that the fillet object will disappear but because of the 2nd transaction it won't

wmayer

2020-03-14 10:01

administrator   ~0014227

Another test case:

doc=App.newDocument()
doc.openTransaction("Test")
doc.commitTransaction()
App.getActiveTransaction() # does not return None but ('None', <int>)

wmayer

2020-03-14 10:31

administrator   ~0014228

With v0.18

doc1=App.newDocument()
doc2=App.newDocument()
doc1.UndoNames # => [] ok
doc2.UndoNames # => [] ok
doc1.openTransaction("Test1")
doc1.UndoNames # => ['Test1'] ok
doc2.openTransaction("Test2")
doc2.UndoNames # => ['Test2'] ok
doc1.addObject("App::FeaturePython")
doc1.UndoNames # => ['Test1'] ok
doc1.commitTransaction()
doc2.commitTransaction()

With v0.19

doc1=App.newDocument()
doc2=App.newDocument()
doc1.UndoNames # => [] ok
doc2.UndoNames # => [] ok
doc1.openTransaction("Test1")
doc1.UndoNames # => [] now an empty list. Can be considered as ok when no actual change was made
doc2.openTransaction("Test2")
doc2.UndoNames # same as above
doc1.addObject("App::FeaturePython")
doc1.UndoNames # => ['Test2'] but expected is ['Test1']. So this is clearly a bug
doc2.UndoNames # => ['-> Test2'] but expected is ['Test2']. So this is clearly a bug
doc1.commitTransaction()
doc2.commitTransaction()

Storing the transaction name and id independent of a document messes up the transaction system.

wmayer

2020-03-14 14:19

administrator   ~0014229

This https://github.com/FreeCAD/FreeCAD/commit/69ea7a3d51d653906d35a86203ce197cf22f73a5 fixes the issue with the remaining sketch on undo.
And this https://github.com/FreeCAD/FreeCAD/commit/8c0dc924269ece2490378889da12636767cd6f9f fixes the disappearance of the fillet on undo

So, what still needs to be fixed is the mess with the active transaction name which is only a medium severe issue.

realthunder

2020-03-22 22:21

developer   ~0014265

Fix committed to master branch.

wmayer

2020-03-23 14:36

administrator   ~0014270

Last edited: 2020-03-23 14:37

 The reported mess with the transaction name is not fixed!!!

Another regression is that GUI changes are not recorded any more:
In v0.18:

doc=App.newDocument()
doc.openTransaction("Create box")
box=doc.addObject("Part::Box","Box")
doc.commitTransaction()
doc.recompute()

doc.openTransaction("Change GUI")
box.ViewObject.ShapeColor=(1.,1.,0.)
doc.commitTransaction()
doc.undo() # restores the previous color

In v0.19:

doc=App.newDocument()
doc.openTransaction("Create box")
box=doc.addObject("Part::Box","Box")
doc.commitTransaction()
doc.recompute()

doc.openTransaction("Change GUI")
box.ViewObject.ShapeColor=(1.,1.,0.)
doc.commitTransaction()
doc.undo() # instead of restoring the color it removes the box

wmayer

2020-03-23 16:08

administrator   ~0014271

Another odd regression compared to v.018:

  • Create a part container, a box and move the box into the container
  • Make the origin visible
  • Select the box and in the property editor open the map mode
  • Select the XZ plane as reference => box will move
  • Confirm with OK
  • Press undo & redo 5 times => box will move
  • Press undo again => box stops moving

In v0.18 you can press undo & redo as often as you like and the box is always updated correctly.

realthunder

2020-03-24 03:18

developer   ~0014273

PR submitted to enable transaction on view object. Can't remember exactly, but I think previously view object changes are not recorded?

The Box movement undo problem is an entirely different issue. It happens because the property changes recorded in transaction is unordered, and AttachEngine will recalculate positioning regardless if it is performing transaction.

The non movement happens in undo if Placement is restored first, and then some attachment related property is restored next, which triggers AttachEngine positionBySupport(). Because MapMode and Support have not been restored yet, the placement is revert back, which nullifies the undo.

The reason this problem does not shown up before is because std::map was used to store property changes of each object, with the pointer to the property as the key. When the application starts up, the memory pointer value is likely increasing. So the order in std::map is temporary stable, but not for long. I have changed map to unordered_map, thus exposing the problem.

I propose to fix this by blocking PropertyContainer::onChange() call temporary while undo/redo, i.e. inside TransactionObject::applyChn(). Once all changes are applied for the current transaction, call Property::touch() for each changed property. Any thoughts?

realthunder

2020-03-24 09:34

developer   ~0014274

See this PR for the proposed fix. Class TransactionGuard is used to temporary disable onChanged() notification, and call touch() when goes out of scope.

This fix exposes another potential problem in PartFeature::onChange(Placement), as it modifies the Shape without recording it in the transaction.

realthunder

2020-03-25 00:50

developer   ~0014287

There is some mistake in the TransactionGuard PR. Fixed by force push. Also split the Part Placement/Shape onChanged() problem into a separate commit.

uwestoehr

2021-01-12 00:53

manager   ~0015205

What's the status of this bugfix? People treat it as release blocker: https://forum.freecadweb.org/viewtopic.php?f=8&t=51016 but we have a PR as fix that was also approved by yorikvanhavre on Github. Why can/is this not merged?

yorik

2022-03-03 13:55

administrator   ~0016865

This ticket has been migrated to GitHub as issue 6031.

Related Changesets

FreeCAD: master 2b16d060

2020-02-23 23:59:19

realthunder


Committer: wmayer Details Diff 		Gui: disable auto transaction on showing task dialog

Fixes 0004265		 Affected Issues
0004265
mod - src/Gui/Control.cpp Diff File

Issue History

Date Modified Username Field Change
2020-02-15 10:38 wmayer New Issue
2020-02-20 21:03 Kunda1 Tag Attached: undo
2020-02-20 21:04 Kunda1 Target Version => 0.19
2020-02-23 11:12 wmayer Note Added: 0014177
2020-02-23 11:13 wmayer Note Edited: 0014177
2020-03-14 10:01 wmayer Note Added: 0014227
2020-03-14 10:31 wmayer Note Added: 0014228
2020-03-14 14:19 wmayer Note Added: 0014229
2020-03-14 14:20 wmayer Priority high => normal
2020-03-14 14:20 wmayer Severity crash => major
2020-03-22 22:21 wmayer Changeset attached => FreeCAD master 2b16d060
2020-03-22 22:21 realthunder Note Added: 0014265
2020-03-22 22:21 realthunder Assigned To => realthunder
2020-03-22 22:21 realthunder Status new => closed
2020-03-22 22:21 realthunder Resolution open => fixed
2020-03-23 14:36 wmayer Note Added: 0014270
2020-03-23 14:36 wmayer Status closed => new
2020-03-23 14:36 wmayer Resolution fixed => reopened
2020-03-23 14:37 wmayer Note Edited: 0014270
2020-03-23 16:08 wmayer Note Added: 0014271
2020-03-24 03:18 realthunder Note Added: 0014273
2020-03-24 09:34 realthunder Note Added: 0014274
2020-03-25 00:50 realthunder Note Added: 0014287
2020-10-17 10:26 Kunda1 Tag Attached: #pending-PR
2021-01-12 00:53 uwestoehr Note Added: 0015205
2021-02-06 06:49 abdullah Target Version => 0.20