Added Detail Daemon
Info
Here's the repo. And here's the original reddit discussion.
Checklist:
- [ ] I have read the
Readme.md - [ ] The description is written in English.
- [ ] The
index.jsonandextension_template.jsonhave not been modified. - [ ] The
entryis placed in theextensionsdirectory with the.jsonfile extension.
tested seems to work without breaking issue
but some minore issue regarding the JavaScript and elem_id
and there is one thing that's better taking care of before it's listed on index and used by the masses
- sensible default values at least from my brief experience with your extensions the default values are... not usable, it could be the 1.5 model I am testing with but, but for me it the default Amount of 0.5 is way to high, something like 0.1 is already a lot and images just end up garbled adjusting all the values may change things but regardless I think it makes more sense to give new users a more sensible default values for them to start with otherwise it would look bad for extension
from your readme
I'll write up some proper docs on how best to set the parameters, as soon as possible.
understandable but similar to above if users don't understand how to use this properly, it looks bad for extension
if a default values are more sensible this is less crucial, as users have a good starting position for them to experiment
the graph color changing based on the amount doesn't help either I'm not sure if your intentions is if good values then would be green and bad values will be red if this is your intention then that is not good enough and actually cause more confusion than it does help I was just disabling the colors until you have a better grass of what is a good value
some minore issue UI JavaScript and elem_id
-
most extension accordions are collapsed by default you have it opened I suggested to collapse consistency sake
-
you have chosen to hardcoately active color, this is fine when you're using the default theme but it won't look as good if you're using a something else, it's best to use a color defined by the gradio theme so it changes based on the theme that the user is using
-
you have set a static elem_id for some of your components but since your extension exist both on the txt2img and img2img tab which means IDs will be duplicated (it cannot be) which means will automatically generate random IDs and causing your JavaScript stuff to break assuming that tab order is default you can see that your active toggle only works on txt2img tab and not img2img tab
| txt2img | img2img |
|---|---|
works on txt2img but not on img2img, color dose not match the custom theme
the current best practice is to use
scripts.Script.elem_id() to automatically generate prefixes for the element ID so that it doesn't conflict with elements of other extensions or different instances on different tabsor multiple instances across different tabs
- there exist a custom element in web UI called
InputAccordionthis it the Accordion with a checkbox that is used by hires-fix refine and other and all the modules maybe instead of coding the color toggling active JavaScript you maded just use theInputAccordionwhen uses clicksInputAccordionit expands and the checkbox is automatically checked and disable when flapsed, note the checkbox can be manually toggle if the user wishes to, this allows it to be enable when collapsed
| collasped | expaned |
|---|---|
last this is just a personal opinion
personally I found the amount of DD_xxxxx: var in infotext is a bit annoying, it is possible to compact it into one dictionary
note there is nothing wrong with your current infotext, I just personally don't like it
I made a fix PR for some parts
- https://github.com/muerrilla/sd-webui-detail-daemon/pull/1
my main concern is sensible default values
if you don't mind this and want to release it to the masses
then I will merge it
Hey! Thanks for the in depth review. Most of your points are spot on. I think it's better if you withhold from merging it until my coming update.
sensible default values
Very true. Actually I also discovered that if the fix is only applied to the uncond latent, the results will be much more controllable and less destructive (especially for new users). So I'm gonna use that mode as default, and set better default parameters in the coming update.
I'll write up some proper docs on how best to set the parameters, as soon as possible.
I've already updated the readme, adding some thorough explanation of how the extension works. But actual (theory-free) examples that can work as quick tutorials for beginners will have to wait till I have some free time. So I'd say this is not very high priority, when the default values are fine as you also suggested.
the graph color changing based on the amount doesn't help either I'm not sure if your intentions
The graph is yellow at zero, then yellow to green when the mean of the adjustment amount in all steps is positive (detail will be increased) and yellow to red when it's negative (detail will be decreased). But perhaps using a pair of colors with less existing semantic baggage (green = good, red = bad) is a better idea.
- most extension accordions are collapsed by default you have it opened
Oops! This was intended for dev stage. Forget to turn it back to closed by default.
- you have chosen to hardcoately active color
Good point, will fix.
- you have set a static elem_id for some of your components...
I have already fixed this in the coming update by using elem_classes instead, so the duplicates can be handled correctly.
the current best practice is to use
scripts.Script.elem_id()
Oooh I didn't know that exists. Awesome. So the way to do it is elem_id=self.elem_id("dd-accordion") ?
- there exist a custom element in web UI called
InputAccordion
This is nice and handy, but I prefer the UX to be consistent with other extensions (ControlNet, FreeU, etc. all do the "expand accordion then check enable" thing). I also need to keep the color toggling to keep it consistent with my other personal extensions and theme. But I'll make the color theme-based.
last this is just a personal opinion personally I found the amount of
DD_xxxxx: varin infotext is a bit annoying, it is possible to compact it into one dictionary
I totally agree. [damn! I just saw this 👇]
I made a fix PR for some parts
Oh man! Is this why the say you should always read the whole thing before you hit reply? What a pleasant surprise! Let me check it out.
set to draft for now message me when you think this extension is ready for the masses
@muerrilla do you think it's ready to be listed?
@muerrilla hello?
@muerrilla hello?
Hey there! Sorry, the day job suddenly got very busy. Let's hold it a few days till I can pin down the best default values, and complete the readme (hopefully by th end of the weekend) and then we're good to go.
ahh ok and no worries no pressure take your time
I was a bit confused and weren't certen that I should merge this because I saw you change the default value back when we are trying to solve the other issues and since you didn't make any changes since I was worried that we had a miscommunication that you think you already said it's good to go but I thought it wasn't
any updates?
any updates?
Hey there! Sorry for disappearing again and keeping you waiting on this. I changed the default values a bit. Did tons of testing but couldn't come up with the perfect values. 😆 Also changed the both mode a bit (it depends on the cfg scale now) so it breaks compatibility with the previous versions. I was planning on adding some cfg rescale functionality to it before release, since it seems to be essential for fixing the burning/wash-out effect, but couldn't find the free time to do it.
So I'd say it's good enough to be listed as is. I'll add those features (and better docs!) when I can. Thanks.
@muerrilla
haha found more major bug
hires fix enabled + batch count > 1
hopefully the last
- https://github.com/muerrilla/sd-webui-detail-daemon/pull/4
I change the tag
removed editing
doesn't seem to fit
https://github.com/AUTOMATIC1111/stable-diffusion-webui-extensions/blob/98a63bc927b0b080a58e17d1662275985333dde1/tags.json#L12-L13
denoising and scheduler manipulation is squarely in stable diffusion operation so manipulation tag
I change the tag removed
editingdoesn't seem to fithttps://github.com/AUTOMATIC1111/stable-diffusion-webui-extensions/blob/98a63bc927b0b080a58e17d1662275985333dde1/tags.json#L12-L13
denoising and scheduler manipulation is squarely in stable diffusion operation so
manipulationtag
oh! Guess I had missed the "not" in that sentence. 🤦♂️