Add Resolution Checker - #229
Conversation
|
It looks like we checked in competing PRs at around the same time. Check out PR #230 . Similar motivation, but a few differences:
Is there an etiquette on GitHub for alerting team-members when you've started to work on a bugfix? It is a waste for both of us to have done the same bit of work. |
In this version, there's no chance for an image to be scaled to a larger area either.
When neither is passed, I default it to the boot value of the T2I which we set to 512. If the user sets it to something else, it'll be that.
I have no idea. I'm not super well versed the working of Github either. But I'll have a look. |
| new_image = Image.new('RGB',(width,height)) | ||
| new_image.paste(resized_image,((width-rw)//2,(height-rh)//2)) | ||
|
|
||
| print(f'>> Resized image size to {width}x{height}') |
There was a problem hiding this comment.
As much as possible, I'd like to keep UI messages out of the low-level libraries. I moved this into simplet2i._load_img().
lstein
left a comment
There was a problem hiding this comment.
Everything checks out, but there is one key difference in how #229 and #230 handle the edge case of the user specifying neither width nor height when passing an init_img. In PR #229, the init image ends up not being resized, and so user can get OOM CUDA error if they pass in a big init img. In PR #230, the image is resized so that its maximum dimension does not exceed the default width/height used in T2I initialization. I think this is what PR #229 intended, but I tested dream.py with a really large image file and got CUDA OOM.
Because it was easiest, I'm combining both PR#229 and #230 and turned into a single new PR which I will proceed to accept and merge.
If a user supplies a large image and gets an OOM, I think that's on the user because they're explicitly setting the higher value. We cannot set a case for this because each system would be capable of handling different sized inputs and we wouldn't want to set a rescale to a size by default. Let the defaults be handled by the user during boot. We can make that more obvious in the documentation. Having a look at the new PR. The combining of these two PR's is the best way forward with this. |
|
Hi @blessedcoolant, I'm sorry to be perseverating on this and feel free to read this later or not at all. I'm in the process of implementing a CLI and web "--fit" option to resize the scaled init image to fit inside the maximum values set by the manual width and height. I'm going through the code you contributed and having trouble understanding this bit in _load_img(): What's bothering me is that self.width (and similarly self.height) is the default width set at T2I initialization time. _load_img() gets called at command evaluation time via prompt2image(width=None,height=None). Within prompt2image(), on simplet2i lines 256-257, the following code appears: What I wrote these lines to do is to keep width's value the same if it is set, or replace it with the default self.width if width is None. However, this behavior means that you can't distinguish between a width that was set to 512 because the user passed 512, or a width that was set to 512 because the user passed None and the default width was picked up. So you can't really tell what the user's intentions were. So, what do we actually want to do? The goal is to get as close to the original aspect ratio while constraining the width and height to be multiples of 64. But you can't do both perfectly. Consider an 1000x860 image. Neither dimension is an even multiple of 64, so we have to fix that. Here are the options: There are plenty of similar cases where you can't get the exact aspect ratio back, but fortunately most common image formats are multiples of 64. I think the best we can do is to floor both the width and height to a multiple of 64 and either simply ignore the passed width and height, or if the user wants to --fit the image, then we resize the image to fit inside width x height while preserving its aspect ratio (adjusted to a multiple of 64). So the code for the non-fit case becomes really simple: Am I missing something here? |
So basically this tracks what input the user has given. if width == self.width and height == self.height:
new_image_width, new_image_height, resize_needed = self._resolution_check(
image.width, image.height)If the width and height that come to img2img are similar to self.width and self.height set at default, that means the user has not prompted a new width or height. And if they did, they gave the defaults. In this case, we get the optimized resolution for input image based on the image's width and height. If none of these conditions satisfy, that means the user has has given a custom input -- either width or height. if height == self.height:
new_image_width, new_image_height, resize_needed = self._resolution_check(
width, image.height)In here we track cases where the user has given an input. Starting first, we check if height == self.height, if it is, it means the user has not given a height input but has given a width input. So we get optimized resolution using the width user has provided but utilize the height of the image to get the optimized height. if width == self.width:
new_image_width, new_image_height, resize_needed = self._resolution_check(
image.width, height)Same thing for height. else:
image = InitImageResizer(image).resize(width, height)
resize_needed=FalseIf neither of the cases above satisfy, that means the user has given both width and height. In that case we get the optimized resolution for both those values and pass it to the resizer. I felt this was a fool proof way of doing this because I know you set the same checks in the image resizer too but the reason I do it here is that I also track
I spoke about this in the other thread. I think rather than black boxing, we rescale the users inputs to the closest valid resolutions as we are doing at the moment and then we scale the input image to fit the bounds. This will mean some of the input image will get cropped but that is a trade off the user is making by opting in for a wrong resolution input and using the auto fixing mechanism. Not to mention, this is what happens if a user manually does the rescaling too. In my view, this would be the best way to do it. Scale the input to match image bounds rather than blackboxing. |
|
Ah, OK. I got it. This is almost what I was trying to do (modulus the dumb
letterboxing), but it has the unexpected behavior that if the user doesn't
specify either the width or height, then the full size of the init image
(after trimming to 64) is passed to the generator. This makes sense on the
CLI, but for the web client the width and height are set by pulldowns and
default to 512. So the user will have the odd experience of getting a
really big image (or OOM) if they leave the size at the default, and get a
smaller image if they change either of the pulldowns.
How about we simplify the logic by using the full size of the image (after
trimming) by default and accepting a --fit_image option to scale the image
to fit inside the width x height box? In the CLI --fit would be opt-in, but
in the web interface it would be a checkbox that is default on. I started
going down this rabbit hole in response to this complaint:
#207
I agree completely that the letterboxing was the wrong solution and I've
rewritten the image resizer so that it shrinks or expands the initial image
to fit within the largest dimension while preserving the aspect ratio.
However, I'm not doing any cropping. Instead I let the aspect ratio go a
little bit off if the scaled dimensions aren't an even multiple of 64. This
could be as much as 12% distortion for images that are around 512 pixel,
and worse for smaller images. Do you think the image should get cropped
instead? We could lose up to 63 pixels if we go that route -- about 12% of
the image if it is around 512 pixels. Odd how that works.
Thanks for bearing with me.
Lincoln
…On Wed, Aug 31, 2022 at 10:30 PM blessedcoolant ***@***.***> wrote:
I'm going through the code you contributed and having trouble
understanding this bit in _load_img()
So basically this tracks what input the user has given.
if width == self.width and height == self.height:
new_image_width, new_image_height, resize_needed = self._resolution_check(
image.width, image.height)
If the width and height that come to img2img are similar to self.width and
self.height set at default, that means the user has not prompted a new
width or height. And if they did, they gave the defaults. In this case, we
get the optimized resolution for input image based on the image's width and
height.
If none of these conditions satisfy, that means the user has has given a
custom input -- either width or height.
if height == self.height:
new_image_width, new_image_height, resize_needed = self._resolution_check(
width, image.height)
In here we track cases where the user has given an input. Starting first,
we check if height == self.height, if it is, it means the user has not
given a height input but has given a width input. So we get optimized
resolution using the width user has provided but utilize the height of the
image to get the optimized height.
if width == self.width:
new_image_width, new_image_height, resize_needed = self._resolution_check(
image.width, height)
Same thing for height.
else:
image = InitImageResizer(image).resize(width, height)
resize_needed=False
If neither of the cases above satisfy, that means the user has given both
width and height. In that case we get the optimized resolution for both
those values and pass it to the resizer.
------------------------------
I felt this was a fool proof way of doing this because self.width and
self.height do not change in the current code after initialization and
they act as a good standard to compare the input width and height against.
I know you set the same checks in the image resizer too but the reason I
do it here is that I also track resize_needed value. This way, I only
call the resizer when it is absolutely essential. If the values are already
optimized, then there is no need to call the resizer again.
------------------------------
So, what do we actually want to do? The goal is to get as close to the
original aspect ratio while constraining the width and height to be
multiples of 64. But you can't do both perfectly. Consider an 1000x860
image. Neither dimension is an even multiple of 64, so we have to fix that.
I spoke about this in the other thread. I think rathe than black boxing,
we rescale the users inputs to the closest valid resolutions as we are
doing at the moment and then we scale the input image to fit the bounds.
This will mean some of the input image will get cropped but that is a trade
off the user is making by opting in for a wrong resolution input and using
the auto fixing mechanism.
In my view, this would be the best way to do it. Scale the input to match
image bounds rather than blackboxing.
—
Reply to this email directly, view it on GitHub
<#229 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAA3EVPJWQ2OSQFEVPQOPR3V4AIMXANCNFSM6AAAAAAQA7BRDQ>
.
You are receiving this because you modified the open/close state.Message
ID: ***@***.***>
--
*Lincoln Stein*
Head, Adaptive Oncology, OICR
Senior Principal Investigator, OICR
Professor, Department of Molecular Genetics, University of Toronto
Tel: 416-673-8514
Cell: 416-817-8240
***@***.***
*E**xecutive Assistant*
Michelle Xin
Tel: 647-260-7927
***@***.*** ***@***.***>*
*Ontario Institute for Cancer Research*
MaRS Centre, 661 University Avenue, Suite 510, Toronto, Ontario, Canada M5G
0A3
@OICR_news
<https://can01.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftwitter.com%2Foicr_news&data=04%7C01%7CMichelle.Xin%40oicr.on.ca%7C9fa8636ff38b4a60ff5a08d926dd2113%7C9df949f8a6eb419d9caa1f8c83db674f%7C0%7C0%7C637583553462287559%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=PS9KzggzFoecbbt%2BZQyhkWkQo9D0hHiiujsbP7Idv4s%3D&reserved=0>
| www.oicr.on.ca
*Collaborate. Translate. Change lives.*
This message and any attachments may contain confidential and/or privileged
information for the sole use of the intended recipient. Any review or
distribution by anyone other than the person for whom it was originally
intended is strictly prohibited. If you have received this message in
error, please contact the sender and delete all copies. Opinions,
conclusions or other information contained in this message may not be that
of the organization.
|
What do you mean by this part? Do we take the image as it is and then scale it down to something that doesn't cause an OOM? Sorry, haven't slept yet. My mind isn't processing that part at all.
I prefer the crop rather than the distortion . I think the distortion feels like a bad output rather than a refined result even with good prompts. I can't speak for all but I personally prefer a cropped version much better. Thanks for taking the time to solve this up. It's been bothering me a lot too. Lol. |
…ain-20260907 chore: sync upstream/main (2026-09-07)
Follow up to ##214 by @lstein
Here's what this PR does.
txt2img
-Wor-H, it performs a check to see if the resolution is a multiple of 64. If it is not, then it will change the resolution to the closest compatible number -- for both width and height or just one depending on what is set. The other will be defaulted to the boot default.img2img
-Wor a-Hthen the input image (if not of compatible res) will be resized as per its aspect ratio to the closest compatible resolution.-W, then the image is resized using the set W as the reference and the height being driven by the input image.-H, then the image is resized using the set H as the reference and the width is driven by the input image.-Wand-H, then the image will be resized to match those exact specifications and black boxes will be generated as per @lstein code.Any code that needs to made to the how the image is scaled exactly needs to be done in
image_util.pydirectly.I've done a lot of testing and it seems to be working fine. But if there's a weird case that doesn't match up, then please let me know so I can fix it up.