Skip to content
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

Update Resilience + SD.Next Support #1

Merged
merged 5 commits into from
Aug 29, 2023
Merged

Conversation

DrCyanide
Copy link
Contributor

As requested in Interpause's version, here's a pull request of the changes I made to make the extension more resilient to changes made by A1111 or SD.Next. I'm going to copy and paste my description from that fork's pull request here just as a way of keeping everything documented in one place.


I've added code to check what the associated backend actually requires for it's txt2img and img2img calls, then pass only those parameters into wrap_gradio_gpu_call(). If a parameter is not in the dictionary of known parameters, the code will look at the datatype and provide a "default" value for that parameter, making the extension more likely to get an image back than crash. Also added console warnings when parameters are missing, giving their name and datatype, making it easier for users to maintain future updates.

Due to the way this update grabs parameters, it works with both Automatic1111 and Vlads/SD.Next backends (and will likely work with any forks of those).

This update also makes the extension drastically more forwards and backwards compatible. Updated versions of the extension won't crash on older UIs, and updated UIs won't crash this version of the extension as easily.

I know @Interpause is looking to get out of maintaining this repo. I think this update might be the best way to do that, because it drastically reduces the complexity of maintenance, and helps give end users the information to fix their local copies.

@poipoi300 poipoi300 self-assigned this Aug 25, 2023
@poipoi300
Copy link
Owner

Thank you, I will likely be available to test this later tomorrow (EST)

@poipoi300
Copy link
Owner

@DrCyanide

I'm done checking out your PR and can confirm it works on 1.5.1, I have no doubt it works on 1.5.2 as well since that version doesn't change anything with args to txt2img or img2img. If you say it works on Vlad's I'll believe you on that 😆. Seems like a good way of preventing catastrophic breaks regardless. Good work!

2 Questions:

1 - How does the gr.Request object get its value for the username field? I'm mostly concerned about whether this is something that would also need to be updated in the future or if your code is taking care of that too. (I assume that's the case since I don't see username anywhere, but I also only see primitives getting default values, which could break fast.)
2 - Can you give me push permission to your fork? There are changes that I want to make (already done) before merging, but I still want to accept your PR.

@DrCyanide
Copy link
Contributor Author

I don't populate the username field.

A1111 is doing something pretty dumb with that, where it's required to make the API work, but it's optional for actually getting used. Specifically, it's only used in modules/processing.py in create_infotext(), with the line "User": p.user if opts.add_user_name_to_info else None,. Since A1111 itself can generate a None for the username it should always be safe to pass in a None there. The add_user_name_to_info setting is also unlikely to be helpful to anyone using Krita, since Krita's metadata would be what gets saved to the final exported image, not whatever A1111 sets.

While A1111 has an authentication option, it seems that is only used for accessing the Web UI itself. If they extend that functionality to the internal API, then we'd need to get the user password as well, at which point both parts of the Request (and the UI) would need to be reexamined.

I'll have to set up permissions another time, it's very late for me now.

@poipoi300
Copy link
Owner

poipoi300 commented Aug 27, 2023

No worries for the permissions, it was late (early lol) for me too and the current state of the extension is working anyways.

As for the username field, I looked into gr.Request and it seems like the default object has a username field set to None already. So that's where it was coming from. I had simply sent the request itself (pydantic model) with a username field initialized to "krita". You're right that it's not really important, but it's potentially useful info to have anyways since the intermediate images in txt2img and img2img folders can contain that metadata. We can generate the request object like this gr.Request(username="krita"). We can make it even more robust if we do gr.Request(username="krita", headers={}, client={"host":"0.0.0.0"}) This seems to cover the example given in the gradio docs. Urgh, why's A1111 doing this... Utterly backwards responsibility management.

In the future the extension definitely needs to switch to using the official API rather than the module function. Your changes make that a lot less necessary though. We can probably last a good while without needing to change anything related to our requests.

As for changing the UI, I've been looking into that because I want to improve how the combo boxes work for searching models, and it doesn't seem incredibly complex, but the documentation is very limited. The python examples are machine translated from C++.. Yeah.

@poipoi300 poipoi300 merged commit 549222d into poipoi300:main Aug 29, 2023
@poipoi300
Copy link
Owner

@DrCyanide Thanks again for the PR! If you'd like to revert your branch to how it was before I made changes so that it may be accepted as it was into the original extension repo, you can now do that.

I hope to see you active on this repo again in the future 😃

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