Skip to content
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.

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

Add basic drag and drop support in the model designer #60664

Draft
wants to merge 32 commits into
base: master
Choose a base branch
from

Conversation

ValentinBuira
Copy link
Contributor

@ValentinBuira ValentinBuira commented Feb 19, 2025

Introduction

This PR add the new way to edit a model using a drag and drop interface. This is part of long waited improvements in the model designer capabilities.

The design is based of the Google Summer of code proposal I did last year, and that we discussed on the mailling list. Compared to the original proposal the UI looks more similar to the current modeler interface

I encourage anyone to open your existing models and give it a try !

Thanks to the Hauts-de-France region for sponsorising this work

How does it works ?

New sockets were added around each parameter and algorithm used in a model.

sockets.example.mp4

Those are interactive and are used to connect component between them. To connect component drag any output socket regardless of it's from a model parameter or a algorithm into an input socket. You can also remove a connection by dragging an edge out of the socket.

peek.drag.and.drop.mp4

Theses new capabilities have been added as part of the select/edit tool. No needs to switch to a new tool when you want to edit connection in your model. The icon has been updated in consequence.

Screenshot From 2025-02-19 08-29-54

This PR remain fully compatible with the previous user interface. When you connect sockets by drag and drop. The dialog used when opening an algorithm is updated as well.

retro.compatible.mp4

@github-actions github-actions bot added this to the 3.42.0 milestone Feb 19, 2025
Copy link

github-actions bot commented Feb 19, 2025

🪟 Windows builds

Download Windows builds of this PR for testing.
Debug symbols for this build are available here.
(Built from commit 0fcc765)

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit 0fcc765)

Copy link
Contributor

@troopa81 troopa81 left a comment

Choose a reason for hiding this comment

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

Good job! 👍

Please let me know when you have finished this work and the PR is not draft anymore, I would make a proper review.


painter->setRenderHint( QPainter::Antialiasing );

float display_size = 3.2;
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe a static constant ?

Copy link
Contributor Author

@ValentinBuira ValentinBuira Feb 24, 2025

Choose a reason for hiding this comment

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

Replaced as constexpr

@nyalldawson nyalldawson added the Frozen Feature freeze - Do not merge! label Feb 19, 2025
@nyalldawson nyalldawson self-assigned this Feb 19, 2025
@nyalldawson
Copy link
Collaborator

This is great! 🥳

I haven't done a code review yet, but just a couple of quick thoughts based on the screencasts:

  • I think we should switch the mouse cursor when hovering over the node to the open-hand cursor, and then to closed-hand when the drag operation actually commences. (This would give users an immediate hint that the nodes are draggable)
  • I'd also give a hint when a node will accept a drop -- eg when in the middle of the drag and the mouse is hovered over a potential destination node, then that node should show in a shaded state (we could potentially use a "red" shading if the node is incompatible with the object being dragged)

@nyalldawson nyalldawson removed the Frozen Feature freeze - Do not merge! label Feb 21, 2025
* manner.
* \see outputDefinitions()
*/
int outputDefinitionIndex( const QString &name ) const SIP_HOLDGIL;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think instead of cluttering the (already large) QgsProcessingAlgorithm API with this, let's move it to a static method in QgsProcessingUtils instead.

Comment on lines +172 to +175
int index() { return mIndex; };
Qt::Edge edge() { return mEdge; };

bool isInput() { return mEdge == Qt::TopEdge; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing docs, const

Comment on lines +114 to +115
// ReOrder in out socket
// always fix on the input end receiving
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is unclear -- can you elaborate?


emit view() -> endCommand();
// Redraw
emit scene() -> rebuildRequired();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Similar here -- externally emitting a signal is ugly

Comment on lines +209 to +211
QgsProcessingModelChildAlgorithm *_alg;
// This is not so nice to have the UI tangled gotta think of a better abstraction later
// Loop trought all items to get the output socket
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are you already planning on reworking all this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not at the moment, I had in mind something similar to the separation between QgsProcessingModelComponent and QgsModelComponentGraphicItem but that would require more signifant work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants