Allow any valid node with no incoming connections and with side effects to run automatically

chaiNNer-org / chaiNNer

A node-based image processing GUI aimed at making chaining image processing tasks easy and customizable. Born as an AI upscaling application, chaiNNer has grown into an extremely flexible and powerful programmatic image processing application.

https://chaiNNer.app

GNU General Public License v3.0

4.49k stars 280 forks source link

Allow any valid node with no incoming connections and with side effects to run automatically #2944

Closed joeyballentine closed 3 months ago

joeyballentine commented 3 months ago

Refactored useRunNode a bit, moving some of the qualifications into the hook
Changed the definition of starting node to be any valid node without an incoming connection. This means we can run far more nodes ahead of time now. The cache is properly cleared once a node has an incoming connection.

This is necessary to be able to allow connections to Load Image. (which I will do in a separate future PR)

RunDevelopment commented 3 months ago

This needs to be an opt-in system, I think. We have nodes like Create Noise that can potentially take multiple seconds to run. I think the a1111 web UI Text to Image node is also valid without incoming connections. Having nodes like this run every time you change an input isn't a good idea.

RunDevelopment commented 3 months ago

And about correctness: isStartingNode now isn't correct anymore, since it doesn't consider the incoming connections of a node. You need to review every usage of this function.

joeyballentine commented 3 months ago

That's a valid concern. But also, I think you misread this PR. I didn't change the actual isStartingNode function. In fact, I don't even use it for this calculation anymore.

joeyballentine commented 3 months ago

Having nodes like this run every time you change an input isn't a good idea.

I thought about this for a bit, and I'm wondering if this is actually true. We're basically just running any node we can run individually ahead of time, which will end up saving a bit of time when the entire chain is run. I don't really see a downside.

In fact, I almost think these nodes (create noise, create gradient, etc) should have a large image output like load image, so you can create the image and visualize it without having to re-run the chain constantly to see it.

joeyballentine commented 3 months ago

One thing to note is that since this doesnt actually change the isStartingNode function, the hook for doing those fast type tags won't run for these anymore, so I'm thinking I probably should change it to be done there instead.

As for implementation, I'm thinking of making "starting nodes" have side effects (technically all of them do anyway since they display some kind of information after being run. The idea is that since these nodes need to run, and we run them automatically, they have side effects. Limiting this to just nodes with side effects would mean we can opt-in to this feature in an easier way. Thoughts?

joeyballentine commented 3 months ago

I got rid of the isStartingNode function and replaced it with a new hook that determines whether "automatic" features should be used. Then, i made it so that the load nodes have side effects, and use side effects to help determine whether the node should run. Technically, these nodes do have side effects, so it was kinda incorrect before anyway.

I still would kinda like to have more nodes run automatically, but i think i'll just give those nodes large image outputs in a separate PR and give them side effects.

joeyballentine commented 3 months ago

Going to merge this as I need it for what i'm going to work on next. Feel free to make PRs for any further suggestions