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

Improved margins for some cropped documents #44

Merged
merged 2 commits into from Feb 14, 2024

Conversation

hugleo
Copy link
Contributor

@hugleo hugleo commented Feb 12, 2024

A change in koptcrop for better margins, as mentioned in the comment: koreader/koreader#11325 (comment).
The bbox shouldn't be smaller than the page again, unless there's a DPI problem, who knows...
I didn't exhaustively test the semi-auto and manual crop modes...
But only if @poire-z finds if is logical :D


This change is Reviewable

A change in koptcrop for better margins, as mentioned in the comment: koreader/koreader#11325 (comment). 
The bbox shouldn't be smaller than the page again, unless there's a DPI problem, who knows... 
I didn't exhaustively test the semi-auto and manual crop modes...
But only if @poire-z finds if is logical :D
@poire-z
Copy link

poire-z commented Feb 12, 2024

But only if @poire-z finds if is logical :D

That's already too much kopt-low level for me to give any opinion :/
I'm sorry that you don't have much company and feedback with your kopt/zooming tweaks :(
I hope somebody else can review and merge.

Copy link
Member

@Frenzie Frenzie left a comment

Choose a reason for hiding this comment

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

The margin stuff looks sane enough, but I'm not really clear on how that is to be used in the frontend.

@hugleo
Copy link
Contributor Author

hugleo commented Feb 14, 2024

Take, for example, this image (attached inline; you can download it). You can see that the image has a lot of whitespace on the left and right sides, but none in the heights:

image

Open the image in Koreader and click on autocrop to apply autocrop settings. koptcrop.c will add margins in the heights as well, and it's caused by the frontend using the page size instead of the bbox because the bbox has a size beyond the page dimensions. The result is that the entire image is rendered without cropping:

a1

It will work when we set the margins to 0, so koptcrop.c will not apply any margins and the bbox will be smaller than the real page size. The crop will be applied, but you can see that we have no margins on the left and right sides, although the image allows for that:

a2

Using this PR:
Open the image and apply autocrop:

b1

We see that some margin is applied. We can even increase the margin:

b2

This PR calculates the maximum allowed margin. If we apply the margin less than that we are good, and if we apply a margin that goes beyond the page size, the maximum allowed margin is used instead.

@Frenzie
Copy link
Member

Frenzie commented Feb 14, 2024

Right, thanks. It's mainly the UI implications I wonder about since then the user sets a margin that's sometimes ignored, but it should beat the alternative.

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

Successfully merging this pull request may close these issues.

None yet

3 participants