Skip to content
This repository was archived by the owner on Sep 8, 2021. It is now read-only.

Crop image instead of stretching - #1755

Open
Faeb35 wants to merge 2 commits into
airsonic:masterfrom
Faeb35:CropCoverArt
Open

Crop image instead of stretching#1755
Faeb35 wants to merge 2 commits into
airsonic:masterfrom
Faeb35:CropCoverArt

Conversation

@Faeb35

@Faeb35 Faeb35 commented Jun 12, 2020

Copy link
Copy Markdown
Contributor

This PR is fixing #1748
Before resizing the image it will be croped first to be square. This is prevent it from being strechted.

@eharris eharris left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think cropping the image is the right thing to do, because that then presents the problem of what portion of the image is kept, and it is impossible to always make the right choice. Instead the image should be padded with transparent pixels on either side of the short axis to keep it centered.

@Faeb35

Faeb35 commented Jun 12, 2020

Copy link
Copy Markdown
Contributor Author

I don't think cropping the image is the right thing to do, because that then presents the problem of what portion of the image is kept, and it is impossible to always make the right choice.

It's always taking the center part of the image and croping the edges. I've seen this behavoir quite a lot in other implementations. Like when I'm using DSub the cover is used as background of the lockscreen. It's doing the same thing there.

I prefered this behavior, as it results in having all the covers looking equaly instead of having transparent spaces on two sides on some of the covers. I think usually the covers already should be almost square, so it shouldn't crop off too much and wouldn't even be noticable. But if you add some transparent space it's defenitly "sticks out" (not sure if this is the right term in english).

@eharris

eharris commented Jun 12, 2020

Copy link
Copy Markdown
Contributor

I disagree. In my library, I have lots of albums that come from DVD-Audio sources, and cover images from DVD cases are nowhere close to square. Applying your cropping method would chop off the entire title of the album and a significant portion of the image in many cases, making the cover look awful. The current method of stretching is vastly preferable to simple cropping because it maintains the full fidelity of the image, although at the cost of a distorted aspect ratio.

Just because "a lot" of other implementations did the easy-and-dumb method of simple cropping doesn't mean it's the right thing to do.

@eharris eharris changed the title Crop image instead of strechting Crop image instead of stretching Jun 12, 2020
@Faeb35

Faeb35 commented Jun 12, 2020

Copy link
Copy Markdown
Contributor Author

Ok I see, we need to have all three options and a setting to choose from these options to make everybody happy :-)

@stale

stale Bot commented Sep 11, 2020

Copy link
Copy Markdown

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

@stale stale Bot added the stale This label will be removed soon label Sep 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

stale This label will be removed soon

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants