Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Increase Upscaler Limits #14589

Merged

Conversation

Sj-Si
Copy link
Contributor

@Sj-Si Sj-Si commented Jan 8, 2024

Description

This PR increases the upscaling limits in the Extras tab. It also removes the upper limit for upscaling_resize in the API.

Screenshots/videos:

Below are images showing the new limits applied to the Extras tab.

image
image

Checklist:

Notes:

I wonder if these limits were originally set to avoid people constantly writing up issues saying that their WebUI keeps crashing whenever they try to run an upscale job that would use up all their memory. If that is the case, then we can just close out this PR and leave it as-is. However I think it would be helpful to increase these limits so that people don't have to dig through code to find out where to bypass them.

@w-e-w
Copy link
Collaborator

w-e-w commented Jan 9, 2024

I'm fine with increasing default the scale to limit to 8192

but why the hell would someone who wants to upscale their image by 100 times
as I recall most upscaler are trained for 4x, setting the default to 100 it is just insane
the only thing it achieves is to make the slider hard to drag

the original 8 is good enough for mose use cases, if you need it to be higer set it in your ui-config.json for your self in your ui-config.json
this update also will only apply to new installs (or if ui-config.json was removed)
changes like this will not override the ui-config new default

@akx
Copy link
Collaborator

akx commented Jan 9, 2024

but why the hell would someone who wants to upscale their image by 100 times as I recall most upscaler are trained for 4x, setting the default to 100 it is just insane

Especially considering the maximum number of times the current implementation will even attempt an upscale is thrice, and it will then (maybe) downscale to the desired size. 😁

for _ in range(3):
if img.width >= dest_w and img.height >= dest_h:
break
shape = (img.width, img.height)
img = self.do_upscale(img, selected_model)
if shape == (img.width, img.height):
break
if img.width != dest_w or img.height != dest_h:
img = img.resize((int(dest_w), int(dest_h)), resample=LANCZOS)
return img

@Sj-Si
Copy link
Contributor Author

Sj-Si commented Jan 9, 2024

I'm fine with increasing default the scale to limit to 8192

but why the hell would someone who wants to upscale their image by 100 times as I recall most upscaler are trained for 4x, setting the default to 100 it is just insane the only thing it achieves is to make the slider hard to drag

the oeiginal 8 is good enough for mose use cases, if you need it to be higer set it in your ui-config.json for your self in your ui-config.json this update also will only apply to new installs (or if ui-config.json was removed) changes like this will not override the ui-config new default

Yeah I'm not sure who would use 100x but I just chose some round number that I figured nobody would ever hit and be limited by. The number can absolutely go down. I usually just type in the scaling factor so I wasn't really thinking about the slider too much. Do you have any suggestions for a reasonable scaling factor maximum?

I was also wondering about the ui-config.json when I was testing these changes. Is there no way to override the ui-config with a PR like this?

@w-e-w
Copy link
Collaborator

w-e-w commented Jan 9, 2024

Do you have any suggestions for a reasonable scaling factor maximum

as mentioned above the current value 8 is good enough
8 is already ridiculously high value that the upscaled output is mostly just the denoise smearing

@AUTOMATIC1111
Copy link
Owner

The reason for having low maximum value is as follows. If you have possible zoom levels from 1 to 8, and a general user usually needs something like, let's say, from 1 to 4, then about 3/7 = 42% of the slider is useful space for him: there are hundreds of pixels of slider width, and he can drag the slider with the mouse, and get different zoom levels that he likes. If you set it from 1 to 100, suddently it becomes 3/99 = 3% of the slider, and that's a very small number of pixels on screen. At this point, slider is more or less unusable, and he just has to input his value into the box. So the smaller limits are to make the slider useful. And you can change the maximum value for yourself in ui-config.json file.

@Sj-Si
Copy link
Contributor Author

Sj-Si commented Jan 9, 2024

The reason for having low maximum value is as follows. If you have possible zoom levels from 1 to 8, and a general user usually needs something like, let's say, from 1 to 4, then about 3/7 = 42% of the slider is useful space for him: there are hundreds of pixels of slider width, and he can drag the slider with the mouse, and get different zoom levels that he likes. If you set it from 1 to 100, suddently it becomes 3/99 = 3% of the slider, and that's a very small number of pixels on screen. At this point, slider is more or less unusable, and he just has to input his value into the box. So the smaller limits are to make the slider useful. And you can change the maximum value for yourself in ui-config.json file.

Okay so I will remove the scale factor changes. Should we continue with the increased dimension limits or should I just close this PR and leave it up to the user to modify the ui-config.json?

@AUTOMATIC1111 AUTOMATIC1111 merged commit abb6301 into AUTOMATIC1111:dev Jan 9, 2024
3 checks passed
@Sj-Si Sj-Si deleted the hotfix/remove-upscaler-size-limits branch January 9, 2024 17:14
@w-e-w w-e-w mentioned this pull request Feb 17, 2024
@pawel665j pawel665j mentioned this pull request Apr 16, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants