-
Notifications
You must be signed in to change notification settings - Fork 27k
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
Compact infotext system, Compact soft inpainting infotext #14731
base: dev
Are you sure you want to change the base?
Conversation
I personally like the verbose style... |
it doesn't have to be acronyms
I'm just using this to illustrate this potential
I will argue grouping everything that's related to this extension into one object is more clearer readable
|
Copying my comment from the Discord:
|
I don't see the point of writing the information twice |
I would deeply like infotext to be json, and so expandable and standardised. I did a quick look in discussion but couldn't see if this had been suggested or not ? Any reason not to ? I'm pretty sure comfy ui already does it. Would also, possibly be a nice way to ensure that no extensions could break the prompt as you could make all extensions follow a standard or just give an extension it's own blob ? That might make it so that png to prompt might be easier to populate extension boxes too ? |
≥ note: I've also remove space after comma and colon You also can remove quotes |
You also can remove quotes ' and save extra 12 characters 🤪 no not really |
You can check if it contains anything except letters and add quotation if yes |
I have to comment here to second this notion. I personally think it would be really beneficial for the entire infotext to be encoded as JSON. I often find myself writing scripts that parse the infotext and currently my method is to shoehorn the current format into valid YAML, then convert YAML -> JSON at which point I can use the JSON easily anywhere. I haven't had much issue doing this so far, but it's extremely brittle and every extension that I install has the potential to break my parsing logic at any time. |
It could be made opt in, it could be named parametersjson and live side by side. I suspect that parameters will require more troubleshooting and maintenance than parametersjson. The main thing is that a1111 can write json to the file and can read it. |
Description
in some cases the infotext amount is getting a bit out of hand
particularly in the cases where an extension has multiple parameters that needs to be individually entered into Intel text
as infotext key name is required to be unique across all webui the common practice in this case for extension is to add a prefix or suffix to the key name
but this has the issue of making the infotext excessively long
when using this built-in extension you will get an infotext like this
to solve this issue I implemented a method that I named
info_json
basically it goes back to the idea of using structural data, and as the name suggests json
the basic idea is that if all these information can be encoded as a dictionary within "One infotax entry"
this will remove the need of using multiple long prefixes and enable the possibility of using acronyms as sub keys in the dictory
the data structure will will have to be encoded befor combineing into infotext and decode back to data structure on parse
there's one problem with using Json string directly in info text, due to how infotax is encoded the double quotes in json string will have to be escaped, this will cause extra clutter in the info text defeating the purpose compacting intotext
to solve this issue double quotes in single quotes are swap
implementation and demonstration this system with Soft Inpainting
to utilize this system an only have to register a infotext key as a
info_json_key
by usinginfotext_utils.register_info_json('Soft Inpainting')
after this when writing
p.extra_generation_params
the extension can directly write a alist
ordict
in to infotextp.extra_generation_params
the system will automatically encode the infotext to info_json and decode it in
parse_generation_parameters
in the implementation of this PR I've compacted the Soft Inpainting infotext to this
Soft inpainting enabled
is is signified by the existence of `Soft Inpainting: "{...}"Schedule bias
->sb
Preservation strength
->ps
Transition contrast boost
->tcb
Mask influence
->mi
Difference threshold
->dt
Difference contrast
->dc
while using acronyms does reduce readability of the infotext, but in general people won't be reading this in a text manually, and when they do even with the full name they still have to have knowledge about webUI elements so I think using acronyms is acceptable
note: I've also remove space after comma and colon in json string maybe this is a bit overkill this can be reverted or made as an option?
this system is actually relatively easily achievable in extensions without implementing it in web UI
I have been using this in multiple extensions
the only difference is that, I have to perform the encoding myself and then add an additional decoding step to
on_infotext_pasted
callback,but if this system is implemented directly into web UI it would be more easy for future extensions to adapt to the new system
example
Screenshots/videos:
current infotext
compacted with info_json
as you can see lots of space is saved
other changes
consolidated the concatenating of infotext in
modules.processing
andmodules.postprocessing
toinfotext_utils.build_infotext(dict)
if this PR is accepted I would like to also perform the same procedure to some other infotext
like
Checklist: