Save All fails if a Component does not have a filename (e.g. newly reconstructed mesh)
About you
CamiTK developer
Overview
SaveAll implementation is wrong and fails in multiple ways when components to save do not have a filename. This can lead to data loss (thinking a file was saved even it was not) and segfault (when no component is selected).
Steps to Reproduce
Open two masks (e.g. skull-binary.stl and head-binary from testdata). Using action Reconstruction to create a mesh from each mask.
- Scenario 1
- Select both meshes
- Click on "Save All" from the File menu
- In the first Save As window, save as Mesh1.obj
- In the second window, save as Mesh2.obj
- Scenario 2
- Deselect all components
- Click on "Save All" from the File menu
Actual VS Expected Result
- Scenario 1
- Expected: both meshes are saved in their own file, each now has a filename set to the one that was used to save them
- Actual: only one of the meshes was saved in both files. This mesh now has a filename set to the latest file saved. The other mesh is ignored and not saved
- Scenario 2
- Expected: both meshes are saved in their own file
- Actual result: CamiTK crashes, all work is lost
Interpretation & Possible fixes
-
SaveAllAction::apply
callsApplication::save
on all top-level components. -
Application::save
checks if the filename is empty. If it is, it calls -
getAction("Save As")->apply()
.
The problem here is that the SaveAsAction::apply
function does not know which component it must save, as this is not a parameter of apply
.
It will try to save the last selected component, which is probably not the one we are trying to save.
Component* comp = Application::getSelectedComponents().last();
If there is no selected component (Scenario 2), this tries to get a Component from an empty list and crashes (qt_assert in debug mode).
If there is at least a selected component, SaveAll will save the last selected component over and over again.
To fix this Modify action "Save As" so we can set a componentToSave parameter, apply it, then reset ComponentToSave to null_ptr. Apply would then check for this parameter, revert to the current behaviour if it is empty, and check if there are selectedComponents before getting the last element.
CamiTK Version
CamiTK 5.2.0.158-camitk-file-format-metadata-scenes-processing-scenarios.c46342cd
please do not remove anything below this line