Skip to content

bunch of improvements #3

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

Closed
wants to merge 0 commits into from
Closed

bunch of improvements #3

wants to merge 0 commits into from

Conversation

w-e-w
Copy link

@w-e-w w-e-w commented Oct 17, 2024


use timestamp when saving the image
there's no reason to use random numbers
image


  • add setting to change output dir

Setting > Paths for saving > Output directory for photo refiner images
defaults to webui default output path paths_internal.default_output_dir\photo_refiner_outputs'

image


  • fix image save logic

the img object given back webui should be a PIL.Image
as the file is save it should also have already_saved_as attr set the it's save path, this action is performed so that gradio does not create a copy of the image for serveing the web page

this also fixis the Open images output directory
image


use InputAccordion
aka Accordion with built-in chackbox, no need to reinvent the wheel
disable
image
enabled
image

the open and close state of the Accordion is linked with the enabe checkbox
but you can manually check the box to detach the Accordion with the checkbox


add to Extras tab as a module

to be honest if it's up to me I would onlly put it here and wouldn't put it in txt2img and img2img tab

image

also you can add modules in Extra tab to the img2img tab in settings
image

because they already exist one module currently if you do so you end up with two photo-refiner in txt2img and img2img tab
image

if it's up to me I would remove the one in the scripts.Script


https://github.com/Marc0ai/sd-webui-photo-refiner/compare/main...w-e-w:sd-webui-photo-refiner:main?expand=1#diff-be9c8a7a324e96ef67051d3911bb13666c7963f9a70a635832b960fc7e218e11L26
also unless you're actively using element ID for you should not static ID
and when you do sympathetic ID you must make sure that there isn't any duplicate ideas otherwise it's pointless
there's helper functions in web UI that helps you create the IDS but considering that you're not using the ID I just removed it

@w-e-w w-e-w marked this pull request as draft October 17, 2024 10:28
@w-e-w w-e-w marked this pull request as ready for review October 17, 2024 10:46
@Marc0ai
Copy link
Owner

Marc0ai commented Oct 18, 2024

Wow, I really appreciate the effort and precision with which you improved the script's structure. Unfortunately, I wrote it with very basic knowledge of Gradio, WebUI, and Python. For the most part, I followed examples from other similar scripts and used some logical reasoning to put it together. Honestly, I wouldn't have been able to apply most of the improvements you made.

Anyway, the script is now updated, and most of the improvements have been implemented. However, I decided not to change a couple of things:

Save folder: This might be because I'm using an old version of WebUI and Gradio, so the option doesn't apply correctly. I preferred to keep the default folder as "outputs/photo_refiner_outputs." I don't think it's a big deal.

Tab location: I still want to keep the script in the "txt2img" and "img2img" tabs for convenience. I don't think it causes any issues.

I also fixed a few small things and cleaned up the code structure to make it clearer. Additionally, I made the extension more compact by splitting the sliders into rows. I also added the "Denoise" effect, which could come in handy.

Once again, thank you very much. I'm glad the script is better now.

PS: Maybe some things are still not 100% optimized, but I think it's much more presentable as an extension now.

@w-e-w
Copy link
Author

w-e-w commented Oct 18, 2024

what version of web UI are you targeting?

@w-e-w
Copy link
Author

w-e-w commented Oct 18, 2024

the output directory is kind of important
it might actually make the difference between useful and non-usable to some setups

also later on I remember that I didn't fix infotext

infotext aka PNG info, metadata / generation parameters

oh so later on I realized that you are using the postprocess call to apply the filters
well generally you don't do this

yes I know the name is confusing but this is not generally not you want to be processing images
this is the post processing for the pipeline / process
the main job of this is to clean up and send results to whatever is waiting

typically this type of post process will be done in places such as postprocess_image

The crucial difference is that, you're not just apply the filter to the "generated results" you are applying the results to "all images" that are returning to the front end
image that includes for example batch size > 1 you , may have a grid of all images,
and since the filter is applied in post process you're also applied the filter too the image grid
some extensions might also includes annotation images that are not actual generated images but you are also applying the filter on those images which is not ideal

also as mentioned you're returning the wrong type to results which breaks stuff


typically when you're offering a post-processing step such as this kind

as opposed to outputting the results to a separate folder you typically give an option to the user to "save a copy before filter is applied", and when the option is enabled a copy of the image will be saved with the suffix such as -before-color-correction
image

is there a particular reason why you implemented your way


please tell me what version of web UI are you targeting so I can make fixes that are compatible to those versions

@Marc0ai
Copy link
Owner

Marc0ai commented Oct 19, 2024

commit 0af28699c45c1c5bf9cb6818caac6ce881123131
version f0.0.17v1.8.0rc-latest-277-g0af28699
gradio: 3.41.2

as I already told you, I assembled everything with a really basic knowledge of python and substantially 0 knowledge of gradio and webui... I wrote the script like this because it is the only method I managed to make work (seeing how other existing similar scripts worked)

@w-e-w
Copy link
Author

w-e-w commented Oct 19, 2024

as I already told you, I assembled everything with a really basic knowledge of python and substantially 0 knowledge of gradio and webui... I wrote the script like this because it is the only method I managed to make work (seeing how other existing similar scripts worked)

that's why I'm here to help

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