chrisgoringe / cg-use-everywhere

Apache License 2.0
355 stars 28 forks source link

[ Bug ] Groups collect nodes while being dragged ( performance issue ? ) #175

Closed JorgeR81 closed 3 months ago

JorgeR81 commented 3 months ago

On some complex workflows, groups collect nodes while being dragged.

What I mean, is that the outside nodes will be added to the group, and placed at random positions, where they made contact with the group ( e.g. see what happens if I drag the ControlNet2 group over ControlNet1 ).

If I convert to real links, this this will not happen.

This seems to happen more in groups with UE? nodes, that also have group restriction or priority boost. If these are deleted, the issue tends to be fixed.

But, if the workflow also has Fast Group Muters ( rgthree ) nodes, they will also need to be removed, for this issue to be fixed, even after converting to real links.

So this could be a more broad Comfy UI performance issue, caused by multiple circumstances ( nodes suites ) !?

push

chrisgoringe commented 3 months ago

Looks like something fishy in the comfy group node code - I'll look into it!

chrisgoringe commented 3 months ago

OK - I can reproduce this with fairly simple workflow with a UE? with a group regex; it doesn't seem to be consistent, and there are no errors in the console. Will dig a bit!

chrisgoringe commented 3 months ago

Recording my analysis and fix here in case it applies to other custom nodes (like rgthree Fast Muter?)

When a group is created, it's _nodes property isn't set. It only gets set when the group is moved. This is noted in the Comfy code here: groupOptions.js:86 // Group nodes aren't recomputed until the group is moved,. The logic is that when you start dragging you need to work out what nodes are going to move with you; at other times LiteGraph doesn't care.

Because of that, UE? nodes, when assessing whether to link, call group.recomputeInsideNodes() to ensure they have an up-to-date list.

But if that happens while the group is being dragged, and the group has been dragged over another node, that node is calculated as in the group and starts moving with it.

The solution, fortunately, is simple. If a group is moving, don't recalculate what's in it.

            if (!app.canvas.selected_group_moving) group.recomputeInsideNodes();
JorgeR81 commented 3 months ago

I opened an issue at rgthree and it's fixed now.

But I still have this issue with UE?, in a more specific case.

Now the issue only happens if there are UE? nodes with group restriction, and these nodes are enabled.

If it's group is muted or the UE? nodes with group restriction nodes are bypassed, this issue does not happen.

chrisgoringe commented 3 months ago

Ooops. Should be fixed now.

JorgeR81 commented 3 months ago

It's working, thanks !