Fix build against Slicer 5.x and Qt 5.15 #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…MacroBuildQtModule
jcfr
changed the title
This commit introduces Slicer_CAMERA_PATH_EXPORT_VIDEO_SUPPORT macro to conditionally build FFMPEG export.
…Copy API Follow up Slicer/Slicer@f88c11043 (ENH: Refactor MRML node API to support shallow and deep copy) switching to use of MRMLNodeModifyBlocker
…aces in indentation"
This commit fixes the following error: SyntaxError: Missing parentheses in call to 'print'
This commit fixes the following runtime error: Failed to extract plugin meta data from '/path/to/lib/Slicer-5.1/qt-loadable-modules/libqSlicerCameraPathModule.so' References: * https://www.slicer.org/wiki/Documentation/Nightly/Developers/Tutorials/MigrationGuide#Qt5:_Update_loadable_modules_to_use_new_plugin_macros * https://wiki.qt.io/Transition_from_Qt_4.x_to_Qt5#Plugin_loading
jcfr
force-pushed
the
fix-build-against-slicer-5.x-and-qt-5.15
branch
from
3a2c8d8 to
5feaad9
Compare
This commit fixes warning like the following:
In file included from /path/to/Modules/Loadable/CameraPath/MRML/vtkMRMLCameraPathNodePython.cxx:11:
/tmp/Slicer-CameraPath/Modules/Loadable/CameraPath/MRML/vtkMRMLCameraPathNode.h:30:13: note: because ‘KeyFrame’ has user-provided ‘KeyFrame& KeyFrame::operator=(const KeyFrame&)’
30 | KeyFrame& operator = (const KeyFrame& a)
| ^~~~~~~~
/tmp/Slicer-CameraPath/Modules/Loadable/CameraPath/MRML/vtkMRMLCameraPathNode.h:106:46: note: initializing argument 2 of ‘void vtkMRMLCameraPathNode::SetKeyFrame(vtkIdType, KeyFrame)’
106 | void SetKeyFrame(vtkIdType index, KeyFrame keyFrame);
| ~~~~~~~~~^~~~~~~~
/path/to/Modules/Loadable/CameraPath/MRML/vtkMRMLCameraPathNodePython.cxx: In function ‘PyObject* PyvtkMRMLCameraPathNode_AddKeyFrame_s1(PyObject*, PyObject*)’:
/path/to/Modules/Loadable/CameraPath/MRML/vtkMRMLCameraPathNodePython.cxx:849:29: warning: implicitly-declared ‘KeyFrame::KeyFrame(const KeyFrame&)’ is deprecated [-Wdeprecated-copy]
849 | op->AddKeyFrame(*temp0);
| ^
This commit converts the usage of null pointer constants (eg. NULL, 0) to use the new C++11 nullptr keyword. Updates were performed following these steps: (1) Reconfigure with CMAKE_EXPORT_COMPILE_COMMANDS cd /tmp/Slicer-CameraPath- cmake -DCMAKE_EXPORT_COMPILE_COMMANDS:BOOL=ON . (2) Run run-clang-tidy cd /tmp/Slicer-CameraPath-build run-clang-tidy-10 -header-filter='.*' -checks='-*,modernize-use-nullptr' -fix References: * https://www.slicer.org/wiki/Documentation/Nightly/Developers/Tutorials/MigrationGuide#C.2B.2B11:_Update_source_code_to_use_nullptr
This commit fixes the following warning:
/path/to/Modules/Loadable/CameraPath/MRML/vtkMRMLCameraPathNode.cxx: In member function ‘void vtkMRMLCameraPathNode::SetKeyFrame(vtkIdType, KeyFrame)’:
/path/to/Modules/Loadable/CameraPath/MRML/vtkMRMLCameraPathNode.cxx:252:58: warning: the address of ‘time_t time(time_t*)’ will never be NULL [-Waddress]
252 | std::cerr << "A keyframe already exists for t = " << time << std::endl
| ^~~~
…butes This commit leverage copy API introduced in Slicer/Slicer@f88c11043 (ENH: Refactor MRML node API to support shallow and deep copy).
Since following Slicer/Slicer@e78f557cd (ENH: Simplify view widgets initialization from view nodes), the camera node is managed as a singleton (the singleton tag corresponds to the layout name), this commit ensures camera nodes associated with other views are not clobbered by systematically instantiating a new camera node that is (1) not added to the scene and (2) only used as convenience inside vtkMRMLCameraPathNode.
|
Create a camera path and save the MRML scene. Loading of the MRML scene causes the following error during loading: Don't know if you want to fix this right now. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request allows to build and run the module using Slicer 5.x.
Support for building against older Slicer version has been removed.
Summary
Following refactoring of the view infrastructure in Slicer (Slicer/Slicer@e78f557cd), camera nodes are associated with a view using a singleton tag 1. This means that the approach that was used in the CameraPath module had to be reviewed to avoid adding a new camera node to the scene for each key frame.
Next steps
Revisit structure used internally to store key frames: One possibility would be to use a plane markup. Since the plane markup already provides translation & rotation, this could be a sensible proxy to edit a given key frame using the mouse (or even virtual reality controller)
Refactor video export leveraging "Screen capture" module. Since the "Screen capture" module allows recording sequences, a given camera path could be exported to a sequence.
File format
Camera paths may be saved as CSV (
*.kcsv) (see below for an example).This could be revisited to be a json based format along with a schema.
Footnotes
https://apidocs.slicer.org/v5.0/classvtkMRMLCameraNode.html#a4d8d55c482e850c08a84ffceac8e4830 ↩