Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 69 additions & 0 deletions meshroom/core/desc/interface.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
class Interface:
"""Marker base class for node descriptor interfaces.

Inherit from this to declare a new interface. Classes that inherit from
an Interface are automatically discovered by InterfaceMeta and listed in
the ``interfaces`` attribute of the concrete node descriptor class.
"""

@staticmethod
def upstreamNodesWithInterface(node, attribute, interface_name):
result = []
visited = set()
stack = [attribute]

while stack:
attr = stack.pop()
if not attr.isLink:
continue
source_node = attr.inputRootLink.node
if source_node in visited:
continue
visited.add(source_node)
if interface_name in source_node.nodeDesc.interfaces:
result.append(source_node)
continue
for _, input_attr in source_node.attributes.items():
stack.append(input_attr)

return result

class InterfaceMeta(type):
"""Metaclass that populates the ``interfaces`` attribute on any class that uses it.

When a class is created with this metaclass, ``interfaces`` is set to the list
of ``Interface`` subclass names found in the class's MRO (excluding the class
itself and the base ``Interface`` marker).
"""
def __new__(mcs, name, bases, namespace):
cls = super().__new__(mcs, name, bases, namespace)
cls.interfaces = [
b.__name__ for b in type.mro(cls)
if b is not Interface and b is not cls
and isinstance(b, type) and issubclass(b, Interface)
]
Comment on lines +40 to +44

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

In Python, subclasses of a class that implements an interface will also return True for issubclass(b, Interface). This means that if a concrete node class (e.g., MyNode) implements FeatureProviderInterface, any subclass of MyNode (e.g., MySubNode) will mistakenly include MyNode in its interfaces list. To prevent concrete parent classes from being treated as interfaces, we should exclude classes that are instances of InterfaceMeta (the metaclass for concrete interfaced classes) by checking not isinstance(b, mcs).

Suggested change
cls.interfaces = [
b.__name__ for b in type.mro(cls)
if b is not Interface and b is not cls
and isinstance(b, type) and issubclass(b, Interface)
]
cls.interfaces = [
b.__name__ for b in type.mro(cls)
if b is not Interface and b is not cls
and isinstance(b, type) and issubclass(b, Interface)
and not isinstance(b, mcs)
]

Comment on lines +40 to +44

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rename it interfaceNames to prevent user to try code like if MyInterface in MyNode.interfaces:

return cls

class InterfacedClass(metaclass=InterfaceMeta):
"""Base class for objects that want automatic interface discovery via InterfaceMeta."""
pass
Comment on lines +47 to +49

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For clarity I would rename it HasInterfaces



class FeatureProviderInterface(Interface):

def getFeaturesFolders(self, node) -> list:
raise NotImplementedError

def getDescriberTypes(self, node) -> list:
raise NotImplementedError

class MatchProviderInterface(Interface):

def getMatchesFolders(self, node) -> list:
raise NotImplementedError

class TrackProviderInterface(Interface):

def getTracksFile(self, node) -> str:
raise NotImplementedError

3 changes: 2 additions & 1 deletion meshroom/core/desc/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

from .computation import Level, StaticNodeSize
from .attribute import Attribute, ChoiceParam, ColorParam, Flow, IntParam, StringParam, ListAttribute
from .interface import InterfacedClass

_MESHROOM_COMPUTE = (Path(_MESHROOM_ROOT) / "bin" / "meshroom_compute").as_posix()
_MESHROOM_COMPUTE_DEPS = ["psutil"]
Expand Down Expand Up @@ -212,7 +213,7 @@ def getInternalFlowOutputs(cls, mrNodeType: MrNodeType) -> list[Attribute]:
return cls.FLOW_OUT


class BaseNode(object):
class BaseNode(InterfacedClass):
"""
"""
cpu = Level.NORMAL
Expand Down
53 changes: 20 additions & 33 deletions meshroom/ui/qml/Viewer/Viewer2D.qml
Original file line number Diff line number Diff line change
Expand Up @@ -844,7 +844,7 @@ FocusScope {
ExifOrientedViewer {
id: featuresViewerLoader
active: displayFeatures.checked && !useExternal
property var activeNode: _currentScene ? _currentScene.activeNodes.get("featureProvider").node : null
property var activeNode: _currentScene ? _currentScene.activeNodes.get("FeatureProviderInterface").node : null
width: imgContainer.width
height: imgContainer.height
anchors.centerIn: parent
Expand All @@ -856,7 +856,13 @@ FocusScope {
if (active) {
// Instantiate and initialize a FeaturesViewer component dynamically using Loader.setSource
setSource("FeaturesViewer.qml", {
"model": Qt.binding(function() { return activeNode ? activeNode.attribute("describerTypes").value : "" }),
"model": Qt.binding(function() {
let result = []
if (activeNode) {
result = _currentScene.callInterfaceMethod("FeatureProviderInterface", "getDescriberTypes") || ""
}
return result
}),
"currentViewId": Qt.binding(function() { return _currentScene.selectedViewId }),
"features": Qt.binding(function() { return mfeaturesLoader.status === Loader.Ready ? mfeaturesLoader.item : null }),
"tracks": Qt.binding(function() { return mtracksLoader.status === Loader.Ready ? mtracksLoader.item : null }),
Expand Down Expand Up @@ -1171,7 +1177,7 @@ FocusScope {
if (!root.aliceVisionPluginAvailable) {
return null
}
return _currentScene ? _currentScene.activeNodes.get("featureProvider").node : null
return _currentScene ? _currentScene.activeNodes.get("FeatureProviderInterface").node : null
}
property bool isComputed: activeNode && activeNode.isComputed
active: isUsed && isComputed
Expand All @@ -1182,23 +1188,16 @@ FocusScope {
// so it can fail safely if the C++ plugin is not available
setSource("MFeatures.qml", {
"describerTypes": Qt.binding(function() {
return activeNode ? activeNode.attribute("describerTypes").value : {}
let result = []
if (activeNode) {
result = _currentScene.callInterfaceMethod("FeatureProviderInterface", "getDescriberTypes") || ""
}
return result
}),
"featureFolders": Qt.binding(function() {
let result = []
if (activeNode) {
if (activeNode.nodeType == "FeatureExtraction" && isComputed) {
result.push(activeNode.attribute("output").value)
}
else if (activeNode.nodeType == "RomaReducer" && isComputed) {
result.push(activeNode.attribute("featuresFolder").value)
}
else if (activeNode.hasAttribute("featuresFolders")) {
for (let i = 0; i < activeNode.attribute("featuresFolders").value.count; i++) {
let attr = activeNode.attribute("featuresFolders").value.at(i)
result.push(attr.value)
}
}
result = _currentScene.callInterfaceMethod("FeatureProviderInterface", "getFeaturesFolders") || ""
}
return result
}),
Expand Down Expand Up @@ -1282,12 +1281,12 @@ FocusScope {
if (_currentScene)
{
//Try first to use tracks
if (_currentScene.activeNodes.get("trackProvider").node)
if (_currentScene.activeNodes.get("TrackProviderInterface").node)
{
return _currentScene.activeNodes.get("trackProvider").node
return _currentScene.activeNodes.get("TrackProviderInterface").node
}

return _currentScene.activeNodes.get("matchProvider").node
return _currentScene.activeNodes.get("MatchProviderInterface").node
}

return null
Expand All @@ -1306,26 +1305,14 @@ FocusScope {
"matchingFolders": Qt.binding(function() {
let result = []
if (activeNode) {
if (activeNode.nodeType == "FeatureMatching" && isComputed) {
result.push(activeNode.attribute("output").value)
} else if (activeNode.hasAttribute("matchesFolders")) {
for (let i = 0; i < activeNode.attribute("matchesFolders").value.count; i++) {
let attr = activeNode.attribute("matchesFolders").value.at(i)
result.push(attr.value)
}
}
result = _currentScene.callInterfaceMethod("MatchProviderInterface", "getMatchesFolders") || []
}
return result
}),
"tracksFile": Qt.binding(function() {
let result = ""
if (activeNode) {
if (activeNode.nodeType == "TracksBuilding" && isComputed) {
result = activeNode.attribute("output").value
}
else if (activeNode.hasAttribute("tracksFilename")) {
result = activeNode.attribute("tracksFilename").value
}
result = _currentScene.callInterfaceMethod("TrackProviderInterface", "getTracksFile") || ""
}
return result
})
Expand Down
42 changes: 35 additions & 7 deletions meshroom/ui/scene.py
Original file line number Diff line number Diff line change
Expand Up @@ -330,12 +330,6 @@ class Scene(UIGraph):
"ApplyCalibration", "SfMExpanding", "SfMBootstraping"],
# All nodes generating depth map files
"allDepthMap": ["DepthMap", "DepthMapFilter"],
# Nodes that can be used to provide features folders to the UI
"featureProvider": ["FeatureExtraction", "FeatureMatching", "StructureFromMotion", "RomaReducer"],
# Nodes that can be used to provide matches folders to the UI
"matchProvider": ["FeatureMatching", "StructureFromMotion", "RomaReducer"],
# Nodes that can be used to provide tracks files to the UI
"trackProvider": ["TracksBuilding", "SfMBootstraping", "SfMExpanding"]
}
# Nodes accessed from the UI
uiNodes = [
Expand Down Expand Up @@ -418,9 +412,16 @@ def initActiveNodes(self):
# For all nodes declared to be accessed by the UI
usedNodeTypes = {j for i in self.activeNodeCategories.values() for j in i}
allLoadedNodeTypes = set(meshroom.core.pluginManager.getRegisteredNodePlugins().keys())

for nodeType, nodePlugin in meshroom.core.pluginManager.getRegisteredNodePlugins().items():
nodeDesc = nodePlugin.nodeDescriptor
for interface in nodeDesc.interfaces:
if not self._activeNodes.get(interface):
self._activeNodes.add(ActiveNode(interface, parent=self))

allUiNodes = set(self.uiNodes) | usedNodeTypes | allLoadedNodeTypes

for nodeType in allUiNodes:
for nodeType in allUiNodes:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for nodeType in allUiNodes:
for nodeType in allUiNodes:

self._activeNodes.add(ActiveNode(nodeType, parent=self))

def clearActiveNodes(self):
Expand Down Expand Up @@ -985,11 +986,38 @@ def setBuildingIntrinsics(self, value):

pluginsReloaded = Signal(list, list)

@Slot(str, str, result="QVariant")
def callInterfaceMethod(self, interfaceName, methodName):
"""Call a method defined by an interface on the active node for that interface.

Args:
interfaceName: name of the interface (e.g. "FeatureProviderInterface")
methodName: name of the method to call on the nodeDesc (e.g. "getFeaturesFolders")

Returns:
The result of the method call, or None if the interface/method is not available.
"""
entry = self.activeNodes.get(interfaceName)
if not entry or not entry.node:
return None
node = entry.node
method = getattr(node.nodeDesc, methodName, None)
Comment on lines +1003 to +1004

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To make this call more robust and prevent potential AttributeErrors (for example, if node is a CompatibilityNode or does not have a nodeDesc attribute), we should safely retrieve nodeDesc using getattr and verify it is not None before attempting to fetch the method.

        node = entry.node
        nodeDesc = getattr(node, "nodeDesc", None)
        if not nodeDesc:
            return None
        method = getattr(nodeDesc, methodName, None)

if callable(method):
try:
return method(node)
except Exception as e:
logging.error(f"callInterfaceMethod: {interfaceName}.{methodName} raised: {e}")
return None
return None
Comment on lines +1005 to +1011

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On failure, it could try to load the next possible activeNode that implement this interface, using a while ?


@Slot(QObject)
def setActiveNode(self, node, categories=True, inputs=True):
""" Set node as the active node of its type and of its categories.
Also upgrade related input nodes.
"""
for interface in node.nodeDesc.interfaces:
self.activeNodes.getr(interface).node = node
Comment on lines +1018 to +1019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

There is a critical typo here: getr is used instead of get. This will raise an AttributeError at runtime when setActiveNode is called. Additionally, we should safely check if nodeDesc exists and has the interfaces attribute, and ensure that self.activeNodes.get(interface) actually returns a valid entry before setting its node. Note that the same typo getr also exists on line 1029 (which is outside the current diff hunk) and should be fixed as well.

Suggested change
for interface in node.nodeDesc.interfaces:
self.activeNodes.getr(interface).node = node
nodeDesc = getattr(node, "nodeDesc", None)
if nodeDesc and hasattr(nodeDesc, "interfaces"):
for interface in nodeDesc.interfaces:
entry = self.activeNodes.get(interface)
if entry:
entry.node = node


if categories:
for category, nodeTypes in self.activeNodeCategories.items():
if node.nodeType in nodeTypes:
Expand Down
Loading