ponty / pyscreenshot

Python screenshot library, replacement for the Pillow ImageGrab module on Linux.
BSD 2-Clause "Simplified" License
499 stars 89 forks source link

ImageMagick backend #10

Closed gastonrobledo-santex closed 10 years ago

gastonrobledo-santex commented 10 years ago

Are you sure that the implementation of bbox is correct in this case,

(bbox[2] - bbox[0], bbox[3] - bbox[1], bbox[0], bbox[1])

Here you are calculating the width and height from points but the other examples are with X,Y, Width, Height

So using this you will have different behaviors from the different plugins!

Does it make sense?

I suggest to change the plugin as

(bbox[2] , bbox[3], bbox[0], bbox[1])

Thanks

ponty commented 10 years ago

Which plugin has different behavior?

I checked scrot plugin, which is using X1,Y1, X2,Y2 ( im.crop -> http://effbot.org/imagingbook/image.htm)

imagemagick needs X,Y, Width, Height as parameter, so that's why there is a conversion in the plugin (http://www.imagemagick.org/script/command-line-options.php#crop)

gastonrobledo-santex commented 10 years ago

Hi -

you example shows that you need to pass X, Y, Width, Height but inside the plugin it makes a calculation of width and height. That's all you need to check the examples. Otherwise normalize everything to X,Y, Width, Height ot X1,Y1,X2,Y2

Thanks

ponty commented 10 years ago

Have you tried the examples?

python -m pyscreenshot.examples.showall

Can you send a sample program which has incorrect output?

gastonrobledo-santex commented 10 years ago

You have the example on Readme file, if you see you are sending X,Y, Width and Height unless that 500,500 are points, but that is not clear! and you will have a box of 500-X as width and the same with Height, and yes I'm using your library, In fact I created a backend for mac using screencapture instead of other backends!

ponty commented 10 years ago

I added X1,Y1, X2,Y2 to README and module doc. It should be clear now.

You can send a pull request, if you have new back-ends!

gastonrobledo-santex commented 10 years ago

I'll as soon I have this tested!

Thanks!

On Tue, Jan 7, 2014 at 12:24 PM, ponty notifications@github.com wrote:

I added X1,Y1, X2,Y2 to README and module doc. It should be clear now.

You can send a pull request, if you have new back-ends!

— Reply to this email directly or view it on GitHubhttps://github.com/ponty/pyscreenshot/issues/10#issuecomment-31746282 .