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

page offsets after trim() are off by one compared to the CLI's #475

Closed
sbraz opened this issue May 8, 2020 · 5 comments
Closed

page offsets after trim() are off by one compared to the CLI's #475

sbraz opened this issue May 8, 2020 · 5 comments
Labels

Comments

@sbraz
Copy link
Contributor

sbraz commented May 8, 2020

Hi,
I have ImageMagick 7.0.10-10 and Wand 0.5.9 (I also tried the latest git master).
I am running this very simple program with the attached test.png file:

#!/usr/bin/env python3
from wand.image import Image

with Image(filename="test.png") as img:
    print(img.size)
    print(img.page)
    img.trim()
    print(img.size)
    print(img.page)

It returns the following:

(10, 10)
(10, 10, 0, 0)
(2, 3)
(12, 12, 5, 4)

On the other hand, convert -trim test.png info: returns test.png PNG 2x3 10x10+4+3 8-bit sRGB 95B 0.000u 0:00.000.

Am I missing an extra step or are the page offsets off by one? I'm also curious about the width and height of the page, why did they increase?

@emcconville
Copy link
Owner

emcconville commented May 10, 2020

Looks like a bug to me.

Am I missing an extra step or are the page offsets off by one? I'm also curious about the width and height of the page, why did they increase?

The "off by one" behavior is coming from 1x1 border being applied to the image to ensure a clean MagickTrimImage call.

From the source code...

        if color is None:
            color = self[0, 0]
        # ... omitted
        with color:
            self.border(color, 1, 1, compose="copy")  # Adds a 1x1 border
        r = library.MagickTrimImage(self.wand, fuzz)
        if reset_coords:  # Reset page to (0, 0, 0, 0) -- New in Wand 0.6.0
            self.reset_coords()

I would guess that the correct behavior would be to reduce the offset if reset_coords are not applied.

        if reset_coords:
            self.reset_coords()
        else:
            adjusted_page = self.page
            adjusted_page[2] = adjusted_page[2] -1 if adjusted_page[2] > 0
            adjusted_page[3] = adjusted_page[3] -1 if adjusted_page[3] > 0
            self.page = adjusted_page

Or something close to that. Thoughts?

@sbraz
Copy link
Contributor Author

sbraz commented May 10, 2020

Yes, that makes sense (I might use max(adjusted_page[2] - 1, 0) but I get the idea).

The 1 pixel border is added on all 4 sizes, right? That is why the dimensions are increased by 2 pixels too if I understand correctly.

Why do you need to add that 1x1 border? Is that in case the image is empty?

@sbraz
Copy link
Contributor Author

sbraz commented May 10, 2020

Oh, I think I get it. You add a border of a certain color because the color parameter isn't something native to ImageMagick.

I guess you should also do adjusted_page[0] -= 2 and adjusted_page[1] -= 2 in the else block, right?

@emcconville
Copy link
Owner

emcconville commented May 11, 2020

I guess you should also do adjusted_page[0] -= 2 and adjusted_page[1] -= 2 in the else block, right?

Right.

Why do you need to add that 1x1 border? Is that in case the image is empty?

Not for empty images, but a old CLI hack for choosing which color to trim against. Or a should say, ensuring that the first color of the first band is the color you wanted to trim with.

... might use max(adjusted_page[2] - 1, 0) ...

🤦 absolutely!

@emcconville
Copy link
Owner

emcconville commented May 11, 2020

Should be fixed. Please checkout the master branch to verify. Thanks for reporting this!

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

No branches or pull requests

2 participants