New Markups functionality retains unset control point positions

Development
11

Actually, the undo functionality is already available in Slicer. You can enable undo/redo for fiducial nodes by copy-pasting these lines into the Python console:

# Enable undo for the scene

slicer.mrmlScene.SetUndoOn()

# Enable undo for markups fiducial nodes

defaultMarkupsNode = slicer.mrmlScene.GetDefaultNodeByClass("vtkMRMLMarkupsFiducialNode")
if not defaultMarkupsNode:
    defaultMarkupsNode = slicer.vtkMRMLMarkupsFiducialNode()
    slicer.mrmlScene.AddDefaultNode(defaultMarkupsNode)

defaultMarkupsNode.UndoEnabledOn()

# Add standard keyboard shortcuts for scene undo/redo

redoKeyBindings = qt.QKeySequence.keyBindings(qt.QKeySequence.Redo)
for redoBinding in redoKeyBindings:
    redoShortcut = qt.QShortcut(slicer.util.mainWindow())
    redoShortcut.setKey(redoBinding)
    redoShortcut.connect("activated()", slicer.mrmlScene.Redo)

undoKeyBindings = qt.QKeySequence.keyBindings(qt.QKeySequence.Undo)
for undoBinding in undoKeyBindings:
    undoShortcut = qt.QShortcut(slicer.util.mainWindow())
    undoShortcut.setKey(undoBinding)
    undoShortcut.connect("activated()", slicer.mrmlScene.Undo)

It is not enabled by default because historically it was attempted to be enabled for all nodes by default, which was too complicated. Since undo was disabled, developers did not pay attention to properly calling markupsNode->SaveStateForUndo() before all user actions, all those need to be added (without that, an undo reverts multiple user interactions), may be duplicated at a few places (requiring hitting undo/redo shortcuts multiple times to get a visible change), and of course some display update issues or other small bugs may be uncovered. Without these fixes, undo/redo will not feel very robust, but these should be all fairly easy to address.

If you want to further discuss of undo/redo then please create a new topic.

Both mask based (separate column specifies if an entry is missing or not) and special placeholder value based approaches are commonly used for dealing with missing values.

Slicer’s table infrastructure already supports both techniques. In table properties, the user can set any placeholder value as null value (0, -1, 1e6, nan, inf, …). This placeholder value is stored in the table schema.

For example, table exported from markups fiducial node:

image

Written to csv file (table.csv):

"label","r","a","s","defined","selected","visible","locked","description"
"F_1-1",-17.3607,-30.2059,-10.2143,1,1,1,0,""
"F_1-2",-12.0642,5.98722,-10.2143,1,1,1,0,""
"F_1-3",-67.678,6.9286,7.38225,0,1,1,0,""
"F_1-4",83.4364,-7.27105,-175.25,0,1,1,0,""
"F_1-5",-111.564,11.9882,-175.25,1,1,1,0,""

Schema file (Table.schema.csv) written out along with the table file:

"columnName","nullValue","type","componentNames"
"label","","string",""
"r","nan","double",""
"a","nan","double",""
"s","nan","double",""
"defined","","bit",""
"selected","","bit",""
"visible","","bit",""
"locked","","bit",""
"description","","string",""

Currently, markups module’s “Export/import table” feature uses only the masking technique (via a separate “defined” Boolean column), but we could add an option to replace point coordinates of undefined points with a null placeholder value.

This refactoring would be a good opportunity to switch to standard csv format from the limited and broken fcsv format (multi-line header, hardcoded columns, no way to specify column type, null and default values, etc.).

1 Like