-
Notifications
You must be signed in to change notification settings - Fork 227
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
Fixed the skipped/missed images and/or panels #393
Conversation
@FulyaDemirkan there are a number of outstanding PRs so I'm not sure when you will get a response to this PR. The first issue sounds rather scary, I've a number of comics that are wider than the target width of my device and I've not seen the problem you described. Do you have a test case? For the border of 4 heuristic. Take a look at https://github.com/FooSoft/mangle/blob/9b5254047901f0c8285742eaa084864cd7f63248/mangle/image.py#L183 which can handle arbitrary sizes that might be a safe solution. Are you able to break these into multiple commits to separate them out? One commit, per bug fix? Again, I'm not sure if/when this will get reviewed so I understand if you don't have time for this. |
Hi @clach04, Agree on the scary part, after getting so confused about so many stories which I always assumed bad story telling or sudden time jumps etc. then I realized the issue was not those but the conversion. ### For the first issue;
So, when I try to convert this chapter, targetWidth is set to 755. Converted chapter ends up missing 14 pages out of 26. For the solution,
### For the second issue:
So, I added a checkpoint for I am not sure what you meant by "For the border of 4 heuristic." though. Are you referring to the third problem which crops 4 pixels from the sides? |
@FulyaDemirkan thank you for taking the time and effort to make the comprehensive and detailed reply! That helped explain things for me :) The screenshot especially made it clear. I knocked up https://github.com/clach04/image_ls to help dump simple metadata for directories of images and CBZ files. Most of mine that I bought from Humble Bundle have the same resolution but I was able to see the problem you described when I created some test images with the differences you mentioned. I'd not realized target width was the width of the most common width in the set! I have not used the 2panel script before (I've only been using the 2ebook script, which doesn't have the width issue). I did some quick experiments and I also see the target width issue for non-merged (i.e. split only) output as well!
Yes, that what I meant and did not explain clearly. As a heads up, I just read #327 which implies PR's are not likely to be merged with the current version - so please ignore my request to break up into smaller commits! |
This pull request consists of several fixes:
mergeDirectory()
Problem: If the image width is greater than the targetWidth, app skips that image and omits from converting it.
Solution: if that happens the code resizes the image to the targetWidth while preserving the aspect ratio.
splitImage()
Problem: If the panel at the bottom of the image is not followed by a solid cropped row then the app doesn't add that panel to the panels array.
Solution: If last the panel is detected but there is no solid cropped row at the bottom of the image, code sets the panel bottom dimension to the heightImg.
Problem: If the source image is badly cropped and has a thin black outline it causes app to consider whole image as one panel which ends up with lots of unnecessary empty pages, or tiny panels.
Solution: 4 pixels from right and left gets cropped by default to avoid that, It's thin enough to not cause a major problem with the actual image but eliminates the problem with some of the outlines.
Problem: Larger sized images (as far as I can see images larger than 1.5-2mb) throws a "DecompressionBombWarning".
Solution: I set the MAX_IMAGE_PIXELS = 1000000000 and put a simple warning.
makeBook()
Problem: Max height is set to 1024 by default. So, even your device resolution is higher than that images are cropped by 1024. This causes some images that might actually fit to the screen to be cropped to two.
Solution: Max height is now set to device's resolution. (For example for Kindle PW 3+ it's 1448 now.)
I also implemented @pavelzw pull request for systems with with a lot of RAM