-
-
Notifications
You must be signed in to change notification settings - Fork 283
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
Add support for AUTOMATIC1111's extras - image upscaling #1692
Conversation
Sorry for the late reply, but thanks for submitting this. @adodge is going to look it over when he has time. As for the |
Sorry for the delay here too! I think we should slightly restructure our inputs before we tackle the output type. In the webui, there are 2 modes: "Scale by" and "Scale to". So let's do that too. Let's define another enum for those 2 states. class ScaleMode(Enum):
SCALE_BY = 0
SCALE_TO = 1 We can then use an self.inputs = [
ImageInput(),
EnumInput(ScaleMode, default_value=ScaleMode.SCALE_BY).with_id(1),
if_enum_group(1, ScaleMode.SCALE_BY)(
SliderInput(
"Relative resize multiplier",
minimum=0,
default=4,
maximum=8,
slider_step=0.1,
controls_step=0.1,
precision=1,
),
),
if_enum_group(1, ScaleMode.SCALE_TO)(
NumberInput("Width", controls_step=1, default=512),
NumberInput("Height", controls_step=1, default=512),
BoolInput("Crop to fit", default=True),
),
...
] |
backend/src/nodes/nodes/external_stable_diffusion/extras_single_image.py
Outdated
Show resolved
Hide resolved
Very cool! Thank you for this! I have a few notes:
It looks like what it does is some number (maybe zero) of the following operations (I believe in this order): A. Upscales with one upscaler Maybe we could have separate nodes that each do one of these steps: Upscale, GFPGAN, and CodeFormer. That might make it easier to understand what they're doing. It's also more modular and node-ish. Another point that I don't really have a strong opinion on but is probably worth noting: I believe all of these things are already supported natively by ChaiNNer. I can think of a few scenarios where someone would want to use this instead of the built-in chainner nodes:
Once we have internal SD, assuming we keep the webui integration, we'll have sort of retroactively established a precedent for webui nodes that duplicate built-in behavior, so it's not automatically a deal-breaker for me. The scenarios above are compelling reasons to have this node, IMO. |
Thank you for an in-depth review and tons of references to look up. I appreciate this PR might not make sense. As @adodge mentioned: "all of these things are already supported natively by ChaiNNer." To elaborate on the use case: I wanted to use LDSR for upscaling in the middle of a pipeline, and LDSR specifically because it gives me the best results. The context is that webui/LDSR works quite well, I tried SDv2 superresolution and it gave mediocre results (for my use case), so I have a feeling someone tuned that LDSR model pretty hard. Webui implementation looks pretty hacked though, and I have a feeling it'd be hard to actually run in chaiNNer :) Let me know if I should continue a bit further. I'm not advocating either way, happy for you to close this PR - I've done my thing by having it hacked in :) |
Actually, I don't see LDSR on the list of things ChaiNNer supports, so I take it back that it's all stuff we already have. For me the only sticking point is figuring out how we want to translate it into nodes (one or several) and how to handle picking from the models the user has installed. I'm certainly not arguing for closing the PR. I think this would be useful. (As it was useful for you, it would probably be useful for others too.) |
Ok I'll take this PR one step further, I think I'll just reduce it to Upscaling via webui, because GFPGAN and CodeFormer are already supported via pytorch. |
I think querying that endpoint is the best option |
I've followed your suggestions and done the following (see the animated gif too):
|
Nice! I get a Navi error on load:
Probably need more aggressive sanitization of the upscaler name. I'm not sure what the set of valid names is. Maybe we could just hash the names to hex, prepend an "x" so it doesn't start with a number, and use that as the ID in Navi. That seems safe. I was going to test what happens when we load a chain that references an upscaler that doesn't exist. I don't know what it should do, but we should make sure it does something reasonable. |
Also we should call "DynamicEnum" something more specific, in case we want to use this strategy in other places. (like to specify models for the text-to-image node) |
Awesome work @mateusz!
Oh god. I didn't know that it was possible to dynamically create enums like this. (I should probably add a check to |
In that case, |
@adodge I haven't run into error while testing and I'm just wondering how I missed it - is there anything specific I need to do before
Oh true, missed that, fixed.
Converted to SNAKE_CASE. |
This will need to be updated/rebased with the new node importing system btw |
I probably have a different set of upscalers from you installed in webui. I think the one it was complaining about was called The name of the model is based on whatever the filename is on the user's machine, so we should try to be resilient to all sorts of possible strings. |
Yes, it doesn't like them. I think the easiest solution here would probably be to use a regex to replace all |
ccf0f96
to
16c4785
Compare
Another push:
|
backend/src/packages/chaiNNer_external/external_stable_diffusion/automatic1111/upscaling.py
Outdated
Show resolved
Hide resolved
16c4785
to
0de1f7c
Compare
backend/src/packages/chaiNNer_external/external_stable_diffusion/automatic1111/upscaling.py
Outdated
Show resolved
Hide resolved
backend/src/packages/chaiNNer_external/external_stable_diffusion/automatic1111/upscaling.py
Outdated
Show resolved
Hide resolved
0de1f7c
to
ab8f3c9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't test this, since I don't have AUTOMATIC1111's webui installed. So someone that has it and has used it before, please test the new node.
@joeyballentine @adodge
The code itself looks good.
Hey folks - just wanted to mention, don't feel bad if you want to reject and close the PR, if it doesn't suit the philosophy of chaiNNer. I don't mind, really. It was fun to hack something together :) |
No you're good, i fully intend to merge this, I just forgot to test it last night. |
I'm guessing this probably doesn't have to do with your code here, and is probably due to me using a somewhat older version of the webui, but if its at all fixable we should probably try to do something about it |
Similar problem here, although only with SwinIR model . Possibly something to do with dependencies for webui, maybe update pip deps? Do you get the same error when you try to use the UI directly? Unless you mean we should handle webui errors better in chaiNNer? |
I just wasn't sure if there was maybe some config thing wrong. But yeah it just looks like a webui issue. So I'm good with calling this good |
Found this bit of AUTOMATIC1111's webui quite useful, and wanted to use it in the interface. "Single Image" is the only bit that's really necessary - your tool can deal with batching via iterators.
Note I tried implementing dynamic size and asserts, but the logic is very convoluted and I couldn't figure it out - and I struggled with the syntax of
ExpressionJson
. The logic is dependent on multiple inputs - if upscaler_1 is None, output is the same shape, if absolute rescaling is on, then shape is different, if clip is on the output is assymetric, plus some rounding to deal with. I tried multiple versions but couldn't figure it out :(View of Webui's interface for extras
View in ChaiNNer