Skip to content

Conversation

StAlKeR7779
Copy link
Contributor

No description provided.

Copy link
Collaborator

@lstein lstein left a comment

Choose a reason for hiding this comment

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

LGTM

@psychedelicious
Copy link
Collaborator

Added the controlnet field to node editor, and your draft does seem to work fine.

One thing we want to do is associate controlnet models to processors - like canny model defaults to canny preprocessor. I think this association should probably be in the frontend, rather than introducing a new concept to the backend.

Any thoughts or concerns there?

Once we decide how to proceed with that, I can get the linear UI controlnet updated.

@lstein
Copy link
Collaborator

lstein commented Jul 10, 2023

Added the controlnet field to node editor, and your draft does seem to work fine.

One thing we want to do is associate controlnet models to processors - like canny model defaults to canny preprocessor. I think this association should probably be in the frontend, rather than introducing a new concept to the backend.

Any thoughts or concerns there?

Once we decide how to proceed with that, I can get the linear UI controlnet updated.

Any resolution on this decision point?

@maryhipp
Copy link
Collaborator

maryhipp commented Jul 11, 2023

Just pushed up some changes for linear workflow - @psychedelicious I actually couldn't figure out how your node implementation was working (I couldn't get network calls to trigger and it seems to use some phantom list of constants that I can't find in the code), so I'm sorry if I broke it.

My changes:

  • removed hard-coded list of control net models based on discord conversation
  • updated controlnet model component to pull list from backend
  • updated controlnet model component to disable models that have incompatible base models with selected main model
  • updated processor logic to make an educated guess of processor based on substrings (I am sure this can be improved)
  • moved processor + autoconfigure toggle to be further up in the UI since we are more likely to not get default correct
  • fixed logic to clear submodels across the board when main base model changes (still would like to make the modal to confirm before doing this)

@psychedelicious psychedelicious force-pushed the feat/model_mgr_controlnet branch from 69b4558 to 69ac89c Compare July 15, 2023 02:19
@psychedelicious psychedelicious changed the title [WIP] Rewrite controlnet to new model manager Rewrite controlnet to new model manager Jul 15, 2023
@psychedelicious
Copy link
Collaborator

feat(ui): fix controlNet models

  • update controlnet state to use object format for model
  • update model-parsing helper functions to log errors
  • update nodes components, types and state
  • remove controlnets from state when models are loaded and the controlnet's model is not available

@blessedcoolant
Copy link
Collaborator

blessedcoolant commented Jul 15, 2023

This PR is good to go. Maybe just changing the assertion of the control_weight to take values up to 2 instead of 1 and updating the linear UI slider to be 0, 1, 2 with 1 as default. But I'll let @psychedelicious @GreggHelt2 make the final call on that one.

Everything else seems to be working fine. Great job guys.

Oh .. the annotator models download to HF cache on first load. Maybe this should be handled by the installer?

@psychedelicious psychedelicious force-pushed the feat/model_mgr_controlnet branch from 86a3dc8 to e58c14f Compare July 15, 2023 08:50
StAlKeR7779 and others added 15 commits July 15, 2023 19:56
…. add disabled state if base model is not compatible. clear control net model if main base model changes. add logic to guess processor and move it up in UI
- update controlnet state to use object format for model
- update model-parsing helper functions to log errors
- update nodes components, types and state
- remove controlnets from state when models are loaded and the controlnet's model is not available
…ectors

makes them more portable and easier to reason about
…disabled

The UX is clearer now, but it's still easy to miss that your individual controlnets are enabled, but the overall controlnet feature is disabled.
It is the most impactful feature, and also takes up the most space when you expand it. Promoted.
@psychedelicious psychedelicious force-pushed the feat/model_mgr_controlnet branch from 0493e2f to 457e4b7 Compare July 15, 2023 09:56
@psychedelicious psychedelicious force-pushed the feat/model_mgr_controlnet branch from 093fb64 to 8e0ba24 Compare July 15, 2023 10:41
@psychedelicious
Copy link
Collaborator

  • Updated layout
  • No more single line layout. It's mini or full
  • Did some minor styling tweaks, like fixing spacings and stuff
  • Better error handling in reducers and overall logic
  • fix disabled state for mantine select components
  • Range is 0 - 1 - 2
  • It was not clear when a specific controlnet is enabled, because its settings are always enabled. Disable a controlnet's controls if the controlnet itself is disabled. It's still not totally clear, because the overall controlnet's enabled toggle is easy to miss, but at least this is a step in the right direction.
  • Promote controlnet to be just under general. It's the most impactful feature (and takes up the most space in the panel) so it deserves to be right up there.
Screen.Recording.2023-07-15.at.8.49.50.pm.mov

@lstein lstein merged commit 07a2da4 into main Jul 15, 2023
@lstein lstein deleted the feat/model_mgr_controlnet branch July 15, 2023 12:24
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.

5 participants