-
Notifications
You must be signed in to change notification settings - Fork 27k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
1 parent
fd4461d
commit ef27a18
Showing
7 changed files
with
96 additions
and
60 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ef27a18
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.
What benefits does this rework have over the old one?
ef27a18
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.
it appears it's not working properly, unless I'm doing something wrong. what would be the best option for upscaler?
ef27a18
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.
This change does not appear to be backwards-compatible. It would be nice to have it as a toggle option. So far, I tend to prefer the former method.
ef27a18
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.
This works very slowly
ef27a18
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.
Yeah, this is painfully slow, compared to previously
ef27a18
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.
Is it possible to add the latent upscaling method from this thread? #4446
ef27a18
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.
this change does kinda wrecks outputs. pasting in seeds/vars from images before this and generating them are not even comparable anymore... what happened?
ef27a18
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 like the idea of swapping start value and end value (hires fix), but dont like the idea of a multiplier itself (the reasons described here #6219 (comment) #6219 (comment))
If specifically
Upscale by
slider will be replaced in two slidersHighRes Width
andHighRes Height
like this #6219 (comment) then I think the problem will be solvedef27a18
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.
oh man someone should really test it in public first if its an improvement before they suggest pull request, most people wont want downgrades
ef27a18
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.
The new methods seem less cohesive.
An example, generating a face.
With the old method it would expand the canvas. Resulting in a larger face, or a face and a torso.
With the new method, If I try to upscale a face, I just get a disjointed sea of faces.
ef27a18
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.
The new method for using Highres Fix is working well for me.
If you are still typing the first pass value as the second pass value, you will experience slowdowns, clonings, and crashes.
ef27a18
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.
@faisalhr1997 please, read this #6219 (comment)
ef27a18
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.
@E-16 So you want to turn 1:1 to 3:4 or 16:9 by cropping in. .. why would you want to ruin the composition of the original generation? have you tried sd upscale script?
ef27a18
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.
Because the models (checkpoint) have been trained with billions of SQUARED RATIO pictures, and when if they get a NON-SQUARED CANVAS with noise, they dont produce the output they were trained for, only produce awful results, just because they weren't designed for that in a first place.
PS: I personally dont have any problems, I keep generating on last new version, I'm just trying to help other peoples
ef27a18
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.
On the first pass, you should keep the resolution of one side at 512px and the other side can go up to 768px without any issues. However, after 768px, the chances of cloning increase. You can reduce these chances by using negative prompts such as clones, twins, or multiple subjects. As you go up to 1024px, the chances of cloning mostly depend on what you're prompting for.
But after 1024px, you are guaranteed to have clones if your base model was 512px. This can also change if the model was trained differently, like https://huggingface.co/HavoAloe/3DKX_1.0b. This model can do 768x1152 without cloning easily and it's just a fine-tuned version of SD1.5, which was trained on billions of squared images as you mentioned.
If you go below 512px on either side, you will definitely get poor results. But if you go above 512px you know what happens by now.
So why would you have to get cropped first pass image after knowing this is going to go over my head. Even if you need to crop it you could just send it to img2img tab and use crop and resize there then use sd upscale script which is basically the highres fix. If you still prefer using your old workflow. Then just use the old commits or wait for a fix.
ef27a18
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.
@faisalhr1997 Then explain it to people what they doing wrong in this huge issue #6219 if you even managed to find that type questions in this whole mess
ef27a18
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.
here are some examples of what I'm talking about without high-res fix. All images are 512px and above. Once I liked I would just use highres fix with swinir4x and denoising .45-.5
@RomanescoAI I just read the last part of the thread. Guess I am non square party. @E-16 HighRes Width and HighRes Height. It's the same thing just switching places. How ever you put it. You have to crop in. You don't even
ask for having the control where it's cropping from top bottom left right or center. You just want the output to be cropped out.
ef27a18
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.
@faisalhr1997 I will answer this way: in old version you could receive both versions (like squared and non-squared firstpass layer), in new version you become limited in settings
ef27a18
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.
@E-16 i updated the comment. Read it.
ef27a18
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.
but what this is proposing is removing upscale by completely and just the 1st pass and 2nd pass res switching places. Maybe they could keep it as an check option named as custom upscale resolution under upscale by option.
ef27a18
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.
Well, you right about this one, and if you read this topic fully #6219 (comment) then you would see that I said about GPU basically doing useless work, which is eventually cropped.
But if we change in way we discuss in that issue, then we will have full compatibility, as well as a new way to switch Hires. Fix, which I like more.
just give people what they want to do what they want, dont limit them, if we consider all settings that way, then there is alot of incompatible setting that give errors eventually, but for some reason we dont limit these settings
ef27a18
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.
If we add something like check option, then DEFINITELY NOT under
upscale by
option, for such cases in Settings there is a Compatibility group, where theoretically we can add ability to choose between: "Upscale By" and "HighRes Width and Height"ef27a18
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.
and you're ready to accept the blind crop HighRes Width and Height is making? Auto focal point crop will help here. It's already implemented in train>preprocess images.
ef27a18
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 wouldn't say that it is blind crop, rather hardcode centred crop. Auto focal point crop is a good idea, ngl, but it's a whole new setting in fact. Which I assume will likely create some serious instability, imagine if you change some values a little in same seed image and by this position of the entire Auto focal point may shift in new pos
ef27a18
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.
you're forehead is going to get chopped off by this hardcode centred crop 😹
ef27a18
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.
8149078 (
added the option to specify target resolution with possibility of truncating for hires fix; also sampling steps
ef27a18
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.
Is this the old hi-res fix or what? Does it let u reproduce old images before the rework?