Proposed combo of 229 230

[lstein]

This is a combination of PRs #229 and #230. Co-authored with @blessedcoolant

[lstein]

@blessedcoolant I've fixed the recent conflicts and will push as soon as a conf call is over.

[lstein]

Regression on WebUI had nothing to do with these PRs. Was due to batch_size cleanup and corrected in the recently-merged pngwriter refactor.

[lstein]

@blessedcoolant Ok, this is ready for review. Recent commit lists you as co-author.

[blessedcoolant]

There's a couple of issues I spotted with this.

loaded input image of size 800x1000 from outputs/test.jpg. maximum image size is 512x512

I am not a fan of this. 512x512 is not the max size. We can do larger depending on if our system supports it. In cases where images with a larger area than 512x512 are presented, I think it should be handled in one of two ways.

1. Let the OOM happen and tell the user that maybe lower dimensions because their system doesnt have enough VRAM to support it.
2. Rescale the image retaining aspect ratio to values closer to 512 and then inform the user that the image was rescaled because it is larger than average inputs and that they can use -W and -H to push things further

Both solutions are finicky but this is case of deciding whether to give the user full control or if we should minimize OOM messages. I don't believe we can achieve both in this case.

---

Another bug. When I supplied a large image, it was auto rescaled to 512x512. So instead, I tried to set manual values. I set the width to 512 but the rescaling set it to be 576 instead. It should because 512 is a multiple of 64?

I think we should use my check on this. Where if the user does not provide a -W or a -H, then the image should be scaled via aspect ratio and not to 512x512 either.

This is happening because it is completely ignoring the -W value set. And only rescaling as per -H. There's no case for when both are defined.

[tildebyte]

Messy history.

I should be able to walk into a PR cold and review it commit-by-commit, and understand everything about it. If the commits are confusing and contradictory, they need to be reconciled and probably squashed.

In a sense, the final changes (the "Files Changed" tab here on GitHub) are irrelevant - it's how we got there which matters.

[lstein]

Thanks for the feedback. I'll go back to the original plan of using @blessedcoolant 's solution. I'll withdraw this PR and my earlier one.

[tildebyte]

Now I'm sad... 😁 I didn't mean to shut this down, but I trust your judgement.

[blessedcoolant]

Thanks for the feedback. I'll go back to the original plan of using @blessedcoolant 's solution. I'll withdraw this PR and my earlier one.

If you're going with mine, then someone please ensure that I covered all use cases. Do not merge until we're sure of it. @tildebyte @lstein

[lstein]

No worries. @blessedcoolant 's version works in the intended manner, and it's now in main.

There's still an unresolved issue that was first brought up by a user. He wanted the init image to be scaled down to fit inside the manually-provided height and width, rather than having to do this in advance. This was the problem I was trying to address. Though I didn't realize it at the time, this problem was different from the issue @blessedcoolant addressed, which was to make everything multiples of 64 without distorting the aspect ratio.

I'm going to add a checkbox to the GUI and a switch to the command line that forces the init image to be scaled to the manually-supplied width and height, so that people can get whatever behavior they expect

[lstein]

Another bug. When I supplied a large image, it was auto rescaled to 512x512. So instead, I tried to set manual values. I set the width to 512 but the rescaling set it to be 576 instead. It should because 512 is a multiple of 64?

I think we should use my check on this. Where if the user does not provide a -W or a -H, then the image should be scaled via aspect ratio and not to 512x512 either.

This is happening because it is completely ignoring the -W value set. And only rescaling as per -H. There's no case for when both are defined.

Oh, I did want to explain this, if just to remind myself what this was about. In order to resize the image to fit within the manually-supplied width and height while maintaining the aspect ratio, you have to adjust either the width (-W) or the height (-H) depending on the relative aspect ratios of the init image size and the manually set values. Think of the problem of fitting two oddly-shaped cardboard boxes inside each other. Sometimes you need to adjust the width of the outer box to change the fit of the inner box, and sometimes the height. The warning message changes depending on which dimension is the constraint.

[blessedcoolant]

There's still an unresolved issue that was first brought up by a user. He wanted the init image to be scaled down to fit inside the manually-provided height and width, rather than having to do this in advance. This was the problem I was trying to address. Though I didn't realize it at the time, this problem was different from the issue @blessedcoolant addressed, which was to make everything multiples of 64 without distorting the aspect ratio.

I'll look into this. That seems like a scaling function that needs to be done within the resizer itself rather than changes in the main script.

[lstein]

Agreed.

[blessedcoolant]

Agreed.

I did some testing on numerous resolutions. After seeing all the outputs, I think the ideal way forward with this would be to paste the resized image scaled up to fully cover the given width and height even if parts of the image get cropped.

I think this is a better approach because the black bars serve no purpose. But more importantly, they take away from the data the generator would want to work with which inevitably leads to worse results I feel.

So instead of black boxing, maybe we should just scale the optimized image up.

Thoughts? And if you agree, maybe you could tweak the resizing script to do it. I haven't look at that code so it'll take me time to figure out how it works.