Problems with module name reuse

[moritz-h]

Describe the bug

Multiple problems with the attached project:

• The project graph is different depending if the project is loaded directly with MegaMol or loaded only in the configurator. Probably there are multiple project loading code paths in MegaMol and in addition they behave differently
• There are two modules with the name VolumetricDataSource_1, one in the group and one outside the group
  • Deleting the module in the group deletes both, MegaMol prints a memory warning
  • Deleting the module outside the group triggers a Visual Studio Debug Assert

Obviously, there is a problem if a name is reused inside and outside of a group at the same time. If names must be unique over several groups, this is at least something already the loader should check and throw an exception on loading the project.

Loaded with MegaMol:

Loaded with Configurator

Additional Context

• Debug assert only visible in Visual Studio Debug build
• MegaMol version: current master
• Project for reproduction:

mmCreateView("GraphEntry_1","View3DGL","::RaycastVolumeExample::View3DGL1")

mmCreateModule("RaycastVolumeRenderer","::RaycastVolumeExample::RaycastVolumeRenderer1")
mmCreateModule("TransferFunctionGL","::RaycastVolumeExample::TransferFunction1")
mmCreateModule("VolumetricDataSource","::RaycastVolumeExample::VolumetricDataSource_1")
mmCreateModule("VolumetricDataSource","::VolumetricDataSource_1")

mmCreateCall("CallRender3DGL","::RaycastVolumeExample::View3DGL1::rendering","::RaycastVolumeExample::RaycastVolumeRenderer1::rendering")
mmCreateCall("VolumetricDataCall","::RaycastVolumeExample::RaycastVolumeRenderer1::getData","::RaycastVolumeExample::VolumetricDataSource_1::GetData")
mmCreateCall("CallGetTransferFunctionGL","::RaycastVolumeExample::RaycastVolumeRenderer1::getTransferFunction","::RaycastVolumeExample::TransferFunction1::gettransferfunction")

[geringsj]

Some context from perspective of the MegaMolGraph:

The graph creates modules with the requested ID, including group namespaces, pretty much verbatim (https://github.com/UniStuttgart-VISUS/megamol/blob/master/core/src/MegaMolGraph.cpp#L597). Currently, not even a check for duplicate names is performed. There is no inherent problem for the graph if multiple modules with the same name exist, except that lookup and graph manipulation of these modules will result in undefined behaviour (i.e., 'the user providing the broken graph is to blame'). Multiple modules with the same name will result in distinct entries in the internal std::vector holding modules.

However, since lookup of modules is implemented as sequential search in a std::vector (https://github.com/UniStuttgart-VISUS/megamol/blob/master/core/src/MegaMolGraph.cpp#L526-L530), only the first module with a given name will be found.
This is what happens when deleting modules (https://github.com/UniStuttgart-VISUS/megamol/blob/master/core/src/MegaMolGraph.cpp#L792-L794).

Deleting the module in the group deletes both

The question is: how are you deleting the module in question? Via the GUI? Using Lua? The module deletion code in the graph can only delete one module per deletion request.

If names must be unique over several groups, this is at least something already the loader should check and throw an exception on loading the project

There are different ideas what the project loader should do (load the whole project despite errors -vs.- refuse to instantiate faulty project files).
When executing a .lua project file the lua interpreter can not look ahead to check for such errors. (Neither should the interpreter surpress execution errors and simply continue executing the .lua script) Building a model of the contents of a lua project file before executing it, to catch errors like multiple modules with the same name, ends up being the same thing as instantiating the graph from the project file.

[moritz-h]

The graph creates modules with the requested ID, including group namespaces

That sounds good from a user perspective that only the combination namespace and module name must be unique and not module name only.

The question is: how are you deleting the module in question? Via the GUI? Using Lua? The module deletion code in the graph can only delete one module per deletion request.

Configurator in the GUI

the lua interpreter can not look ahead to check for such errors.

I am simply meaning something like, when executing mmCreateModule, just not create the module like if any other module creation error would happen, print an error that the module cannot be created, because the name is already in use, and then just continue. This would be way better, than creating the graph anyway, but then strange things happens on graph manipulation later on.

[braunms]

There is some related bug when deleting a module in the Configurator that will be fixed with PR #1221. But this is not the root of this problem.

FYI: When deleting a module, MegaMol breaks when trying to delete the call:

megamol/core/src/MegaMolGraph.cpp

Line 830 in 7d26f59

std::for_each(discard_calls.begin(), discard_calls.end(), [&](auto const& call_it) {