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

Feature: Extra Networks Tree View #14588

Merged

Conversation

Sj-Si
Copy link
Contributor

@Sj-Si Sj-Si commented Jan 8, 2024

Latest Update

Most recent major update to this PR: #14588 (comment)

Description

This PR adds an option to the Extra Networks section to switch from the "Card View" to a "Tree View". This allows users to easily navigate more complex directory structures in order to find networks.

Along with this, a new button has been added to both the existing Cards in the extra networks tabs and to the entries in this new tree view. This button (shown in the screenshot) can be clicked to copy the file path of the network to the clipboard.

Finally, if an Extra Networks tab's corresponding root directory (in stable-diffusion-webui/models) is empty, that directory will remain expanded and display the message DIRECTORY IS EMPTY (see the screenshots below).

I made these changes after getting sick of the existing Card View format for these networks. They become too cumbersome when you have many networks in a more complex directory structure.

Screenshots/videos:

Shown below is the new entry in the Extra Networks section of the settings page. This setting is off by default so the user will have to actually go and change this setting to see any of the changes in this PR.

pr_screenshot_1

The following screenshot shows what the Extra Networks tabs look like with this new change enabled in the settings.

pr_screenshot_0

The folders that appear in these tabs are collapsed by default and can be clicked to expand.

Additionally, this is the new Copy Path button that was added to the existing network cards.
pr_screenshot_2

Checklist:

Additional Notes:

The Search and Sort options are not implemented in the tree view. If anyone wants to tackle that beast then be my guest.

This is pretty low priority and I really just made it for my own use anyway so no rush.

@Sj-Si
Copy link
Contributor Author

Sj-Si commented Jan 8, 2024

Should I be resolving conflicts with the dev branch? Or should I only be resolving conflicts against the master branch?

@AUTOMATIC1111
Copy link
Owner

Only dev. PRs are to be merged with the dev. People generally don't resolve conflicts, I do that, but if you do, that would help.

@Sj-Si
Copy link
Contributor Author

Sj-Si commented Jan 9, 2024

Only dev. PRs are to be merged with the dev. People generally don't resolve conflicts, I do that, but if you do, that would help.

Understood. I'll resolve the conflicts.

It could be a load off your shoulders if you require the PR creator to resolve conflicts though... :)

@AUTOMATIC1111
Copy link
Owner

Now, as for the feature itself, I'm not a big fan of having a second mode for extra networks page so that the user chooses to either have cards with pictures, or a folder tree. I feel a better solution is if a tree is added (if enabled in settings), into the main mode with cards: only directories are shown in the tree, and clicking on the directory sets the search filter to only show cards from this directory. Do you have any thoughts about this way to implement the tree?

@Sj-Si
Copy link
Contributor Author

Sj-Si commented Jan 9, 2024

Now, as for the feature itself, I'm not a big fan of having a second mode for extra networks page so that the user chooses to either have cards with pictures, or a folder tree. I feel a better solution is if a tree is added (if enabled in settings), into the main mode with cards: only directories are shown in the tree, and clicking on the directory sets the search filter to only show cards from this directory. Do you have any thoughts about this way to implement the tree?

I think I get what you're asking for. Were you thinking maybe something like GitHub's Files Changed window where the file explorer is on the left side and the cards could be on the right?

tmp

Personally I think that would be the cleanest looking option. Could also chuck that search bar at the top of the file explorer like github did.

I'll work on a quick demo of this solution and get back to you.

@AUTOMATIC1111
Copy link
Owner

AUTOMATIC1111 commented Jan 9, 2024

Yes. Thanks. With the exception that github's tree also shows files, and we would just show directories - three in your screenshot, or four if you include root.

@w-e-w
Copy link
Collaborator

w-e-w commented Jan 9, 2024

oh nice after all this time someone finally decide actually work on this tree view

I think the extra network path should be truncated to its extra network directory, no need to display the absolute path

@Sj-Si
Copy link
Contributor Author

Sj-Si commented Jan 10, 2024

oh nice after all this time someone finally decide actually work on this tree view

I think the extra network path should be truncated to its extra network directory, no need to display the absolute path

Which of these options would you prefer (using the Lora directory as an example)?

  1. stable-diffusion-webui/models/Lora
  2. models/Lora
  3. or just Lora

@AUTOMATIC1111

One issue I'm running into is that the current card filter will display all cards in the filtered directory and its subdirectories.

I'm not really sure how we'd like to handle this, though. My preference would be to only show cards in that directory and not in any subdirectories but I can also see some arguments for showing all of it.

The other issue is I don't know any javascript which is making this take longer than I'd hoped. :\

@w-e-w
Copy link
Collaborator

w-e-w commented Jan 11, 2024

fourth option, none
I think it shouldn't even have lora at all
like you're already on the lora tab, so you are in the lora dir whitch means lora is the root

also if you are using a custom dir with --lora-dir there's no guarantee that what directory name is it will be in let alone that theres exist a upper level dir
(yes most likely that will be a upper level directory but if someone wants they can put all the models under system root)

@Sj-Si
Copy link
Contributor Author

Sj-Si commented Jan 11, 2024

fourth option, none I think it shouldn't even have lora at all like you're already on the lora tab, so you are in the lora dir whitch means lora is the root

also if you are using a custom dir with --lora-dir there's no guarantee that what directory name is it will be in let alone that theres exist a upper level dir (yes most likely that will be a upper level directory but if someone wants they can put all the models under system root)

If someone is putting their models at system root they deserve an error haha. Good point though I didn't think about that. My concern is that the Lora tab looks inside of both the Lora and LyCORIS directories by default. What if they have a subdirectory in each of those directories with the same name? So models/Lora/one and models/LyCORIS/one. Then they would just have two folders showing up that look the same until they navigate into them; it just feels a bit odd.

Also the textual inversion tab looks in the embeddings directory by default. I think at the very least we should inform the user what directory these things are in. If the models are at root then it would just show /.

@w-e-w
Copy link
Collaborator

w-e-w commented Jan 11, 2024

My concern is that the Lora tab looks inside of both the Lora and LyCORIS directories by default. What if they have a subdirectory in each of those directories with the same name? So models/Lora/one and models/LyCORIS/one

irrc if you have two models of the same name at the same relative position in both Lora and LyCORIS only one will work (I forgot which one has priority)
so it is a non issue or at least this will only be an issue if extra network is updated to support this

also the LyCORIS dir is left for compatibility reasons only (you don't have to use it and I think it's better you don't use it)

if this tree view can display multiple networks from different roots with the same relative position and name then one can consider this to be a bug because extra networks doesn't support it
(I don't think we even support if you have multiple lora under the same name across subdirectories I believe only one will be used)

basically unless non-unique names and multiple root is supported by the extra network the tree should not display the networks that can't be used

@Sj-Si
Copy link
Contributor Author

Sj-Si commented Jan 11, 2024

Okay I've updated the format a bit. Now the tree view is on a left pane alongside the cards. The filters work and when you click on directories it will use that as the search filter. Clicking on a directory will expand that directory and use that directory as a search filter for the right side of the pane. Clicking an already opened directory will just select that directory but leave it expanded. To collapse a directory, you just click that directory again if it is already selected.

Currently if your search is a directory then all networks within that directory and subdirectories will be shown. This is because the search is just checking for a substring in the search terms and relative paths are included in the search terms for each network.

I also updated the code so that switching between tabs clears the search box. Having the search filter stay the same when switching between tabs was annoying since the models don't have the same search terms.

I also decided to keep the networks in the directory tree since I personally find it much quicker to just use the tree view to select my models.

It's still unpolished because I don't know any html/css/js so this is about as good as I can get it within a reasonable time.

One thing I would have liked to see is the search textbox with an x in it to clear the search but I couldn't find a way to do that with a gr.Textbox.

Below is a quick demo video showing how clicking on directories updates the search box.

2024-01-11.15-02-36.-.Copy.mp4

The code still definitely needs to be cleaned up. I also need to resolve conflicts with dev.

@w-e-w

also the LyCORIS dir is left for compatibility reasons only (you don't have to use it and I think it's better you don't use it)

In the modules/ui_extra_networks.py::ExtraNetworksPage class, we are retrieving the root directories using the self.allowed_directories_for_previews() function. ui_extra_networks_lora.py overrides this function to return [shared.cmd_opts.lora_dir, shared.cmd_opts.lyco_dir_backcompat]. So that's the only reason that directory shows up in the tree view at this time.

@Sj-Si
Copy link
Contributor Author

Sj-Si commented Jan 13, 2024

Hey so I'm running into an issue that I cant seem to resolve.

In extraNetworks.js I want to register event handlers for various elements in the page. However, when I do gradioApp().querySelector("#my_id") it cannot find any elements with that ID. But when I run that command in the browser console I do get elements in the result. So it seems like the extraNetworks.js file is being run prior to the html actually being built.

Am I correct in this assumption and is there any way to have a .js file that runs after the html is generated?

@w-e-w
Copy link
Collaborator

w-e-w commented Jan 14, 2024

Am I correct in this assumption and is there any way to have a .js file that runs after the html is generated?

I think one of these callbacks might work for you

var uiUpdateCallbacks = [];
var uiAfterUpdateCallbacks = [];
var uiLoadedCallbacks = [];
var uiTabChangeCallbacks = [];
var optionsChangedCallbacks = [];

@Sj-Si
Copy link
Contributor Author

Sj-Si commented Jan 14, 2024

Am I correct in this assumption and is there any way to have a .js file that runs after the html is generated?

I think one of these callbacks might work for you

var uiUpdateCallbacks = [];
var uiAfterUpdateCallbacks = [];
var uiLoadedCallbacks = [];
var uiTabChangeCallbacks = [];
var optionsChangedCallbacks = [];

Thank you that helped. I wasn't able to use any of these predefined callbacks but I was able to figure out a solution using a similar approach to uiAfterUpdateCallbacks. What I found is that the content within the extra networks tabs doesn't finish loading before uiLoadedCallbacks. The uiAfterUpdateCallbacks should work but it gets called on every single update to the page (clicking tabs, buttons, etc.) so I don't really want to use that either.

var uiAfterScriptsCallbacks = []
var uiAfterScriptsTimeout = null;
var executedAfterScripts = false;

function scheduleAfterScriptsCallbacks() {
    clearTimeout(uiAfterScriptsTimeout);
    uiAfterScriptsTimeout = setTimeout(function() {
        executeCallbacks(uiAfterScriptsCallbacks);
    }, 200);
}

document.addEventListener("DOMContentLoaded", function() {
    var mutationObserver = new MutationObserver(function(m) {
        if (!executedAfterScripts &&
            gradioApp().querySelectorAll("[id$='_extra_search']").length == 8) {
            executedAfterScripts = true;
            scheduleAfterScriptsCallbacks();
        }
    });
    mutationObserver.observe(gradioApp(), {childList: true, subtree: true});
});

uiAfterScriptsCallbacks.push(setupExtraNetworks);

This method waits for all of the elements matching *_extra_search to finish getting generated on the page before it sets up the extra networks tabs. I wish there were some better way to handle this than by checking if specific elements finish loading. Maybe adding a dummy element somewhere on the page that we can guarantee will always be loaded last could be a solution though it feels a bit hacky for my taste. I'm also not a fan of hardcoding the length check in gradioApp().querySelectorAll("[id$='_extra_search']").length == 8) but it works at the moment since I know that there will always be 4 of these elements generated in each of the txt2img and img2img tabs.

@Sj-Si
Copy link
Contributor Author

Sj-Si commented Jan 20, 2024

@AUTOMATIC1111

The diff is too big to be reviewed properly... I'll probably have to #yolo merge it, which I always regret.

Just let me know if there is anything I can do to help make the review process easier for you. I really don't like putting this much work on your plate, especially if you're the only one reviewing these things and you're kinda just doing this all pro bono in the first place.

Anyway, in the current state, extra_networks_card_order_field/extra_networks_card_order settings are ignored, which they shouldn't be.

Good catch. I just added those back in. Completely missed those last time.

There is also an issue of this being always enabled. For people who do not have have a rich directory structure, this is just a clear degradation because it takes 1/4 of space without being particularly useful. There should be an option of not including the tree view.

Added an option in settings (along with a fancy new button) for this. The option in settings enables/disables tree view by default on launch (the default value for this setting is disabled). The new button allows the user to easily toggle the tree view.

When tree view is disabled, tree-list-controls elements should still be available - I suggest you put them where they were in that case, into the row with tabs (it's possible to still keep unique tree-list-controls per page like you implemented it - just place one into the location when a tab is selected) (I also have to add that I really like how it looks, neat ,small, and properly aligned).

I moved the controls outside of the tree view. I was also thinking it didn't really feel right to have them in that tree view div.

Tree View Disabled:
image
Tree View Enabled:
image

Sorry if it feels like I'm pushing back a lot on these controls but I just feel like having them inside each tab looks cleaner. It also made some of the code a bit easier to separate as well.

One thing that I am unsure of is whether the icons for the sort mode and sort direction make their function clear enough without having to hover over them to view the tooltip. Like it makes sense to me but I don't really know what the average user of this UI is like...

Additionally:

If you feel like it, there is working horizontal resize system, which works, like in txt2img tabs you can make the left column smaller or bigger. You may be able to reuse that for your purposes with treeview, if you'd like.

At one point I did look into reusing that component but it looked like I'd only be able to use it if I added it via python using the ResizeHandleRow class. I still haven't really figured out how to properly mix the custom HTML/CSS with these python components so I kinda gave up on that.

@AUTOMATIC1111
Copy link
Owner

I was thinking more about something like this - to not take an extra row.

firefox_Y975U7Nq0x

@Sj-Si
Copy link
Contributor Author

Sj-Si commented Jan 21, 2024

@AUTOMATIC1111
Honestly because of the way that I set up the tree view pane, I don't think I would be able to move those buttons up there while keeping as many of the features as they have. Like... I'm sure it's possible but it would take a massive amount of work for me to figure out since those html elements live outside of the tab divs.

@Dasor92
Copy link

Dasor92 commented Feb 28, 2024

is there a way to roll back having folders? When you have more than 2000 loras the new tree view is unusable. Also, the "change preview" button doesn't update automatically.

@tsjslgy
Copy link

tsjslgy commented Mar 2, 2024

This is a controversial feature, I like to train lora myself, so there are different directories to divide and I know very well the location and role of each folder, the previous version can click on the folder and go directly to the directory, but now you have to click on the tree network several times to enter, very troublesome. Of course, I can only represent individuals, so if you need to add this function, please also add a button that does not enable this function, thank you

kaalibro added a commit to kaalibro/stable-diffusion-webui that referenced this pull request Mar 5, 2024
All changes are current up to eee46a5

extensions-builtin/Lora/ui_extra_networks_lora.py
Rollback to d4945f4

html/extra-networks-card.html
Rollback to 699108b

javascript/extraNetworks.js
Rollback to a2f23f9

modules/shared_options.py
Remove options:
- "extra_networks_tree_view_default_enabled"
- "extra_networks_tree_view_default_width"
- "extra_networks_card_description_is_html"

modules/ui_extra_networks.py
Rollback to 320a217
Apply 321b2db

modules/ui_extra_networks_checkpoints.py
Rollback to 337bc4a

modules/ui_extra_networks_hypernets.py
Rollback to 74b80e7

modules/ui_extra_networks_textual_inversion.py
Rollback to 74b80e7

modules/ui_extra_networks_user_metadata.py
Rollback to 5d7d182

style.css
Rollback to 2f98a35
Apply 9f3ba38..1466dae
kaalibro added a commit to kaalibro/stable-diffusion-webui that referenced this pull request Mar 7, 2024
All changes are current up to 58f7410

• extensions-builtin/Lora/ui_extra_networks_lora.py
Rollback to d4945f4

• html/extra-networks-card.html
Rollback to 699108b

• javascript/extraNetworks.js
Rollback to a2f23f9
Apply 801461e..ecffe85

• modules/shared_options.py
Rollback to 1ddb886
Apply 569dc19..e3fa46f
Remove unnecessary options:
- "extra_networks_tree_view_default_enabled"
- "extra_networks_tree_view_default_width"
- "extra_networks_card_description_is_html"

• modules/ui_extra_networks.py
Rollback to 320a217
Apply 321b2db

• modules/ui_extra_networks_checkpoints.py
Rollback to 337bc4a

• modules/ui_extra_networks_hypernets.py
Rollback to 74b80e7

• modules/ui_extra_networks_textual_inversion.py
Rollback to 74b80e7

• modules/ui_extra_networks_user_metadata.py
Rollback to 5d7d182

• style.css
Rollback to 2f98a35
Apply 9f3ba38..1466dae
@kaalibro kaalibro mentioned this pull request Mar 7, 2024
4 tasks
@pawel665j pawel665j mentioned this pull request Apr 16, 2024
@Sj-Si Sj-Si deleted the feature/extra-networks-tree-view branch April 16, 2024 20:13
ruchej pushed a commit to ruchej/stable-diffusion-webui that referenced this pull request Sep 30, 2024
ruchej pushed a commit to ruchej/stable-diffusion-webui that referenced this pull request Sep 30, 2024
ruchej pushed a commit to ruchej/stable-diffusion-webui that referenced this pull request Sep 30, 2024
ruchej pushed a commit to ruchej/stable-diffusion-webui that referenced this pull request Sep 30, 2024
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.

6 participants