matiasgali / guillotine

jQuery plugin to crop images within an area (fully responsive), allowing to drag (touch support), zoom and rotate.
http://guillotine.js.org
327 stars 100 forks source link

Scale parameter ignored when width and height is specified #53

Closed sigma-technology closed 8 years ago

sigma-technology commented 8 years ago

After implementing Guillotine into my application, I found an issue when trying to add a scale value to the init options when adding the crop window to an image. It seems that the scale value is completely ignored and the default is used if the Width and Height parameters are specified. Whereas, if none are specified and it works from the default width and height values (400x300) then the scale is applied fine.

sigma-technology commented 8 years ago

To add to this, I have also found that if you set the x and y position of the crop window in the init options, then the height of the crop window doesn't get set and instead uses the image aspect ratio to determine the height based on the width.

matiasgali commented 8 years ago

Hi @sigmatec, please don't forget to read the CONTRIBUTING file before posting an issue. You need to post the least amount of code needed to reproduce your issue or link to an example so others can work on it.

This example sets custom height, width, x, y and scale, everything seems to work just fine. Please fork or update that example and try to reproduce the problem. Post the new jsfiddle back here if you can reproduce it or please close this issue if the problem was somewhere else.

sigma-technology commented 8 years ago

Hi,

I did read the contributing file, apologies that I forgot to mention that it was the latest version of the plugin and also the same version of jQuery used in the demo, I thought this would be assumed.

I also didn't provide any code, because I tested in the demo files provided and had the same issue occur.

Again, the example that you have provided, it isn't in fact working. Try change the angle to 270 in the settings and the scale to 10, it doesn't change at all and reverts to the defaults.

Thanks.

sigma-technology commented 8 years ago

Code example:

picture.one('load', function() { picture.guillotine({ width: 500, height: 500, init: { x: 410, y: 90, angle: 90, scale: 2.5 }, eventOnChange: 'guillotinechange' })

The image is not rotated 90 degrees (angle) and scale is not set to 2.5.

matiasgali commented 8 years ago

This one has an angle of 270 and scale of 10.

picture.guillotine({
  width:  500,
  height: 500,
  init: { x: 410, y: 90, angle: 270, scale: 10 },
  eventOnChange: 'guillotinechange'
})

angle_270_scale_10

And this one has an angle of 90 and scale of 2.5.

picture.guillotine({
  width:  500,
  height: 500,
  init: { x: 410, y: 90, angle: 90, scale: 2.5 },
  eventOnChange: 'guillotinechange'
})

angle_90_scale_2 5

In both cases the picture is properly transformed and both report accurate data as shown in the images.

What browser (and version) are you using? It's possible that the issue exists only in some browsers.

You can also ping me through Guillotine's chat room on Gitter to iterate on the issue.

sigma-technology commented 8 years ago

Huge apologies Matias for any time wasted - it does indeed work in the way you have said.

We added functionality to draw a crop area on top of the existing area, then we wanted to re-initialize guillotine with the new width, height, x, y and scale. It was on this re-initialization that the scale wouldn't "take". We wrongly assumed that this was a bug with the initialization.

We don't seem to be able to fully disable and re-enable guillotine from outside the 'load' function, which I'm not sure is a bug or intentional. Either way, we've implemented a work-around now.

If being able to draw a different crop area within the existing one is functionality you'd be interest in implementing into guillotine then please let me know and I'd be happy to share the code with you.

Thank you very much for you time.

matiasgali commented 8 years ago

@sigmatec I'm not sure what you mean by drawing a crop area on top of an existing one but if you mean to re-initialize the plugin over the same image (or if you change the image's src) you must remove the existing instance of the plugin before doing anything else.

This issues #18, #3, #5 and ultimately this answer outline the problems of not removing the current instance and how to solve it.

Happy coding!