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

Allow replacing edges #1355

Merged
merged 1 commit into from
Apr 13, 2021
Merged

Allow replacing edges #1355

merged 1 commit into from
Apr 13, 2021

Conversation

ChemicalXandco
Copy link
Contributor

Description

This makes using the graph editor more efficient because if I have node A connected to node C but then I want to connect node B to node C instead then I would have to first delete the edge from node A to node C before it was possible to drag the edge from node B to node C. With these changes it is possible to do this without having to explicitly delete the previous edge.

@fabiencastan
Copy link
Member

Thanks @ChemicalXandco for the contribution. That's a good feature.

In term of implementation, I would like to avoid adding complexity to the low level commands.
The idea is to keep the basic commands as unitary as possible.

From the ui, just create a group of modifications: remove the edge and add a new one.
So you end up with 2 commands in one undo group.

What do you think?

@fabiencastan
Copy link
Member

The PR looks good to me. I will merge it now, but it would be good to display in red the edge that will be removed, like here between MeshFiltering and Texturing:
image

@fabiencastan fabiencastan added this to the Meshroom 2021.2.0 milestone Apr 13, 2021
@fabiencastan fabiencastan merged commit 36d410e into alicevision:develop Apr 13, 2021
@ChemicalXandco
Copy link
Contributor Author

display in red the edge that will be removed

I agree, so I will make a PR.

@ChemicalXandco ChemicalXandco deleted the allow_replace_edge branch June 4, 2021 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants