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

crop() works incorrectly when x = 0 and w = current image width #675

Closed
tdobes opened this issue Dec 10, 2018 · 3 comments · Fixed by #741
Closed

crop() works incorrectly when x = 0 and w = current image width #675

tdobes opened this issue Dec 10, 2018 · 3 comments · Fixed by #741
Labels
released This issue/pull request has been released.

Comments

@tdobes
Copy link

tdobes commented Dec 10, 2018

Expected Behavior

Running crop() should result in a buffer of the appropriate length

Current Behavior

Running crop() results in a buffer with an incorrect length

Failure Information (for bugs)

There is a bug on this line:
https://github.com/oliver-moran/jimp/blob/master/packages/plugin-crop/src/index.js#L30

Specifically, it looks like we're shifting things left by 3 bits instead of 2 bits. Assuming that we have 4-byte pixels (RGB+alpha), we should only shift by 2 bits. (2**2 = 4)

Steps to Reproduce

$ node
> const Jimp = require('jimp');
undefined
> t = new Jimp(100, 100, 0x00000000);
<Jimp 100x100>
> t.bitmap.data.length == 4*100*100
true
> t.clone().crop(0, 0, 100, 50).bitmap.data.length == 4*100*50
false

(That last line should report true!)

Context

  • Jimp Version: 0.6.0
  • Operating System: Ubuntu Linux 18.04
  • Node version: 8.14.0

Fix

I'm not going to hassle with making a pull-request since this is a one-line fix. Basically, change the bit shift to 2 instead of 2+1. Specifically, the line should read:
const end = (start + h * w) << 2;

@hipstersmoothie
Copy link
Collaborator

One line fixes are the best though ;)

@piyushkantm
Copy link

Hey, is this fixed and merged in/before 0.6.4

@hipstersmoothie
Copy link
Collaborator

🚀 Issue was released in v0.6.6 🚀

@hipstersmoothie hipstersmoothie added the released This issue/pull request has been released. label Sep 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants