How to apply the current version LG to ComfyUI?

[liusida]

I've tried to replace this file: https://github.com/comfyanonymous/ComfyUI/blob/master/web/lib/litegraph.core.js with this file: https://github.com/daniel-lewis-ab/litegraph.js/blob/master/build/litegraph.core.js

Though the filename is the same, it doesn't work.

What is the correct procedure to apply the current version LG to ComfyUI?

[atlasan]

I think the inclusion in ComfyUI has to be revised: it could be just the single litegraph.core probably built with your webpack solution. The thing about new features is that some of those has to be enabled (in the repo there is a defaults.js file that sets some of those, the defaults_debug.js enable some more 'cause some are still WIP and because I think it's always better to increment possibilities without replacing old behaviors) The way I see many features is actually implementing those as extensions, like already done in ComfyUI. Those are temporary in an extension folder and needs to be included in the js bundle or included aside. I image the end user could choose if and which of those has to be loaded, as choosing some of the options that we could expose for LiteGraph and LGraphCanvas. The way to do so is yet to be determined. We could implement that with localStorage and so load everything and allow to choose options from a deedicated panel maybe. Or leave that in a config file. Thoughts?

[liusida]

Let's set aside any configuration or control panel for now.

I think the key part is to be compatible with current ComfyUI, so we can manually replace files or folders to enactivate the new version LG.

Once we can do it manually, we can think of a way to automate the process.

[atlasan]

Agree. Does your webpack solution work to make a bundle and include that?

[liusida]

It was my attempt. But it doesn't work. I'll start from here. Make the current LG work at least partially with ComfyUI.

[liusida]

Currently, that bundle only works with this HTML file: https://github.com/liusida/litegraph.js/blob/daniel-master/examples/use-bundle.html

[liusida]

To understand how ComfyUI uses LG, I've compared three files:

1. https://github.com/comfyanonymous/ComfyUI/blob/master/web/lib/litegraph.core.js

2. https://github.com/comfyanonymous/litegraph.js/blob/master/src/litegraph.js

3. https://github.com/jagenjo/litegraph.js/blob/master/src/litegraph.js

1 is the current LG working with ComfyUI by default, and it is the newest version (the latest commit was 2 months ago). 2 is Comfy's attempt to fix some of the bugs, and the fix will be pulled into ComfyUI's repo, but the modifications in ComfyUI's repo aren't pulled back to this repo. 3 is the original LG, which Comfy copied the original code from.

Though the filenames are different, litegraph.js and litegraph.core.js have the same content.

So, if I copied back ComfyUI's litegraph.core.js into the original LG and replace the src/litegraph.js, the original LG works perfectly.

[liusida]

Now, if I copied all the Javascript files in forwardcompatible branch into ComfyUI's web/lib/ folder, the target folder will look like this (I've backed up ComfyUI's litegraph.core.js into litegraph.original.js):

-a---            6/6/2024  8:50 PM          11978 callbackhandler.js
-a---            6/6/2024  8:50 PM          31161 contextmenu.js
-a---            6/6/2024  8:50 PM           8424 dragandscale.js
-a---            6/6/2024  8:50 PM          60270 lgraph.js
-a---            6/6/2024  8:50 PM         343436 lgraphcanvas.js
-a---            6/6/2024  8:50 PM           4625 lgraphgroup.js
-a---            6/6/2024  8:50 PM          96569 lgraphnode.js
-a---           6/13/2024 11:37 AM           1507 litegraph.core.js
-a---           6/13/2024 10:03 AM         505948 litegraph.original.js
-a---           5/11/2024  8:15 AM          13780 litegraph.css
-a---           5/11/2024  8:15 AM            545 litegraph.extensions.js
-a---           6/13/2024 11:23 AM          46053 litegraph.js
-a---            6/6/2024  8:49 PM           2050 llink.js

and I modified the litegraph.core.js into a hack like this:

// litegraph.core.js

const script = document.createElement('script');
script.type = 'module';
script.textContent = `
    import { LiteGraph } from './lib/litegraph.js';
    import { LGraph } from './lib/lgraph.js';
    import { LLink } from './lib/llink.js';
    import { LGraphNode } from './lib/lgraphnode.js';
    import { LGraphGroup } from './lib/lgraphgroup.js';
    import { DragAndScale } from './lib/dragandscale.js';
    import { LGraphCanvas } from './lib/lgraphcanvas.js';
    import { ContextMenu } from './lib/contextmenu.js';

    // Attach components to the global window object for global access
    window.LiteGraph = LiteGraph;
    window.LGraph = LGraph;
    window.LLink = LLink;
    window.LGraphNode = LGraphNode;
    window.LGraphGroup = LGraphGroup;
    window.DragAndScale = DragAndScale;
    window.LGraphCanvas = LGraphCanvas;
    window.ContextMenu = ContextMenu;

    // Function to load extensions script
    function loadExtensions() {
        return import('./lib/litegraph.extensions.js');
    }

    // Load extensions script after attaching components to window object
    loadExtensions().then(() => {
        console.log('Extensions loaded successfully');
    }).catch(err => {
        console.error('Failed to load extensions:', err);
    });

    // Export as ES6 module
    export { LiteGraph, LGraph, LLink, LGraphNode, LGraphGroup, DragAndScale, LGraphCanvas, ContextMenu };

`;
document.head.appendChild(script);

Without any modification in HTML file, ComfyUI will start use the forwardcompatible branch.

If we want to change to the default ComfyUI litegraph temperarily, we can change the line in HTML into:

        <script type="text/javascript" src="./lib/litegraph.original.js"></script>

[liusida]

But there's compatibility issue. For example, here in ComfyUI's web/extensions/core/contextMenuFilter.js, there's a redefine of contextMenu:

https://github.com/comfyanonymous/ComfyUI/blob/605e64f6d3da44235498bf9103d7aab1c95ef211/web/extensions/core/contextMenuFilter.js#L10

And it causes the bug when clicking the mouse button, the contextMenu.closeAll() function is not found.

I'm not sure why the original ComfyUI code works with such a refefination....

(ok, I've found out why, ComfyUI redefine another function for this purpose: LiteGraph.closeAllContextMenus(ref_window); so ComfyUI throws away the original contextMenu and redfines it...)

There are many other compatibility issues... any thoughts?

[atlasan]

Hi liusida, thanks for this, and nice hack by the way. I was trying a different approach: making another .html file that includes our files instead of release ones, finally the result will be the same: we have to integrate into LG the hacks and changes that have been made in ComfyUI to adapt LG or fix or simplify LG in a way that those re-definitions are not needed anymore. ContextMenu is one of the pieces, but we have Groups too and surely many more. I will try to look into this in parallel.

[liusida]

There are maybe two ways to make it possible for people to use our improved LG when using ComfyUI:

1. make an upgraded UI for ComfyUI, replacing its web directory completely.
2. make a file that can replace litegraph.core.js.

So maybe you are aiming for the first one, right?

[atlasan]

The second one would be optimal and clean, but considering that some of the actual web/lib "extensions" in ComfyUI could/should be upgraded/modified I would opt for the first one, this would allow to easily have a pre-release UI along the stable one. Actually I was trying a third one: use the same web folder but with a different .html file.. this way there's no need for any configuration and versions could works seamlessly together. For this I was considering a folder inside /lib like lib/forward where to put all related files. I was trying this but struggling a bit with modules inclusions.. not used to and not sure how we should generally address it. Probably the direct override of methods of LGraphCanvas and so could be revised. How should things be ideally? Maybe @daniel-lewis-ab too has some thoughts about this.

[liusida]

Yeah. For solution one, I've heard about someone is using pure react-based javascript to rewrite the UI. https://github.com/canisminor1990/kitchen-comfyui (but I am not sure about React, since it is much slower)

However, solution one provides more flexibility, and I heard that Comfy himself is willing to see more UIs working with his backend Python via HTTP API. But one of the problems I can think of is to rewrite the current extensions. (or we can just copy them and follow the GPL-3.... ) (maybe it can be think as solution 3?)

If we go with solution two, I think we should take all the existing Javascript files into account, and basically make LG in that environment, so later if new LG works well, Comfy might suddenly decide to pull our code.

[atlasan]

Found and fixed some issues. Now canvas background renders correctly but a call to ContextMenu like a function brake clicking. Found https://github.com/digital-flowers/classy-decorator this but can't make it working and feels like another hack on hack. Without replacing the extension is maybe not that easy to fix, but anyway extensions should be revised, so probably a new web folder is finally the right answer and a ComfyUI setting to choose between them.

[daniel-lewis-ab]

I put forward a non-breaking reversion for them to pull.

Its es5, and doesn't break anything yet afaik. I am thinking to push changes as we figure out how to make them non-breaking and resolve issues.

On Thu., Jun. 13, 2024, 10:45 atlasan, @.***> wrote:

Found and fixed some issues. Now canvas background renders correctly but a call to ContextMenu like a function brake clicking. Found https://github.com/digital-flowers/classy-decorator this but can't make it working and feels like another hack on hack. Without replacing the extension is maybe not that easy to fix, but anyway extensions should be revised, so probably a new web folder is finally the right answer and a ComfyUI setting to choose between them.

— Reply to this email directly, view it on GitHub https://github.com/daniel-lewis-ab/litegraph.js/issues/22#issuecomment-2166206279, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC36LN3VDF4M3HOKHS4QZFDZHHEBNAVCNFSM6AAAAABJDOFKJSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRWGIYDMMRXHE . You are receiving this because you were mentioned.Message ID: @.***>

[atlasan]

I think we have a problem with forward having everything converted to classes. ComfyUI core extensions, and who know how many custom extensions, replace ContextMenu as a function, but that doesn't work with it being a class. That will happen for all the other classes too. Option a: rename all actual classes and restore original name as a function Option b: revert to simple functions I can't see any other, we can't just brake 1/2 extensions been made for ComfyUI

[atlasan]

Tried with option 1, just for ContextMenu, that is the one being called often. Seems working fine. Going to push to mi ConfyUI fork my actual testing. If you want to try it check index_forward, while index_atlasan had some of my previous working and index should work as usual.

Haven't tried yet to include my recent extensions: autoconnect, keyboard navigation and ramer

[daniel-lewis-ab]

Thats encouraging. I think ill copy you and do the same for the ES6 migration when that's the step.

Might try Proxy class for ContextMenu for now?

On Thu., Jun. 13, 2024, 14:20 atlasan, @.***> wrote:

Tried with option 1, just for ContextMenu, that is the one being called often. Seems working fine. Going to push to mi ConfyUI fork my actual testing. If you want to try it check index_forward, while index_atlasan had some of my previous working and index should work as usual.

— Reply to this email directly, view it on GitHub https://github.com/daniel-lewis-ab/litegraph.js/issues/22#issuecomment-2166695165, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC36LN3M5BHQTU54C23JPHDZHH5JNAVCNFSM6AAAAABJDOFKJSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRWGY4TKMJWGU . You are receiving this because you were mentioned.Message ID: @.***>

[liusida]

I think we have a problem with forward having everything converted to classes. ComfyUI core extensions, and who know how many custom extensions, replace ContextMenu as a function, but that doesn't work with it being a class. That will happen for all the other classes too. Option a: rename all actual classes and restore original name as a function Option b: revert to simple functions I can't see any other, we can't just brake 1/2 extensions been made for ComfyUI

we don't need to consider all custom nodes (extensions), I have a list of top-100 😆 https://github.com/liusida/top-100-comfyui/

[liusida]

Option a: rename all actual classes and restore original name as a function

I think this might be a good idea so people can migrate gradually and willingly. So if node authors want their users to use their new custom nodes with new features in LGv1, they can tell their users to download LGv1 as a requirement.

[daniel-lewis-ab]

I mean, right now it's completely non-breaking for reversion branch, so so far it's not required. There'll come a time when we start caring, but it's not yet.

Most of what we've been up to is stuff I'm seeing as v2 stuff. Adding coolness to move the world forward. v1 is going to be just straight code upgrades and boring as all heck. I'm wanting to rip through the v1 upgrades as fast as possible so I can catch the world up to what you two are up to.

[atlasan]

pushed a working test to my ComfyUI fork things apparently working

using default index.html at now to better test

can try F2 to rename, 'a' to autoconnect, arrows to move node, shift/cntrl + rightArrow to create new destination node, 'tab' to select prev next node

looking forward to branch again using a different web directory instead of modifying original code (at now only a small change has been needed, couldn't get working onKeydown replacement without a small mod: instead of replacing processKey function a CBHandler has been used)

[atlasan]

started a web_alpha branch, added a --web-directory parameter, will use that for the server instead

[daniel-lewis-ab]

Interesting. We does the web directory branch do, curious what the pattern is - if its already on chat, ignore this and I'll see it when I get home.

On Sun., Jun. 16, 2024, 09:31 atlasan, @.***> wrote:

started a web_alpha branch, added a --web-directory parameter, will use that for the server instead

— Reply to this email directly, view it on GitHub https://github.com/daniel-lewis-ab/litegraph.js/issues/22#issuecomment-2171744120, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC36LN7QYVF24ECXOPPYUZDZHWVT7AVCNFSM6AAAAABJDOFKJSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNZRG42DIMJSGA . You are receiving this because you were mentioned.Message ID: @.***>

[atlasan]

just pushed That branch is on my ComfyUI fork, I've implemented the current LG working at alpha stage, seems working fine. Can run ComfyUI with _--web-directory webalpha it will use _webalpha folder instead of web Works better than having a different .html because of some implication and give freedom to rethink the WebUI Unique thing I've noticed couple custom_nodes use "web" const directory to do stuff. Nothing too serious for the moment.

[liusida]

That branch is on my ComfyUI fork

Which branch to be exact? Can you quote the branch name for me?

[update] I found it: https://github.com/atlasan/ComfyUI_atlasan/tree/web_alpha

Unique thing I've noticed couple custom_nodes use "web" const directory to do stuff.

The custom nodes do all kinds of unexpected things... xD