How to apply the current version LG to ComfyUI? #22
Comments
|
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) |
|
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. |
|
Agree. Does your webpack solution work to make a bundle and include that? |
|
It was my attempt. But it doesn't work. I'll start from here. Make the current LG work at least partially with ComfyUI. |
|
Currently, that bundle only works with this HTML file: |
|
To understand how ComfyUI uses LG, I've compared three files:
1 is the current LG working with ComfyUI by default, and it is the newest version (the latest commit was 2 months ago). Though the filenames are different, So, if I copied back ComfyUI's |
|
Now, if I copied all the Javascript files in -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.jsand I modified the // 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
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> |
|
But there's compatibility issue. For example, here in ComfyUI's And it causes the bug when clicking the mouse button, the 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: There are many other compatibility issues... any thoughts? |
|
Hi liusida, thanks for this, and nice hack by the way. |
|
There are maybe two ways to make it possible for people to use our improved LG when using ComfyUI:
So maybe you are aiming for the first one, right? |
|
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. |
|
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. |
|
Found and fixed some issues. |
|
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
<#22 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AC36LN3VDF4M3HOKHS4QZFDZHHEBNAVCNFSM6AAAAABJDOFKJSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRWGIYDMMRXHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
I think we have a problem with forward having everything converted to classes. |
|
Tried with option 1, just for ContextMenu, that is the one being called often. Seems working fine. Haven't tried yet to include my recent extensions: autoconnect, keyboard navigation and ramer |
|
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
<#22 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AC36LN3M5BHQTU54C23JPHDZHH5JNAVCNFSM6AAAAABJDOFKJSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRWGY4TKMJWGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
we don't need to consider all custom nodes (extensions), I have a list of top-100 😆 https://github.com/liusida/top-100-comfyui/ |
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. |
|
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. |
|
pushed a working test to my ComfyUI fork 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) |
|
started a web_alpha branch, added a --web-directory parameter, will use that for the server instead |
|
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
<#22 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AC36LN7QYVF24ECXOPPYUZDZHWVT7AVCNFSM6AAAAABJDOFKJSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNZRG42DIMJSGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
just pushed |
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
The custom nodes do all kinds of unexpected things... xD |
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?
The text was updated successfully, but these errors were encountered: