[UI] Add an auto-scroll feature to connect edges on huge scenes - #3159
[UI] Add an auto-scroll feature to connect edges on huge scenes#3159Alxiice wants to merge 6 commits into
Conversation
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #3159 +/- ##
========================================
Coverage 85.75% 85.75%
========================================
Files 78 78
Lines 12087 12087
========================================
Hits 10365 10365
Misses 1722 1722 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![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 an autoscroll feature to the Graph Editor when dragging attribute pin edges near the workspace boundaries. It also refactors the QmlInstantEngine to inherit from QObject instead of QQmlApplicationEngine to facilitate cleaner hot-reloads. The review feedback identifies several key issues: inefficient signal emission and a correctness bug in the edge dragging logic of AttributePin.qml, potential runaway scrolling speeds when dragging far outside the Graph Editor boundaries, and a missing parent reference for a QTimer in utils.py that could lead to memory leaks.
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.
8d2fe99 to
030c50a
Compare
2997ee4 to
567d3d9
Compare
… send edge dragging info
…pproach the GraphEditor borders
…t previously triggered
567d3d9 to
96fc92e
Compare
| root.highlightedBorderRight=false | ||
| root.highlightedBorderLeft=false | ||
| root.highlightedBorderTop=false | ||
| root.highlightedBorderBottom=false |
There was a problem hiding this comment.
Since those attributes are always reset together, maybe regroup all those booleans inside a single object ?
| } | ||
| // Bottom | ||
| else if (mouseY > root.height - root.autoscrollMargin) { | ||
| var factorY = Math.min(root.maxScrollSpeed, (mouseY - (root.height - root.autoscrollMargin)) / root.autoscrollMargin) |
There was a problem hiding this comment.
The computations for factorX and factorY are almost identical in each case. Maybe this could be refactored within a function ?
raphaelKoskas
left a comment
raphaelKoskas
left a comment
There was a problem hiding this comment.
- Function-wise, this code does what it supposed to do and I'm good with the scrolling speed and margins you defined.
- Keeping the name of the attribute getting pulled visible is a good idea, but brings out an odd behaviour in the context of this fix :
- the label square follows the view translation within the graph editor, but is clipped to the meshroom window, meaning specifically in the context this fix was made for, I can easily get the label square to leave the graph editor panel,
- and I believe a more interesting feature, especially for this context, would be to have the label square to track the mouse position (with a small offset for readability) instead.
Aside from this, it looks good to me.
2 participants
Description
When scenes are too big and we want to connect nodes that cannot be seen at the same time, it's not convenient to create edges.
This PR aims to add an "auto-scroll" feature that will move the graph when we approach the edges of the graph editor. This way users can easily drag edges, move in the graph and connect the edges.
Notes for the review
Aside from the code review I would like to know what would be the best settings for
Demo