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

[Feature Request]: Snap all group borders do grid, when Fit Group to Nodes is used #1185

Open
1 task done
JorgeR81 opened this issue Oct 9, 2024 · 3 comments · May be fixed by Comfy-Org/litegraph.js#362
Open
1 task done
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@JorgeR81
Copy link

JorgeR81 commented Oct 9, 2024

Is there an existing issue for this?

  • I have searched the existing issues and checked the recent builds/commits

What would your feature do ?

Since snap to grid is now in the core, I'm opening here a feature request, that I also have in the extension repo:

pythongosssss/ComfyUI-Custom-Scripts#261

It's a small improvement to make snap to grid work better with groups.

This would allow to snap all group borders to the grid, when Fit Group to Nodes is used ( see last image ).

Proposed workflow

Now groups can also snap to the grid, when dragged, which is great !

But this works in a different way than the Fit Group to Nodes option.


When I use Fit Group to Nodes, the group's bottom border it's aligned with the grid ( but not the top one ).

before_m


And after moving the group, it's the top border that's snapped to grid ( not the bottom one ).
So, the node vertical position is changed in relation to the group.

The nodes can be displaced a little up or down in the group ( see the next 2 images ).

after

after2


So after moving, I still need to redimension the bottom border manually, to snap it to the grid, like this:

fit

Once the group's top and bottom borders are aligned to the grid, the node's relative positions will always remain the same, after moving the group again.

But every time I use Fit Group to Nodes the group's top border will be misaligned again.

A solution would be to snap all group borders to the grid, when Fit Group to Nodes is used, like on the last image.

Additional information

No response

@JorgeR81 JorgeR81 added the enhancement New feature or request label Oct 9, 2024
@HaydenReeve
Copy link

This one drives me insane. I actually thought I might have mucked something up on my end but I'm glad that it's not that.

Hopefully, this one isn't too hard to fix either, since I think the change should effectively be to respect the fit group padding px with the snapping when using any type of resize.

@JorgeR81
Copy link
Author

@webfiltered, now that "snap to grid" is fixed, can you take a look at this ?

When "snap to grid" is enabled, we want the group to also snap to the grid, if we use Fit Group To Nodes.

@webfiltered
Copy link
Contributor

This should be pretty straight forward. Just need the existing snap logic to be assessed after running the fit function - which is all in in the one place, now.

@webfiltered webfiltered added the good first issue Good for newcomers label Nov 30, 2024
@webfiltered webfiltered linked a pull request Nov 30, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants