Skip to content

Minor fixes#1

Merged
muerrilla merged 6 commits intomuerrilla:mainfrom
w-e-w:minor-fixes
May 14, 2024
Merged

Minor fixes#1
muerrilla merged 6 commits intomuerrilla:mainfrom
w-e-w:minor-fixes

Conversation

@w-e-w
Copy link
Contributor

@w-e-w w-e-w commented May 14, 2024


unnecessary


UI fixes

there are ways of making your versions work if you prefer your version but this way is simpler

  • switch to InputAccordion
    using the webui custom gradio component InputAccordion an Accordion with checkbox
    as opposed to using the custom JavaScript activation color change you've created yourself
    image
    circumvents the issue where you have multiple elements of the same ID across different tabs causeing togging the img2img enable will chenge the color of the txt2img accordion

  • use class for css
    use class to specify these styles as opposed to using ID, it's also circumvents the issue where you have multiple elements of the same ID across different tabs


if for some reason in the future or in other extensions you do need elem_id then use self.elem_id('XXX') to create the id


video

before video

2024-05-14.19_25_56_631.chrome.mp4

after video

2024-05-14.19_28_07_192.chrome.mp4


optional change

remove this if you don't like it

  • compact infotext
    theres is no issue with the current implementation but I feel like it's taking up lots of space
    I feel it makes sence to compact all of the info into one key removing the need of adding so many DD_xxxxxx
    note: has backwards compatibility with old format

before PR

DD_enabled: True, DD_amount: 0.5, DD_start: 0, DD_end: 1, DD_bias: 0.5, DD_exponent: 1, DD_start_offset: 0, DD_end_offset: 0, DD_fade: 0, DD_smooth: True

after PR

Detail Daemon: "amount:0.5,st:0,ed:1,bias:0.5,exp:1,st_offset:0,ed_offset:0,fade:0,smooth:1"

if you don't like my abbreviations change it


I think every commit in this PR is Cherry Pickable if you don't like a certain change

@muerrilla
Copy link
Owner

Hey! Thanks a bunch. ❤ There's a lot to learn here for me.

I think every commit in this PR is Cherry Pickable if you don't like a certain change

I absolutely love all of them, but the InputAccordion might have to go in the next update as I explained here to keep it consistent with other extension. But I'm willing to take it for a spin till then and see if it can change my mind. 😁

@muerrilla muerrilla merged commit 02ca0ab into muerrilla:main May 14, 2024
@w-e-w
Copy link
Contributor Author

w-e-w commented May 14, 2024

keep it consistent with other extension

fair poing and I understand what you're saying but I will also like to point out that controlnet is not a good example because they require multiple sub modules on different sub tabs to be active individually
where as your use case is more similar to refine or high-res fix
the reason why you see extensions not use InputAccordion that often is mostly becauls old extension may not want to switch because of backwards compatibility consideration
the ui uses lots of InputAccordion, currently all default modules on the extras tab uses InputAccordion
image

it is possible to make it so that you can choose between InputAccordion or Accordion + checkbox
I have done something similar in the past for backwards compatibility back around the time when InputAccordion was introduced in webui


also another thing
there is a default system in web UI, default values of elements such as sliders will be saved to ui-config.json
customization of default values if they find the initial values provided by the developer is not suitable for their use case
the can do so by edit ui-config.json directly or use settings > defaults to updata the default values

but this system also has a side effect, if a extension developer later on wishes to change the default value of a element they will NOT be able to do so as the values are already written into ui-config.json

is is possible to force the change by changeing the lable of a element because the ID in ui-config.json will also change so will no longer be recognized
I wish we have a better system that allows developer to push changes but also allows user customization at the same time but unfortunately currently is not the case

this is another reason why the default values should be sensible before releasing it to the masses
as it will be written to any one that has installed this extention

it is possible to disable default system this for a specific element if the developer wishes to by adding the do_not_save_to_config attribute the a element

# for single element
gr_element.do_not_save_to_config = True
# for a list of elements
elements = [gr_e1, gr_e2, gr_e3]
[setattr(e, 'do_not_save_to_config', True) for e in elements ]

but I don't recommend you do so because this would remove the user ability to customise them

however you could disable the default system during development phase and re-enable it when you have good default values that can be used by the masses

unless you wish to implement your own default value system then you would want to disable it

@muerrilla
Copy link
Owner

...where as your use case is more similar to refine or high-res fix

True.

the reason why you see extensions not use InputAccordion that often is mostly becauls old extension may not want to switch because of backwards compatibility consideration the ui uses lots of InputAccordion,

This might indeed be me simply resisting change. 😁 I actually prefer the way the InputAccordion is activated, to the old way of opening the accordion and clicking enable. But the color coding is really crucial for me. I have a long list of extensions and I often need to collapse them the avoid scrolling all day, then I kept forgetting which ones where active quite often. So I made myself a neutral grey theme where the only color on screen are the generation and the active extensions, so I couldn't miss them anymore.

Compare Detail Daemon to the other ones:

What do you think about keeping the InputAccordion but coloring the title on activation? I can imagine that such practice on the grander scale of the extension ecosystem can easily turn the UI into myspace (or was it geocities? the memories are fading), but I think I really need this. The checkbox can easily go though.

it is possible to make it so that you can choose between InputAccordion or Accordion + checkbox I have done something similar in the past for backwards compatibility back around the time when InputAccordion was introduced in webui

You mean through the settings tab? I don't know how to do it, but if you did it I would be very happy. This would be ideal solution to the myspace problem.


also another thing there is a default system in web UI

Oh I had never thought about the implications of that.

this is another reason why the default values should be sensible before releasing it to the masses as it will be written to any one that has installed this extention

Good point.

however you could disable the default system during development phase and re-enable it when you have good default values that can be used by the masses

Great tip. I think the new default values are as good as it gets. It's not easy to come up with good universal values for this thing. I think a more robust solution to this problem is providing presets, which is near the top of my todo list.


While we're at it, there seems to be a bug in the new infotext paste functionality. Upon a fresh start or restart, the first time I drop a picture into the prompt box and click the 'read gen params from prompt' button, it works fine. But the times after that it throws this for every DD parameter:

*** Error executing <function Script.ui.<locals>.<lambda> at 0x0000021DFCAE8430>
    Traceback (most recent call last):
      File "E:\A1111\stable-diffusion-webui\modules\infotext_utils.py", line 492, in paste_func
        v = key(params)
      File "E:\A1111\stable-diffusion-webui\extensions\sd-webui-detail-daemon\scripts\detail_daemon.py", line 76, in <lambda>
        (gr_start, lambda d: extract_infotext(d, 'st', 'DD_start')),
      File "E:\A1111\stable-diffusion-webui\extensions\sd-webui-detail-daemon\scripts\detail_daemon.py", line 70, in extract_infotext
        return d['Detail Daemon'].get(key)
    AttributeError: 'str' object has no attribute 'get'

Can you replicate that?

@w-e-w
Copy link
Contributor Author

w-e-w commented May 15, 2024

Compare Detail Daemon to the other ones:

lol

I'm impressed that webui didn't crash with so meny extension


Can you replicate that?

it looks like somehow

def parse_infotext(infotext, params):
try:
d = {}
for s in params['Detail Daemon'].split(','):
k, _, v = s.partition(':')
d[k.strip()] = v.strip()
params['Detail Daemon'] = d
except Exception:
pass

was not executed, very weird
didn't happen to me in my other extensions
let me hava a look later


What do you think about keeping the InputAccordion but coloring the title on activation? I can imagine that such practice on the grander scale of the extension ecosystem can easily turn the UI into myspace (or was it geocities? the memories are fading), but I think I really need this. The checkbox can easily go though.

I think it might be possible to patch in a javascript that change the color on acative for all InputAccordions
I might have a try later and if I manage to do so I might made a standalone extension for those who wants this

don't let me stop you from doing yourself, I'm not sure if I will be able to succeed

lol, I seems to have habit of writing extensions that I don't use myself

@w-e-w
Copy link
Contributor Author

w-e-w commented May 15, 2024

@muerrilla

While we're at it, there seems to be a bug in the new infotext paste functionality. Upon a fresh start or restart, the first time I drop a picture into the prompt box and click the 'read gen params from prompt' button, it works fine. But the times after that it throws this for every DD parameter:

*** Error executing <function Script.ui.<locals>.<lambda> at 0x0000021DFCAE8430>
    Traceback (most recent call last):
      File "E:\A1111\stable-diffusion-webui\modules\infotext_utils.py", line 492, in paste_func
        v = key(params)
      File "E:\A1111\stable-diffusion-webui\extensions\sd-webui-detail-daemon\scripts\detail_daemon.py", line 76, in <lambda>
        (gr_start, lambda d: extract_infotext(d, 'st', 'DD_start')),
      File "E:\A1111\stable-diffusion-webui\extensions\sd-webui-detail-daemon\scripts\detail_daemon.py", line 70, in extract_infotext
        return d['Detail Daemon'].get(key)
    AttributeError: 'str' object has no attribute 'get'

Can you replicate that?

I figured it out this is caused by
remove_current_script_callbacks()

it will work up to you first gen then
remove_current_script_callbacks() removes every callback of your script including parse_infotext that is used to parse the infotext

@w-e-w
Copy link
Contributor Author

w-e-w commented May 15, 2024

Compare Detail Daemon to the other ones:

image
what version are you on?
on my they have switched to input accordion


looked it up, they switch 2 mounts age
pkuliyi2015/multidiffusion-upscaler-for-automatic1111@0d88a38

@w-e-w
Copy link
Contributor Author

w-e-w commented May 15, 2024

What do you think about keeping the InputAccordion but coloring the title on activation

I throw together a quick extension to add this
care to have go?
https://github.com/w-e-w/sd-webui-input-accordion-highlight

@muerrilla
Copy link
Owner

muerrilla commented May 15, 2024

I figured it out this is caused by remove_current_script_callbacks()

Oh I see.

looked it up, they switch 2 mounts age

Mine is (was) from 2024-03-09. lol!

I throw together a quick extension to add this

Oooh this is perfect. 👌
So it didn't even need js? huh. Amazing.

Guess that's bye bye to the old ways then. 😁
Also, time to switch my other extensions to InputAccordion.

@w-e-w
Copy link
Contributor Author

w-e-w commented May 16, 2024

So it didn't even need js? huh. Amazing

yeah CSS is magic, the :has() is a new syntax apparently.
https://developer.mozilla.org/en-US/docs/Web/CSS/:has

this could be done in pure CSS, Python is just there to allow user customization of the colors

if you want to do something more advanced beyond what I allow in the settings, you could ditch the extension and write your own CSS rule into your own user.css

linear-gradient() could look nice with some theme

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.

2 participants