Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: better cover image generation #2612

Closed
wants to merge 1 commit into from

Conversation

arition
Copy link

@arition arition commented Jan 15, 2024

Changed

  • Changed: Resize and crop cover image instead of forcing it to resize to a specified resolution (which breaks its original aspect ratio).

@majora2007
Copy link
Member

Please attach before and afters for the cover types listed in #2139.

@arition
Copy link
Author

arition commented Jan 16, 2024

Standard

IMG_001
image

Square cover

img00116
image

Another square cover

108942011_p0
image

Horizontal

img00115
image

Long

AXVUqdzi3rsC
image

@majora2007
Copy link
Member

I ran some tests with this and it doesn't behave well enough in my opinion.

Left is new code, Right is old
image
image
image
image
image

@arition
Copy link
Author

arition commented Jan 18, 2024

Yeah, it is not perfect in every situation. At least I think we can give user an option to change between these two modes.

@majora2007
Copy link
Member

That's not an avenue I'd want to go down. Could you add a check if the file is a webtoon, aka height is at least 2/3 more than width, and use the attention crop code there only?

This would add benefit for webtoon users while keeping it working for comic and Manga users out of the box.

@arition
Copy link
Author

arition commented Jan 18, 2024

That's not an avenue I'd want to go down. Could you add a check if the file is a webtoon, aka height is at least 2/3 more than width, and use the attention crop code there only?

This would add benefit for webtoon users while keeping it working for comic and Manga users out of the box.

I think it should to be "width 2/3 more than height", width > height * 5 / 3?

@majora2007
Copy link
Member

I think it should to be "width 2/3 more than height", width > height * 5 / 3?

I can't understand the math you're proposing. The goal is to identify if the image is a long panel (webtoon) and apply the new logic based on that.

@therobbiedavis
Copy link
Collaborator

I think it should to be "width 2/3 more than height", width > height * 5 / 3?

I can't understand the math you're proposing. The goal is to identify if the image is a long panel (webtoon) and apply the new logic based on that.

Very confused, would the conditional just be if (width === (2/3 * height)) { //Width is 2/3 of height }else{ //Width is not 2/3 of height} ?

@arition
Copy link
Author

arition commented Jan 23, 2024

I can't understand the math you're proposing. The goal is to identify if the image is a long panel (webtoon) and apply the new logic based on that.

Got it. In my opinion, the new method also works well on wide images. Maybe we can apply it when width > height * 5 / 3 || height > width * 5 / 3?

@majora2007
Copy link
Member

Hey I'm planning to take this code into a test branch sometime and tweak the code. I think there can be some benefit to this new code with a little more prodding into the source cover/archive. I want to do some pretty heavy investigation to improve webtoon cover generation and test this against more user's servers.

I have no plans when (as I'm in the middle of huge code refactors to support new features), but I wanted to share that I do plan to try this PR out in full for potential inclusion.

@arition
Copy link
Author

arition commented Apr 3, 2024

Thank you! @majora2007

@majora2007
Copy link
Member

I ended up taking a look at this and wrote my own logic along with a test suite to help tweak this in the future. Here are some of my results (original, old code, new code):
image
image
image
Uploading image.png…

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

3 participants