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

[Draft] Model library sidebar tab #837

Merged
merged 40 commits into from
Sep 24, 2024
Merged

Conversation

mcmonkey4eva
Copy link
Member

@mcmonkey4eva mcmonkey4eva commented Sep 15, 2024

Opening a draft PR for @huchenlei 's visibility and comments

This PR adds a sidebar tab to list and manage available models

Current State

  • it properly lists all main model folders, your personal folders within, and the models within
  • It does a nice lil "loading..." thing when you first click into a folder for which the model list isn't populated
  • Has a "(No Content)" for empty folders
  • displays the thumbnails of models next to the model
  • can hover models to get a popup showing some text info about the model
  • can click or drag a model to autospawn a loader node for the model
  • it uses the same tree system from the node library so it handles subfolders and wotnot just fine
  • Has code to compensate between windows & linux filepaths.
  • Has a setting for whether to load all model lists immediately or opening, or only load a list once you click a folder
  • Looks ugly.

modellisty

TODO

  • Pretty it up, naturally
    • figure out why the css is very subtly different from the other sidebar wtf aaaa
    • Load model thumbnail if available and render it (rather than just a lil file icon)
      • slightly hack css but works
    • maybe hide the .safetensors for cleanliness
      • Done
    • Add a preview view on hover, like the node preview, but showing the model's metadata
      • ugly but works
    • add a way to actual interact with the metadata (eg copy the trigger phrase text to put in your prompt box)
  • Make it possible to drag the models out to nodes
    • Done
    • possibly a registry/store of x model type yields y node type? The core models are easy but needs an accessible storage API so custom nodes can register their own model types are handled in specific ways
      • Done
  • actually pull a proper list of folder types from upstream (currently hardcoded to the most common core types)

Maybe Todo Now Or Later

(Things that either can be in this PR or a followup)

  • Favoriting models you like
  • possibly a way to swap what node is used when dragging a model out for the user? eg there's a lot of custom lora loader nodes other than just the default one, so while the "initial MVP valid behavior" is just spawn a LoraLoader node, users will want ways to choose others
  • possibly a way to drag a model onto an existing node to swap the path in the node or something

Definitely Not Todo In This PR

  • Edit model data - that's more complex, should be a future followup
  • Move/rename/delete/etc. the model files - also can be a future followup
  • Metadata related things that require core comfy upgrades first:
    • Filters on common metadata things (eg filter by model architecture class)
    • search by text other than filename (eg search in description text)

src/stores/modelStore.ts Show resolved Hide resolved
src/components/sidebar/tabs/modelLibrary/ModelTreeLeaf.vue Outdated Show resolved Hide resolved
src/App.vue Outdated
@@ -100,6 +101,14 @@ const init = () => {
component: markRaw(NodeLibrarySidebarTab),
type: 'vue'
})
app.extensionManager.registerSidebarTab({
id: 'model-library',
icon: 'pi pi-folder-open',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can probably pick another icon. Maybe pi-box?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah wasn't sure what icon would work here. Swapped to box

const children = node.children?.map(fillNodeInfo)
const model: ComfyModelDef | null =
node.leaf && node.data ? node.data : null
if (model && model.name.startsWith('\0')) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a isLoading getter on ComfyModel should make logic here more explicit.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ngl every version of this code will be a hack other than directly adding the loading entry to the tree instead of passing a fake def, but that adds significant code complexity for the sake of dodging the hack.

But yeah I'll add a bool on the modeldef to at least mark it a bit more properly than this nullchar indicator

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessary adding a new field, you just need to add a getter that returns the expression model.name.startsWith('\0')

@huchenlei
Copy link
Member

@mcmonkey4eva Gentle ping on the investigation of why calling api/models/{xxx} super slow.

@mcmonkey4eva
Copy link
Member Author

mcmonkey4eva commented Sep 17, 2024

In the current case I saw the other day and showed in that gif, I believe it may actually just be npm run dev has a rather slow proxy, the delay only happens on 5173 (dev server) but not on a direct call to 8188 (the comfy server). ie on actual deployed version it will not be so slow at all. (Though I have seen the comfy builtin webserver being quite slow to pickup calls in the past)

(well it will be slow if you have a lot of models but when I'm not on a laptop in tokyo I'll do some loadtesting and PR some aggro caching to comfy core, and maybe some fancy fastloading of file data too if python will let me get away with it)

@mcmonkey4eva mcmonkey4eva marked this pull request as ready for review September 22, 2024 13:26
@mcmonkey4eva
Copy link
Member Author

added a partial revert of 5a5a69d which was submitted directly to main without review and shockingly enough broke things

@huchenlei
Copy link
Member

added a partial revert of 5a5a69d which was submitted directly to main without review and shockingly enough broke things

Can you explain why the revert is necessary?

@mcmonkey4eva
Copy link
Member Author

mcmonkey4eva commented Sep 23, 2024

surprise unreviewed commit broke the format in a way that doesn't make sense (text just yeets off the edge of the screen) and caused problems for this PR

For reference here's what that commit did that I reverted here:
image

the model library was even worse off than that

@mcmonkey4eva mcmonkey4eva changed the base branch from main to dev1.3 September 23, 2024 09:39
@mcmonkey4eva
Copy link
Member Author

Retargeted at dev1.3 per request in today's sync, seems to be working as intended locally

@huchenlei huchenlei merged commit 08ba155 into dev1.3 Sep 24, 2024
9 checks passed
@huchenlei huchenlei deleted the model-library-sidebar-tab branch September 24, 2024 00:48
huchenlei pushed a commit that referenced this pull request Sep 24, 2024
* basic/empty model library sidebar tab

in-progress

* make it actually list out models

* extremely primitive search impl

* list out available folders

(incomplete list atm)

* load list dynamically

* nice lil loading icon

* that's not doing anything

* run autoformatter

* fix up some absolute vue shenanigans

* swap to pi-box

* is_fake_object

* i think apply the tailwind thingo

* trim '.safetensors' from end of display title

* oop

* after load, retain title if no new title is given

* is_load_requested to prevent duplication

* dirty initial model metadata load & preview

based on node preview code

* update model store tests

* initial image icon for model lib

* i hate this

* better empty spacer

* add api handler for '/models'

* load model folders list instead of hardcoding

* add a 'no content' placeholder for empty folders

* autoformat

* autoload model metadata

* error handling on metadata loading

* larger model icons

* click a model to spawn a node for it

* draggable model nodes

* add a setting for whether to autoload or not

* autoformat will be the death of me

* cleanup promise code

* make the model preview actually half-decent

* revert bad unchecked change

* put registration back
huchenlei pushed a commit that referenced this pull request Sep 25, 2024
* basic/empty model library sidebar tab

in-progress

* make it actually list out models

* extremely primitive search impl

* list out available folders

(incomplete list atm)

* load list dynamically

* nice lil loading icon

* that's not doing anything

* run autoformatter

* fix up some absolute vue shenanigans

* swap to pi-box

* is_fake_object

* i think apply the tailwind thingo

* trim '.safetensors' from end of display title

* oop

* after load, retain title if no new title is given

* is_load_requested to prevent duplication

* dirty initial model metadata load & preview

based on node preview code

* update model store tests

* initial image icon for model lib

* i hate this

* better empty spacer

* add api handler for '/models'

* load model folders list instead of hardcoding

* add a 'no content' placeholder for empty folders

* autoformat

* autoload model metadata

* error handling on metadata loading

* larger model icons

* click a model to spawn a node for it

* draggable model nodes

* add a setting for whether to autoload or not

* autoformat will be the death of me

* cleanup promise code

* make the model preview actually half-decent

* revert bad unchecked change

* put registration back
huchenlei pushed a commit that referenced this pull request Sep 25, 2024
* basic/empty model library sidebar tab

in-progress

* make it actually list out models

* extremely primitive search impl

* list out available folders

(incomplete list atm)

* load list dynamically

* nice lil loading icon

* that's not doing anything

* run autoformatter

* fix up some absolute vue shenanigans

* swap to pi-box

* is_fake_object

* i think apply the tailwind thingo

* trim '.safetensors' from end of display title

* oop

* after load, retain title if no new title is given

* is_load_requested to prevent duplication

* dirty initial model metadata load & preview

based on node preview code

* update model store tests

* initial image icon for model lib

* i hate this

* better empty spacer

* add api handler for '/models'

* load model folders list instead of hardcoding

* add a 'no content' placeholder for empty folders

* autoformat

* autoload model metadata

* error handling on metadata loading

* larger model icons

* click a model to spawn a node for it

* draggable model nodes

* add a setting for whether to autoload or not

* autoformat will be the death of me

* cleanup promise code

* make the model preview actually half-decent

* revert bad unchecked change

* put registration back
@807502278
Copy link

807502278 commented Sep 27, 2024

Excuse me, is the model library now finding the thumbnails and names of models through the Internet? I find that the names of many models are not the names of my local models, and can't read preview images with the same name as the model. Can a setting be added to switch between reading locally or obtaining from the Internet?

The authors of many model names write them casually. If so, I will change it, but it still has its original name in the model library.
image

@mcmonkey4eva
Copy link
Member Author

mcmonkey4eva commented Sep 28, 2024

@807502278 no it's not through the internet, it's the metadata in the header of the model file - so whatever title pops up is what the author typed in as the title at some point, and has been sitting inside your model the entire time you've had it.

Re showing the raw filename as an option instead of title, that's added in this pending PR: #1005

In the coming future we'll add tools to edit metadata freely, in the meantime you can use SwarmUI to edit the metadata (also the swarmui downloader can autoload better metadata from civit than what's baked into models by default if you give it a url)

@807502278
Copy link

Thanks for telling me. It turns out to be the read model metadata. For each model I downloaded, the url and preview image are saved. I am very much looking forward to a tool that can freely edit metadata : )

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

Successfully merging this pull request may close these issues.

3 participants