Skip to content

Conversation

@cbentejac
Copy link
Contributor

@cbentejac cbentejac commented Dec 26, 2025

Description

This PR replaces #2544 and #2754.

The main feature of this PR is that it adds the support of connections between attributes that are part of a GroupAttribute. Prior to this PR, these attributes could neither be connected individually, nor be displayed in the Graph Editor; they were however visible and editable in the Node Editor.

GroupAttributes are now displayed in the Graph Editor with a right chevron, which means they can be expanded when clicked upon. When this happens, all the children attributes of the clicked GroupAttribute are displayed below it, in a tree view with the indentation reflecting their depth level.

Connections between two GroupAttributes of a similar structure are now supported. If the GroupAttributes have different structures, the connection will be refused straight away and the user will not even be able to select the group's pin.

Connecting two GroupAttributes together explicitly connects all their children attributes together. If any of the children attributes is disconnected or reconnected to another attribute, then the connection between the groups is broken.

Features list

  • Add the notion of depth for attributes that are children of a GroupAttribute. An attribute that has no parent will have a depth of 0, and it will increase of 1 for every parent;
  • Add a flatStaticChildren property for every attribute that may have children that contains the flattened list of said children;
  • Correctly evaluate links when they reference children attributes, hence adding the support of the connection of individual attributes within groups;
  • Add an icon to expand or collapse GroupAttributes and display their children attributes in their node in the Graph Editor with a tree view. These children attributes are graphically connectable;
  • Display the full name of the attribute in its tooltip in the Graph Editor. If an attribute childAttribute is a child of a group groupAttribute, its name will be displayed as groupAttribute.childAttribute, making the relationship between the two obvious.

Implementation remarks

Connections between attributes used to be handle directly with the addEdge and removeEdge methods. Two new methods, connectTo and disconnectEdge, are added and now directly call addEdge and removeEdge while handling all the specific connection/disconnection operations that are required when connecting GroupAttributes together.

@cbentejac cbentejac added this to the Meshroom 2026.1.0 milestone Dec 26, 2025
@cbentejac cbentejac self-assigned this Dec 26, 2025
@codecov
Copy link

codecov bot commented Dec 26, 2025

Codecov Report

❌ Patch coverage is 95.72447% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.68%. Comparing base (f7a48c9) to head (aafd268).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
meshroom/core/attribute.py 90.51% 13 Missing ⚠️
meshroom/core/graph.py 85.18% 4 Missing ⚠️
meshroom/core/node.py 80.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2965      +/-   ##
===========================================
+ Coverage    78.91%   79.68%   +0.76%     
===========================================
  Files           59       64       +5     
  Lines         8305     8600     +295     
===========================================
+ Hits          6554     6853     +299     
+ Misses        1751     1747       -4     

☔ 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.

@cbentejac cbentejac force-pushed the dev/groupConnections branch from 021a674 to f71f32a Compare December 26, 2025 15:20
Centralize the handling of link assignment by value in a dedicated
private method.
Store link assignment by value in a separate member variable
`_linkExpression` to avoid manipulating an internal `_value` that can
represent different concepts (value VS serialized edge expression).
Make `GroupAttributes` connectable by supporting:
- link assignment
- applying link expressions
- considering `linkParam` within appropriate methods
@cbentejac cbentejac force-pushed the dev/groupConnections branch 4 times, most recently from a623484 to d4f596e Compare January 5, 2026 17:44
@cbentejac cbentejac added the feature new feature (proposed as PR or issue planned by dev) label Jan 5, 2026
By default, it only checks whether the base type of the incoming
attribute matches the current attribute's. It is meant to be overriden
by custom methods if need be.
`depth` describes the level of depth the attribute is located at
(i.e. how many attributes can be accessed through the `root` property
before it becomes `None`).

`flatStaticChildren` lists all the attributes that refer to the current
one as `root`, whether directly or through intermediate attributes.
`flatStaticChildren` is overridden with a function that lists all the
children of the attribute.

`_validateIncomingConnection` checks, in addition to the base type
of the incoming attribute, that the structures of the two
`GroupAttributes` match, with the same types of attributes at the same
depth levels.
The `exposed` property, which determines whether the attribute is
displayed in the upper part of the node in the Graph Editor, is set
for each attribute individually in their node's description.

If an attribute has a parent (meaning it depends on a `ListAttribute` or
a `GroupAttribute`) whose `exposed` property value differs, it does not
make sense to display it separately. The attribute's `exposed` should be
aligned with its parent's.
`connectTo` runs checks to ensure the connection is valid before
performing the `addEdge` operation. The connection of `GroupAttributes`
is handled by an overridden version of the method, so all the attributes
of a group are automatically individually connected.
This allows to get straight away the list of all the edges that were
created upon the connection of a single attribute. This is relevant in
the case of `GroupAttributes`, as connecting one group to another consists
in connecting all the sub-attributes from those groups to one another.

For other attribute types, this consists of returning a single edge in
a list.
This prevents returning references from the connecting `GroupAttribute`
when checking the values of the children attributes of the connected one.
@cbentejac cbentejac force-pushed the dev/groupConnections branch from d4f596e to 8b15de5 Compare January 5, 2026 17:52
…butes` with connected children

If a group is not connected but has children that are connected, then its
pin does not appear filled, although, when collapsed, the edges of the
children seems to connect from/to it.

With this commit, if such an event occurs, the border of the pin for
the group takes the usual color for a connected attribute, and its inner
pin takes the usual border's color.
This allows to immediately identify that connections occur surrounding the
`GroupAttribute`, although it is not connected itself.
In the `connectTo` and `disconnectEdge` methods, we now save the pairs
of attributes that were connected or disconnected in the process, as
several edges that do not connect the given input/output attributes
may be removed when performing an edge addition or removal.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds comprehensive support for connecting and displaying GroupAttribute children in the Graph Editor. Previously, attributes nested within GroupAttributes were only accessible in the Node Editor. Now users can expand GroupAttributes to view and connect their child attributes directly in the Graph Editor.

Key changes:

  • Introduces connectTo() and disconnectEdge() methods to replace direct addEdge()/removeEdge() calls, enabling proper handling of GroupAttribute connections
  • Adds expand/collapse UI with chevron icons for GroupAttributes in the Graph Editor, displaying children in a tree view with depth-based indentation
  • Implements structural validation for GroupAttribute connections, ensuring only compatible structures can be connected

Reviewed changes

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

Show a summary per file
File Description
meshroom/core/attribute.py Core logic for attribute connections, depth tracking, flatStaticChildren property, and GroupAttribute validation/connection handling
meshroom/core/graph.py Updated addEdge/removeEdge to return edge tuples, added cyclic dependency detection
meshroom/core/exception.py Added InvalidEdgeError and CyclicDependencyError exceptions
meshroom/core/node.py Enhanced CompatibilityNode to handle GroupAttribute deserialization
meshroom/ui/commands.py Updated AddEdgeCommand and RemoveEdgeCommand to use new connectTo/disconnectEdge methods with proper undo/redo
meshroom/ui/graph.py Fixed edge removal logic for non-ListAttribute edges
meshroom/ui/qml/GraphEditor/AttributePin.qml Major UI enhancements: expand/collapse functionality, tree view with depth-based padding, improved connection validation
meshroom/ui/qml/GraphEditor/Node.qml Updated to use generateAttributesModel for dynamic attribute display including children
meshroom/ui/qml/GraphEditor/GraphEditor.qml Enhanced edge rendering with getAttributePin() to show edges for collapsed GroupAttributes
meshroom/ui/qml/MaterialIcons/MaterialToolLabel.qml Refactored component to expose icon and label separately for better flexibility
tests/test_groupAttributes.py Comprehensive test suite for GroupAttribute connections, structure validation, and serialization
tests/nodes/test/*.py New test nodes (GroupAttributes, Position, Color, NestedPosition, NestedColor, NestedTest)
All other test files Converted from graph.addEdge() to attribute.connectTo() calls

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature new feature (proposed as PR or issue planned by dev)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants