kra-mo / cartridges

A GTK4 + Libadwaita game launcher
GNU General Public License v3.0
575 stars 28 forks source link

Preserve Image Aspect Ratio #177

Closed michaelneverwins closed 11 months ago

michaelneverwins commented 11 months ago

Is your feature request related to a problem? Please describe. When an image with an aspect ratio other than 2:3 is added to Cartridges, the image is stretched to 2:3 when it is resized to 200x300. Whether this is a "problem" is subjective; some may prefer stretching over the alternatives (padding and cropping) because it leaves no empty space and none of the image is lost. However, in my opinion, cropping sometimes looks right (if the image is close enough to 2:3 that nothing of value is removed) while stretching never looks right.

Describe the solution you'd like There could be an option to disable image stretching, i.e. to crop images to 2:3 before resizing so that images are not stretched to the 2:3 aspect ratio. Stretching can remain enabled by default just to preserve the current behavior for users who don't want to change it.

Describe alternatives you've considered

Additional context This issue is probably applicable only to manually added images. SteamGridDB always tends to have 600x900 images, and Steam provides images in the expected aspect ratio as well (either 600x900 or 300x450 in my experience). I don't know about other image sources.

kra-mo commented 11 months ago

Images are stretched only before a certain threshold. If their aspect ratio differs significantly, they are padded.

Cropping does not work, it could remove important parts of the image. We cannot know whether it would.

kra-mo commented 11 months ago

https://github.com/kra-mo/cartridges/blob/38d47dae339d508cef6ae68d776089adce05d77e/src/store/managers/cover_manager.py#L104

michaelneverwins commented 11 months ago

Is that code used when images are added manually (as opposed to being imported automatically from Steam, SteamGridDB, etc.)? If so, I would argue that the threshold needs adjustment, because Cartridges stretches this 4:1 (800x200) image to 2:3.

test

stretched 4:1 to 2:3

Not that I would try to use a 4:1 image as a cover, but it definitely does differ significantly from 2:3.

michaelneverwins commented 11 months ago

Ditto for 1:4 image (200x800).

test

stretched 1:4 to 2:3

michaelneverwins commented 11 months ago

As for whether cropping is a good idea, I agree that it won't always look right. That's why I suggested making it optional, but of course I understand the desire not to add options which are reasonably likely to do bad things. I would still argue that stretching never looks quite right, but slight stretching is sometimes undetectable at a glance, so stretching only to a certain threshold and padding beyond that threshold isn't a bad idea. (Padding isn't my favorite solution, but I'll backpedal a bit on the "[padding] never looks good" comment above. I was thinking specifically of the way Steam does it, especially the way it's off-center.) The problem is that I didn't know this feature existed in Cartridges because it doesn't appear to be working for cover images I add manually. I don't know whether this is a bug (i.e. it should work for manually added images but doesn't) or a missing feature (i.e. it was implemented only for images imported automatically and doing it for manual images is new scope). But I did confirm that Cartridges ridiculously stretches images before opening this issue.

kra-mo commented 11 months ago

That's why I suggested making it optional

The problem is that, as I said, this is a per-cover thing. Making it a global option does not work here.

The padding only happens for automatically added games, so that should probably be changed.

GeoffreyCoulaud commented 11 months ago

👀

michaelneverwins commented 11 months ago

kra-mo closed this as completed in https://github.com/kra-mo/cartridges/commit/c2d671273ae47f5dd51cc25e33bdb56817e681cf

That was quick. Just to test my ridiculous pepper images again, I've confirmed that manually added wide images are now padded. :+1:

fixed

I see that taller-than-2:3 images always being considered "stretchable" is the intended behavior: https://github.com/kra-mo/cartridges/blob/c2d671273ae47f5dd51cc25e33bdb56817e681cf/src/store/managers/cover_manager.py#L104-L107 So the absurdly tall image not being padded is no surprise.