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

I've made a new feature for the for reaching the most frequently used node faster in client-side, but I don't know where should I PR to. #735

Open
liusida opened this issue Jun 2, 2024 · 2 comments

Comments

@liusida
Copy link
Contributor

liusida commented Jun 2, 2024

Description of the new feature:

image

image

The right-click context menu currently only contains a static list of nodes, which often isn't very helpful. To address this, I've implemented a feature that traces the creation of all nodes. When building the right-click context menu, the top three most frequently created nodes will be listed after the 'Search' section.

This functionality is implemented entirely in JavaScript, with the information stored in localStorage.getItem("nodeSelectionFrequency") as a dictionary.

For the proof of concept, I modified litegraph.core.js, adding about 20 lines of code, and it works well.

However, I've noticed that comfyanonymous/ComfyUI rarely modifies this file; it is only updated when changes are made upstream. The original jagenjo/litegraph.js has not been updated for five months, and it appears they might be planning a feature-less rewrite of the code before adding new features.

Considering this, I am wondering if I should implement this feature in ComfyUI-Manager by using injection to modify the behavior of the original JavaScript functionality.

===

FYI, I'll attach the code I've added for a proof-of-concept:

// in LGraph.prototype.add:
        // Everytime created a node, update the frequency
        function generateNodeKey(node) {
            let type = node.type;
            let inputs = node.inputs ? node.inputs.map(input => input.type).join(",") : "";
            let outputs = node.outputs ? node.outputs.map(output => output.type).join(",") : "";
            return `${type}|${inputs}|${outputs}`;
        }

        var selectionFrequency = JSON.parse(localStorage.getItem("nodeSelectionFrequency") || "{}");
        let key = generateNodeKey(node);

        if (!selectionFrequency[key]) {
            selectionFrequency[key] = 0;
        }
        selectionFrequency[key] += 1;
        localStorage.setItem("nodeSelectionFrequency", JSON.stringify(selectionFrequency));
// in LGraphCanvas.prototype.showConnectionMenu:
        function findTopFrequentNodes(isFrom, fromSlotType, topN = 3) {
            var selectionFrequency = JSON.parse(localStorage.getItem("nodeSelectionFrequency") || "{}");
            let candidates = [];
        
            for (let key in selectionFrequency) {
                let [type, inputs, outputs] = key.split("|");
                if (isFrom ? inputs.includes(fromSlotType) : outputs.includes(fromSlotType)) {
                    candidates.push({ type: type, frequency: selectionFrequency[key] });
                }
            }
        
            // Sort candidates by frequency in descending order
            candidates.sort((a, b) => b.frequency - a.frequency);
        
            // Return the types of the top N candidates
            return candidates.slice(0, topN).map(c => c.type);
        }        

		// get defaults nodes for this slottype
		var fromSlotType = slotX.type==LiteGraph.EVENT?"_event_":slotX.type;

        var topFrequentNodes = findTopFrequentNodes(isFrom, fromSlotType, 3);  // Get top 3 frequent nodes
        if (topFrequentNodes.length) {
            topFrequentNodes.forEach(nodeType => {
                options.push(nodeType);
            });
            options.push(null);
        }
@atlasan
Copy link

atlasan commented Jun 2, 2024

Nice feature!
I would try to push those to LiteGraph.slot_types_default_out and see if that is enough. ^_^

Regarding this and many other features implementation, I'm currently working on a solution to callbacks that should enable multi extensions to works together allowing to register and return data for the same event without overriding the original (and others) behaviors. Stay tuned.

@liusida
Copy link
Contributor Author

liusida commented Jun 2, 2024

Nice feature! I would try to push those to LiteGraph.slot_types_default_out and see if that is enough. ^_^

Regarding this and many other features implementation, I'm currently working on a solution to callbacks that should enable multi extensions to works together allowing to register and return data for the same event without overriding the original (and others) behaviors. Stay tuned.

I see. So I probably should be working on this file:
https://github.com/comfyanonymous/ComfyUI/blob/master/web/extensions/core/slotDefaults.js

And I think I'll need a callback from the creation of nodes to update the default list, so I should create a static function LGraph.onNodeAdded(node) to get the callback... Let me try it out. Thanks.

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

No branches or pull requests

2 participants