New Custom Attributes#3136
Conversation
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new CustomAttributes feature to Meshroom, enabling dynamic input and output attributes on nodes that can be duplicated, removed, and connected dynamically. It includes core attribute model updates, serialization support, UI adjustments for collapsable attributes, and new tests. The review feedback identifies several critical issues, including a missing mixin initialization in GroupAttribute that leads to an AttributeError, potential KeyError crashes when querying DictModel with .get(), incomplete cleanup during command execution failures, property loss in Attribute.clone(), a security risk from using exec() in a test node, and a syntax issue with a non-breaking space in QML.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #3136 +/- ##
===========================================
+ Coverage 85.36% 85.55% +0.19%
===========================================
Files 73 78 +5
Lines 11455 11779 +324
===========================================
+ Hits 9778 10077 +299
- Misses 1677 1702 +25 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR introduces a new CustomAttributes container attribute type that can dynamically create (duplicate) child attributes when users connect an edge to the container, with supporting UI changes and serialization/deserialization support so these runtime-created attributes persist across save/load.
Changes:
- Add
desc.CustomAttributes+core.attribute.CustomAttributeswith runtime child duplication/removal and serialization support. - Update Graph Editor QML to treat CustomAttributes as collapsable/group-like and to keep edge visuals/pin mapping working with dynamic attributes.
- Add UI commands + tests to validate duplication, connection, and graph save/load behavior.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_dynamic_attribute.py |
New tests covering CustomAttributes creation, duplication, serialization, and graph save/load. |
tests/nodes/test/DynamicNode.py |
Test node exposing CustomAttributes input/output containers. |
meshroom/ui/qml/GraphEditor/Node.qml |
Collapsable logic updated to use isCollapsable/fullName; model helper added. |
meshroom/ui/qml/GraphEditor/GraphEditor.qml |
Edge pin lookup switched to attribute.fullName; adds a forced visual update mechanism. |
meshroom/ui/qml/GraphEditor/CollapsableAttributePin.qml |
New QML component stub. |
meshroom/ui/qml/GraphEditor/AttributePinCollapsable.qml |
New QML component stub. |
meshroom/ui/qml/GraphEditor/AttributePin.qml |
Adds dynamic/custom styling + adjusts connection validation call direction. |
meshroom/ui/qml/GraphEditor/AttributeItemDelegate.qml |
Renders CustomAttributes using the group attribute component. |
meshroom/ui/graph.py |
Routes edge creation involving CustomAttributes to a dedicated undoable command. |
meshroom/ui/commands.py |
Adds ConnectCustomAttributesCommand to duplicate + connect attributes via undo stack. |
meshroom/nodes/general/PluginSubmitter.py |
Adds new submitter-related nodes under shipped node set. |
meshroom/core/nodeFactory.py |
Adjusts compatibility checks to ignore CustomAttributes dynamic children. |
meshroom/core/node.py |
Treats CustomAttributes as dynamic-output-bearing for values I/O; tweaks output serialization filtering. |
meshroom/core/mixins.py |
Adds Collapsable mixin with collapsed state and QML-facing properties. |
meshroom/core/graphIO.py |
Adds CustomAttributes serialization behavior (notably for partial graph serialization). |
meshroom/core/graph.py |
Alters edge compatibility checks (special-casing CustomAttributes). |
meshroom/core/desc/node.py |
Marks nodes with CustomAttributes outputs as having dynamic outputs. |
meshroom/core/desc/customAttributes.py |
New descriptor for CustomAttributes. |
meshroom/core/desc/attribute.py |
Adds clone() / asDict() helpers and adjusts a GroupAttribute error message. |
meshroom/core/desc/__init__.py |
Exports CustomAttributes and adds a string→desc-class map for reconstruction. |
meshroom/core/attribute.py |
Core runtime implementation for CustomAttributes + child duplication, plus Collapsable integration. |
meshroom/common/core.py |
Adds count and ordered insert() support to the core DictModel. |
.vscode/launch.json |
Enables QML debug by default in repo launch config. |
Comments suppressed due to low confidence (1)
meshroom/core/attribute.py:1205
- GroupAttribute now mixes in Collapsable, but Collapsable.init is never called, so '_collapsed' is never initialized. Accessing the 'collapsed' property will raise at runtime.
def __init__(self, node, attributeDesc: desc.GroupAttribute, isOutput: bool,
root=None, parent=None):
super().__init__(node, attributeDesc, isOutput, root, parent)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
nicolas-lambert-tc
force-pushed
the
feature/custom-attributes
branch
3 times, most recently
from
a57a2af to
bbc9a8e
Compare
nicolas-lambert-tc
requested review from
Alxiice,
CedricThebault,
cbentejac,
demoulinv,
fabiencastan and
servantftransperfect
nicolas-lambert-tc
force-pushed
the
feature/custom-attributes
branch
2 times, most recently
from
8b25b52 to
9b8a205
Compare
nicolas-lambert-tc
force-pushed
the
feature/custom-attributes
branch
from
ce78a1f to
fd6028c
Compare
nicolas-lambert-tc
force-pushed
the
feature/custom-attributes
branch
from
fd6028c to
43bbe20
Compare
| STR_TO_ATTRIBUTE_DESCRIPTION = { | ||
| 'ColorParam': ColorParam, | ||
| 'File': File, | ||
| 'FloatParam': FloatParam, | ||
| 'Flow': Flow, | ||
| 'Geometry': Geometry, | ||
| 'Size2d': Size2d, | ||
| 'Vec2d': Vec2d, | ||
| 'GroupAttribute': GroupAttribute, | ||
| 'AnySet': AnySet, | ||
| 'IntParam': IntParam, | ||
| 'ListAttribute': ListAttribute, | ||
| 'PushButtonParam': PushButtonParam, | ||
| 'StringParam': StringParam, | ||
| 'Shape': Shape, | ||
| 'ShapeList': ShapeList, | ||
| 'Point2d': Point2d, | ||
| 'Line2d': Line2d, | ||
| 'Rectangle': Rectangle, | ||
| 'Circle': Circle, | ||
| } |
There was a problem hiding this comment.
it's only used in attribute.py... why not putting it there ?
There was a problem hiding this comment.
I've chosen to put it closer to the sources. A new description added will need the developer to expose it to other modules via init so it is easier to see that it needs to add this to the dictionary, it would be easy to forget to update the core.attributes.py
|
Would it be possible to implement the reset at least ? |
|
I had several errors : class DynamicNode(desc.InputNode):
inputs = [
desc.File(
name="test"
),
desc.AnySet(
name="ins",
label="Custom Inputs",
description="Custom Inputs",
exposed=True
)
]
connect a ListImages -> we get these errors: Then remove the File input and do a hot reload -> the connection breaks and we get this : (For some reasons we get the error messages multiple times) |
| from meshroom.common import Property, Variant | ||
|
|
||
|
|
||
| class AnySet(GroupAttribute): |
There was a problem hiding this comment.
Why not "DynamicGroupAttribute" ?
There was a problem hiding this comment.
We chose to not used the dynamic term to avoid confusion with dynamic values.
…y override the needed clone method
| @@ -0,0 +1,180 @@ | |||
| """Tests for the AnySet feature.""" | |||
|
|
|||
| from meshroom.core import desc | |||
Description
CustomAttributesconcept:It's a container of custom attributes. It can be input/output as usual attribute.
When an attribute is connected to it's slot, a clone of the attribute connected is added to the container, and both attributes a re connected by default.
Features list
Known Limitations:
Implementation remarks
To avoid concepts conflict, the parameter is called CustomAttributes, and no more DynamicAttributes.
The Dynamic attribute/value already exists for output attributes that have a value of None.
In this version there is no way to rename/delete an attribute created by the user using the UI.
It would be added in an other PR
Add the Collapsable class as a mixin to indicate in qml if the attribute should have the
Collapsablebehavior.The idea is to add some internal properties later to store the expanded state, ...
A
forceEdgesVisualUpdatehas been added in the GraphEditor.qml to force the edge evaluation when an attribute is added at runtime.A refacto of the AttributePin.qml/AttributeItemDelegate.qml should be done to help writing of new attributes display.
It should be decomposed in smaller parts
At serialization, the CustomAttributes serialize it's children atttribute with the necessary information to recreate the same attributes at deserialization time.
-> name, label, type, value