Skip to content

[core/ui] Dev/connect group attributes and avoid cyclic dependency #2745

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 8 commits into
base: develop
Choose a base branch
from
Draft

[core/ui] Dev/connect group attributes and avoid cyclic dependency #2745

wants to merge 8 commits into from

Conversation

nicolas-lambert-tc
Copy link
Contributor

@nicolas-lambert-tc nicolas-lambert-tc commented Jun 11, 2025 •
edited
Loading

Description

This PR is a rebase of this PR from @yann-lty : #2684 with some adjustment.

Group Attributes connection

Screenshot_2025-06-11_18-28-22

Features list

Original PR

  • Support connection between GroupAttributes (API, serialization, UI).
  • [ui] Make connected GroupAttribute readonly.

Adjustments

  • GroupAttributeConnection do not replace the attributes, but copy value only
  • Only GroupAttribute with same structure can be connected. (Ordered sub attributes with same types)
  • Nested GroupAttributes are handled recursively
  • Cyclic dependency is now forbidden

Implementation remarks

  • A new DAGVisitor has been added and used as default visitor to avoid the cyclic dependency
  • Attributes are now responsible of the connection validation

yann-lty and others added 4 commits June 10, 2025 14:51
@yann-lty @nicolas-lambert-tc
[core] Simplify handling of link assignment by value on Attribute
2a3b220 
Centralize the handling of link assigment by value in a dedicated
private method.
Store link assignment by value in a separate member variable
`_linkExpression`, to avoid manipulating a internal `_value` that
can represent different concepts (value VS serialized edge expression).
@yann-lty @nicolas-lambert-tc
[core] Add support for connecting GroupAttributes
ec4276d 
Make GroupAttributes connectable by supporting:
- link assignment
- applying link expression
- considering linkParam within appropriate methods
@yann-lty @nicolas-lambert-tc
[ui] AttributeItemDelegate: make connected GroupAttribute readOnly
e81a096
@nicolas-lambert-tc
[ui] GroupAttribute: Connection validation responsability has been tr…
bcabb5b 
…ansfered to Attribute.

It let the opportunity to override it in specific Attribute
@nicolas-lambert-tc nicolas-lambert-tc self-assigned this Jun 11, 2025
@codecov Codecov
Copy link

codecov bot commented Jun 11, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 91.04478% with 18 lines in your changes missing coverage. Please review.

Project coverage is 78.83%. Comparing base (e36804f) to head (fc15c55).

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
meshroom/core/attribute.py 84.94% 14 Missing ⚠️
meshroom/core/graph.py 84.61% 4 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop    #2745      +/-   ##
===========================================
+ Coverage    78.46%   78.83%   +0.36%     
===========================================
  Files           47       51       +4     
  Lines         6542     6689     +147     
===========================================
+ Hits          5133     5273     +140     
- Misses        1409     1416       +7

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

@nicolas-lambert-tc nicolas-lambert-tc changed the title Dev/connect group attributes and avoid cyclic dependency [core/ui] Dev/connect group attributes and avoid cyclic dependency Jun 11, 2025
@nicolas-lambert-tc nicolas-lambert-tc added this to the Meshroom 2025.1.0 milestone Jun 11, 2025
@nicolas-lambert-tc nicolas-lambert-tc added the feature new feature (proposed as PR or issue planned by dev) label Jun 11, 2025
@nicolas-lambert-tc nicolas-lambert-tc force-pushed the dev/connectGroupAttributesAndAvoidCyclicDependency branch from 1624c9a to 70751af Compare June 11, 2025 16:38
@nicolas-lambert-tc nicolas-lambert-tc force-pushed the dev/connectGroupAttributesAndAvoidCyclicDependency branch from 70751af to fc15c55 Compare June 12, 2025 09:49
@nicolas-lambert-tc nicolas-lambert-tc marked this pull request as ready for review June 12, 2025 09:51
@fabiencastan fabiencastan marked this pull request as draft June 14, 2025 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cbentejac cbentejac Awaiting requested review from cbentejac

@yann-lty yann-lty Awaiting requested review from yann-lty

@fabiencastan fabiencastan Awaiting requested review from fabiencastan

At least 1 approving review is required to merge this pull request.

Assignees

@nicolas-lambert-tc nicolas-lambert-tc

Labels

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

Projects

None yet

Milestone

Meshroom 2026.1.0

Development

Successfully merging this pull request may close these issues.

3 participants

@nicolas-lambert-tc @fabiencastan @yann-lty