[ui] Rename node #2953

Alxiice · 2025-12-11T16:27:58Z

Why this PR ?

We miss a way to rename the node name. Node names are usually decided when we create the node by the graph part, but there is no way to set the name.
There is a label system that was used but that has only an impact on the display, it doesn't really changes the node name
Finally now if the name differs from the node type the node type is displayed on parenthesis :

I find this cleaner because previously it was the "default label" (so it would have been TestCLNode_N here)

Features implemented

Here is a gif that shows the new rename system

another gif to show the update on the label

double click on the node editor to rename
the name is changed
it is added in the GraphCommand so undo/redo should work
if the set name doesn't start with the node type (like "NodeType_") then the node type is precised in a label after
On the graph editor if no specific label is set, then the label is updated to the new node name

Special point of attention for reviewers

Compute/Submit : When the node is locked, the node renaming is supposed to be locked so this should prevent most issues
Graph editor : Renaming the node should set the correct label too
try to close/reopen meshroom to make sure everything works well
NodeEditor.qml cancelNodeNameChange : is it ok to set an empty string to force the update ? I haven't found a better way to make this work but Copilot flagged it as a fragile workaround. I'm open to suggestions on this point

Remaining TODOs

add tests

servantftransperfect · 2026-01-02T10:47:05Z

There is no restriction on characters used. It accepts spaces, quotes, double quotes. did you test everything whenre there is those characters ?
When partially editing the name, nothing works well. (_1 is added each time, the label is kept empty, etc etc)

cbentejac

One minor change is needed on the QML side, otherwise everything looks good to me!

I do think it'd be good to add some unit tests though, as it looks like a feature that may unintentionally break without realising it otherwise.

meshroom/ui/qml/GraphEditor/NodeEditor.qml

Copilot

Pull request overview

This PR implements a node renaming feature in the UI, allowing users to double-click on node names in the NodeEditor to rename them. The feature includes undo/redo support via GraphCommand and updates node labels accordingly.

Changes:

Added interactive node name editing in the NodeEditor with validation and keyboard shortcuts
Implemented RenameNodeCommand for undo/redo support
Modified node name property to be mutable with change notifications
Extended data structures to support renaming operations

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
meshroom/ui/qml/GraphEditor/NodeEditor.qml	Adds custom title component with editable TextField for node name and display logic for node type
meshroom/ui/qml/GraphEditor/Node.qml	Updates node label binding to refresh when node name changes
meshroom/ui/qml/Controls/Panel.qml	Adds support for custom title components via Loader
meshroom/ui/graph.py	Implements renameNode slot with name sanitization logic
meshroom/ui/commands.py	Adds RenameNodeCommand for undoable node renaming
meshroom/core/node.py	Changes name property from constant to notify signal and fixes nameToLabel for names without underscores
meshroom/core/graph.py	Implements renameNode method to update node names and emit change signals
meshroom/common/qt.py	Adds rename method to QObjectListModel
meshroom/common/core.py	Adds rename method to DictModel

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

meshroom/ui/graph.py

meshroom/ui/commands.py

meshroom/ui/qml/GraphEditor/NodeEditor.qml

meshroom/core/graph.py

meshroom/ui/commands.py

meshroom/common/qt.py

meshroom/common/core.py

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 13 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

meshroom/core/graph.py

Copilot · 2026-01-14T14:47:34Z

meshroom/ui/qml/GraphEditor/Node.qml

+            root.nodeLabel = ""
+            // Restore binding to root.node.label
+            root.nodeLabel = Qt.binding(function() { return root.node.label; })


Similar to the pattern in NodeEditor.qml, the empty string assignment followed by Qt.binding() restoration is used to force a property update. This is the same fragile workaround pattern. While this works, it would be better to have a consistent approach across the codebase. Consider refactoring both occurrences to use a more explicit state management approach.

yeah but I don't have another way yet

I would propably use the declarative way by creating a property on the core.BaseNode:
nodeLabel = Property(str, getLabel, notify=nodeNameChanged)
So you can directly use this property in the qml:
text: node.nodeLabel

One advantage is that anywhere you are using this property, any change on it will send the signal, so you don't have to explicitly connect in the qml.

I have issues making this working because nodeLabel is already a property that binds to another signal (internal properties) and it doesn't prevent the issues I had when cancelling the textEdit editing which was the initial reason why I had to force reset the binding.
I will text you directly to see if I missed something in the approach here

Right, not so obvious problem....

meshroom/core/graph.py

tests/test_graph.py

meshroom/ui/qml/GraphEditor/NodeEditor.qml

tests/test_graph.py

meshroom/ui/graph.py

tests/test_graph.py

meshroom/ui/qml/GraphEditor/NodeEditor.qml

nicolas-lambert-tc · 2026-01-16T08:08:24Z

meshroom/ui/qml/GraphEditor/Node.qml

+            root.nodeLabel = ""
+            // Restore binding to root.node.label
+            root.nodeLabel = Qt.binding(function() { return root.node.label; })


I would propably use the declarative way by creating a property on the core.BaseNode:
nodeLabel = Property(str, getLabel, notify=nodeNameChanged)
So you can directly use this property in the qml:
text: node.nodeLabel

One advantage is that anywhere you are using this property, any change on it will send the signal, so you don't have to explicitly connect in the qml.

nicolas-lambert-tc · 2026-01-16T08:30:31Z

meshroom/ui/qml/GraphEditor/NodeEditor.qml

+    property variant nodeName: node !== null ? node.name : undefined
+    property string displayNodeName: node !== null ? node.name : ""
+    property string validatedNodeName: displayNodeName
+    property string displayNodeType: ""
+
+    function updateNodeNameDisplay() {
+        if (_reconstruction.selectedNode) {
+            const nodeName = _reconstruction.selectedNode.name
+            root.displayNodeName = nodeName
+            root.validatedNodeName = nodeName
+            // Set the display node type only if it is not contained in the node name
+            const nodeType = _reconstruction.selectedNode.nodeType
+            root.displayNodeType = nodeName.startsWith(nodeType + "_") ? "" : nodeType
+        }
+    }
+
+    Connections {
+        target: _reconstruction
+        function onSelectedNodeChanged() {
+            updateNodeNameDisplay()
+        }
+    }
+
+    onNodeNameChanged: {
+        updateNodeNameDisplay()
+    }
+


It would be preferable to use the properties of the BaseNode (nodeType, and the suggested in the other comment nodeLabel) ?
You can remove all those imperative declaractions, and use the power of reactivity qml offer (see comments below)

nicolas-lambert-tc · 2026-01-16T08:33:35Z

meshroom/ui/qml/GraphEditor/NodeEditor.qml

+                text: "(" + root.displayNodeType + ")"
+                visible: root.displayNodeType !== "" && _reconstruction.selectedNode


No need to check selectedNode, the component activeness is already linked on the selection, I guess.
You can directly use node, and it's reactive properties

visible: node.nodeType text: `(${node.nodeType})`

codecov · 2026-01-21T11:29:02Z

Codecov Report

❌ Patch coverage is 80.43478% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.83%. Comparing base (d78b127) to head (9dde23e).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
meshroom/common/qt.py	12.50%	7 Missing ⚠️
meshroom/common/core.py	83.33%	1 Missing ⚠️
meshroom/core/node.py	66.66%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #2953      +/-   ##
===========================================
+ Coverage    79.80%   79.83%   +0.03%     
===========================================
  Files           64       64              
  Lines         8674     8718      +44     
===========================================
+ Hits          6922     6960      +38     
- Misses        1752     1758       +6

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…t specified index

… small ui issue

…+ other fixes suggested by copilot)

Alxiice self-assigned this Dec 11, 2025

Alxiice requested review from CedricThebault, cbentejac, demoulinv, fabiencastan and servantftransperfect December 11, 2025 16:28

Alxiice added the feature new feature (proposed as PR or issue planned by dev) label Dec 12, 2025

Alxiice added this to the Meshroom 2026.1.0 milestone Dec 12, 2025

Alxiice force-pushed the dev/rename_node branch 5 times, most recently from abbc49f to a417207 Compare January 8, 2026 16:05

cbentejac requested changes Jan 9, 2026

View reviewed changes

meshroom/ui/qml/GraphEditor/NodeEditor.qml Outdated Show resolved Hide resolved

cbentejac requested a review from Copilot January 14, 2026 10:02

Copilot started reviewing on behalf of cbentejac January 14, 2026 10:05 View session

Alxiice force-pushed the dev/rename_node branch from a417207 to 56d8033 Compare January 14, 2026 10:10

Copilot AI reviewed Jan 14, 2026

View reviewed changes

Alxiice force-pushed the dev/rename_node branch from 2b6aa45 to c0542f8 Compare January 14, 2026 14:34

Alxiice requested a review from cbentejac January 14, 2026 14:36

cbentejac requested a review from Copilot January 14, 2026 14:36

Copilot started reviewing on behalf of cbentejac January 14, 2026 14:37 View session

Copilot AI reviewed Jan 14, 2026

View reviewed changes

alicevision deleted a comment from codecov bot Jan 15, 2026

nicolas-lambert-tc reviewed Jan 16, 2026

View reviewed changes

Alxiice force-pushed the dev/rename_node branch 2 times, most recently from 6ae4f6c to 4039a2c Compare January 21, 2026 10:57

[ui] Rename node

1f565dd

Alxiice added 10 commits January 21, 2026 15:05

[core] Handle node rename implication on graph DictModel

5f0c9eb

[ui] Remove dead code

1fcd889

[ui] node rename : lock rename on node lock + update label on rename

66771fb

[node] add more protection to nameToLabel if we have node name withou…

3de772e

…t specified index

[graph] Prevent several bugs in the node rename feature

9db96f6

[core/ui] node rename : Add documentation + improve node rename & fix…

1641c73

… small ui issue

[tests] Add tests and more protection for the rename in core/graph

b879697

[core] node rename : assert the node cannot be renamed in lock mode (…

96976df

…+ other fixes suggested by copilot)

[tests] Remove cyclic dependency test (issue with rebase)

dc515d4

[refacto] remove reconstruction from the code

9dde23e

Alxiice force-pushed the dev/rename_node branch from e59751f to 9dde23e Compare January 21, 2026 16:08

cbentejac approved these changes Jan 23, 2026

View reviewed changes

		text: "(" + root.displayNodeType + ")"
		visible: root.displayNodeType !== "" && _reconstruction.selectedNode

Uh oh!

[ui] Rename node #2953

Are you sure you want to change the base?

[ui] Rename node #2953

Uh oh!

Conversation

Alxiice commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why this PR ?

Features implemented

Special point of attention for reviewers

Remaining TODOs

Uh oh!

servantftransperfect commented Jan 2, 2026

Uh oh!

cbentejac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Alxiice Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

nicolas-lambert-tc Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Alxiice Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicolas-lambert-tc Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nicolas-lambert-tc Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicolas-lambert-tc Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

nicolas-lambert-tc Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Alxiice commented Dec 11, 2025 •

edited

Loading

nicolas-lambert-tc Jan 16, 2026 •

edited

Loading

Alxiice Jan 21, 2026 •

edited

Loading

nicolas-lambert-tc Jan 16, 2026 •

edited

Loading

codecov bot commented Jan 21, 2026 •

edited

Loading