CamiTK Community Edition issueshttps://gricad-gitlab.univ-grenoble-alpes.fr/CamiTK/CamiTK/-/issues2024-03-25T17:54:17+01:00https://gricad-gitlab.univ-grenoble-alpes.fr/CamiTK/CamiTK/-/issues/151Picking a point changes selected object and makes current Action disappear2024-03-25T17:54:17+01:00Manik BhattacharjeePicking a point changes selected object and makes current Action disappear## About you
CamiTK developper
## Overview
While creating a CEP, an action that uses multiple components was written and needs to get point coordinates from a click in a viewer.
When clicking on the point, if the image in the viewer is...## About you
CamiTK developper
## Overview
While creating a CEP, an action that uses multiple components was written and needs to get point coordinates from a click in a viewer.
When clicking on the point, if the image in the viewer is not the selected imageComponent, the picking action changes the selection. The action is then hidden because the selection changed.
## Steps to Reproduce
This is visible when using Meniscare CamiTK CEP and the ManualRegistration3D action.
Load two volumes (e.g. brain.mha and sinus-displaced.mha).
In the action, select one of the images, and Ctrl+Click on the image in a slice viewer.
Click on the "pick" button in the action to get the coordinates.
## Actual VS Expected Result
If the image shown in the viewer (the one "selected" in the Action widget) is not the selected component of CamiTK Application, the image is selected, and the action disappears because the component selection changed.
## Interpretation & Possible fixes
This is due to the [selectionChanged(comp)](https://gricad-gitlab.univ-grenoble-alpes.fr/CamiTK/CamiTK/-/blob/develop/sdk/libraries/core/viewer/InteractiveViewer.cpp#L2165) in InteractiveViewer. The same applies to other picking modes in the same InteractiveViewer::picked() function.
I think this is done to show in the PropertyExplorer the coordinates of the picked point when it is clicked (which needs the ImageComponent to be selected), and hiding the action is a side effect of that.
I see two ways to fix this problem:
- do not change the selected component when a point is picked, but that might break the behavior of displaying the picked coordinates in the "selection" tab of the PropertyExplorer
- A large change in CamiTK: separate completely the notion of selected components in the Application with the currently running Action. I mean that when an action is open on selected components, the fact that the selection changes in the application should not hide the action. The action should only be closed if the component disappears (closed) or the action is closed by the user. This means rethinking what an action can be, e.g. an action can be instantiated multiple times, and there should be an ActionExplorer that lists currently opened actions. In this way, we could run the same action twice with different parameters at the same time.
## CamiTK Version
CamiTK 5.1.dev.develop.f3f104d8
---
**please do not remove anything below this line**https://gricad-gitlab.univ-grenoble-alpes.fr/CamiTK/CamiTK/-/issues/137Unix file separator "/" is used explicitely in some file handling code of the...2023-05-31T12:15:49+02:00Manik BhattacharjeeUnix file separator "/" is used explicitely in some file handling code of the SDK instead of a portable value such as QDir::separator()## About you
CamiTK developper
## Overview
While reading code in libraries/core/viewer/RendererWidget.cpp I found a line of code which assumes that file path are using the unix separator '/'.
```c++
QString filePrefix = QFileInfo(filen...## About you
CamiTK developper
## Overview
While reading code in libraries/core/viewer/RendererWidget.cpp I found a line of code which assumes that file path are using the unix separator '/'.
```c++
QString filePrefix = QFileInfo(filename).absolutePath() + "/" + QFileInfo(filename).baseName();
```
The same type of separator use is found at multiple places in the SDK:
```bash
$ grep -n --include "*.cpp" --include "*.h" '"/"' * -R
```
```c++
actions/application/file/SaveAsAction.cpp:178: QFileDialog saveFileDialog(nullptr, tr("Save File As..."), QFileInfo(compfileName).dir().canonicalPath() + "/" + suggestedName, test);
actions/application/file/SaveAsAction.cpp:183: QString filename = QFileDialog::getSaveFileName(nullptr, tr("Save File As..."), QFileInfo(compfileName).dir().canonicalPath() + "/" + suggestedName, test);
applications/actionstatemachine/SaveActionState.cpp:107: (*outIt)->setFileName(saveDirName + "/" + compName + ".mha");
applications/actionstatemachine/SaveActionState.cpp:109: (*compIt)->setFileName(compDir + "/" + compName + compExt);
applications/actionstatemachine/SaveActionState.cpp:114: (*logStream) << compDir + "/" + compName + compExt << "' type='" << compType << "'/>" << Qt::endl;
applications/testcomponents/main.cpp:190: comp->setFileName(outputDirectory + "/" + inputComponent.fileName());
applications/wizard/GeneratingCEPState.cpp:105: QString outputFileName = dir.absolutePath() + "/" + cepDirName + "/libraries/" + libraryName + "/" + shortFileName;
components/vtkimage/VtkImageComponentExtension.cpp:251: QString filePattern = fileInfo.absoluteDir().absolutePath() + "/" + fileInfo.baseName();
components/off/OffExtension.cpp:71: QString baseFilename = QFileInfo(component->getFileName()).absolutePath() + "/" + QFileInfo(component->getFileName()).completeBaseName();
components/obj/ObjExtension.cpp:68: QString baseFilename = QFileInfo(component->getFileName()).absolutePath() + "/" + QFileInfo(component->getFileName()).completeBaseName();
libraries/core/viewer/RendererWidget.cpp:572: QString filePrefix = QFileInfo(filename).absolutePath() + "/" + QFileInfo(filename).baseName();
libraries/core/utils/CamiTKLogger.cpp:158: QFile::rename(fileToMove.absoluteFilePath(), logFileDirectory.path() + "/" + logFileName);
libraries/core/utils/CamiTKLogger.cpp:162: logFile = new QFile(logFileDirectory.path() + "/" + logFileName);
libraries/core/ExtensionManager.cpp:322: QString privateLibToLoad = Core::getGlobalInstallDir() + "/" + QString(Core::libDir) + "/" + QString(Core::shortVersion) + "/" + libname.cap(1);
libraries/core/ExtensionManager.cpp:328: privateLibToLoad = Core::getGlobalInstallDir() + "/" + QString(Core::libDir) + "/" + QString(Core::shortVersion) + "/" + libname.cap(1);
libraries/core/ExtensionManager.cpp:367: // add one cdUp() for each "/" inside Core::libDir (to take into account any multiarch configuration that
libraries/core/ExtensionManager.cpp:576: appFilePath.replace("/", "\\");
libraries/core/ExtensionManager.cpp:717: if (potentialPath.cd(potentialLibDir + "/" + QString(Core::shortVersion))) {
libraries/core/Core.cpp:314: QStringList installDirectories = getInstallDirectories(QString(Core::libDir) + "/" + QString(Core::shortVersion) + "/" + extensionType);
libraries/core/Core.cpp:318: QStringList libInstallDirectories = getInstallDirectories("lib/" + QString(Core::shortVersion) + "/" + extensionType, false);
libraries/cepgenerator/CepGenerator.cpp:95: devDirectoryName = devDirectoryName + "/";
libraries/cepgenerator/ComponentExtensionGenerator.cpp:309: QString testFileName = currentDirectory.absolutePath() + "/" + "empty." + suffix;
```
Same with '/':
```cpp
libraries/core/ExtensionManager.cpp:369: for (int i = 0; i < QString(Core::libDir).count(QLatin1Char('/')); i++) {
components/vtkmesh/VtkMeshUtil.cpp:230: size_t nbegin = vtkFileName.find_last_of('/');
```
Outside the SDK, tutorials and modeling also contain instances of the same problem:
```cpp
modeling/libraries/mml/monitoring/MonitoringManager.cpp:565: if (mmlIn->simulatorFile().get()[0] != '/') {
modeling/libraries/mml/monitoring/AnsysSimulator.cpp:48: if (workingDir[workingDir.length() - 1] != '/') {
modeling/libraries/mml/monitoring/ArtiSynthSimulator.cpp:44: if (workingDir[workingDir.length() - 1] != '/') {
modeling/applications/pmltools/obj2pml/obj2pml.cpp:111: unsigned int slash = buff.find('/');
modeling/actions/mml/GenerateModel.cpp:86: QString baseFilename = QFileInfo(originalFilename).absolutePath() + "/" + QFileInfo(originalFilename).completeBaseName();
modeling/libraries/mml/monitoring/MonitoringManager.cpp:53: size_t slashPlace = mmlPath.find_last_of("/");
modeling/libraries/mml/monitoring/MonitoringManager.cpp:97: QString guessPMLFilename = QFileInfo(fi).absolutePath() + "/" + QFileInfo(fi).baseName() + ".pml";
modeling/libraries/mml/monitoring/SofaSimulator.cpp:80: QString scnFileName = mmlFileInfo.absolutePath() + "/" + mmlFileInfo.baseName() + ".scn";
modeling/libraries/mml/monitoring/SofaSimulator.cpp:88: scnFileName = QDir::tempPath() + "/" + mmlFileInfo.baseName() + ".scn";
modeling/libraries/mml/monitoring/SofaSimulator.cpp:92: QString mshFileName = QDir::tempPath() + "/" + QFileInfo(monitoringManager->getPmlFileName().c_str()).baseName() + ".msh";
modeling/libraries/mml/monitoring/SofaWidget.cpp:87: size_t slashPlace = scnFileTemp.find_last_of("/");
modeling/libraries/mml/monitoring/AnsysSimulator.cpp:49: workingDir = workingDir + "/";
modeling/libraries/mml/monitoring/ArtiSynthSimulator.cpp:45: workingDir = workingDir + "/";
modeling/libraries/mml/monitoring/Reference.cpp:82: size_t slashPlace = refpath.find_last_of("/");
modeling/components/mmlcomponent/MMLComponent.cpp:61: file = QDir::tempPath() + "/" + QFileInfo(fi).baseName() + ".mml";
modeling/applications/pmltools/extractCells/extractCells.cpp:147: extracted = new StructuralComponent(pm, c1 + "/" + c2 + " extracted");
modeling/applications/pmltools/extractCells/extractCells.cpp:172: cout << '\r' << (i + 1) << "/" << ok;
modeling/applications/pmltools/extractCells/extractCells.cpp:192: pm->setName(pm->getName() + " " + c1 + "/" + c2 + " Extracted");
```
```cpp
tutorials/components/mixed/MixedComponent.cpp:41: QString mhaFile = QFileInfo(file).absolutePath() + "/" + filename.c_str();
tutorials/components/mixed/MixedComponent.cpp:43: QString vtkFile = QFileInfo(file).absolutePath() + "/" + filename.c_str();
tutorials/applications/fancy/FancyMainWindow.cpp:221: ui.slideValue->setText(QString("%1").arg((currentSliceId + 1), 3) + "/" + QString("%1").arg(maxSliceId, 3));
tutorials/applications/nogui/main.cpp:85: QString outputFileName = QDir::currentPath() + "/" + QFileInfo(image->getFileName()).completeBaseName() + "-output." + QFileInfo(image->getFileName()).suffix(
```
## Actual VS Expected Result
As CamiTK is multiplaform, the code should not assume that the path separator is the '/' char.
Using Qt Resources might be an exception to this, so each line of code should be checked manually before replacing '/' by QDir::separator().
## Interpretation & Possible fixes
Replacing '/' char with QDir::separator() when building file paths.
## CamiTK Version
CamiTK 5.1.dev.develop.f3f104d8
---
**please do not remove anything below this line**Manik BhattacharjeeManik Bhattacharjee