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

initi top #47

Closed hchor closed 9 years ago

hchor commented 9 years ago

When I set init parameters (such as initi { y:0 }), the library doesn't seem to set them properly.

So in _offset, I added a console.log('canvas style top is '+this.canvas.style.top); (line 268) to see when canvas style top is set. If you don't include an init call, canvas style top is only issued once. However, if you issue an init call, canvas style top is called twice. But only the first time, canvas.style.top is set to 0%; on the second run, it is set back to a negative value to center the image vertically.

So, it seems like the init parameters are properly set in _offset, but _offset is called a second time to overwrite whatever I set in init. Or is this just me?

hchor commented 9 years ago

I think what I'm trying to say is, if I'm not mistaken, the call to _offset made by _init should be called last in the chain of calls to _offset?

hchor commented 9 years ago

More info: I also set width and height in the call to guillotine. So I can see _center() is being called, which re-centers it and kills everything I set in init. Is that proper behavior?

hchor commented 9 years ago

I fixed this by moving

  if (this.op.init != null) {
    this._init();
  }      

underneath

  if (this.width < 1 || this.height < 1) {
    this._fit() && this._center();
  }

Any new problems it could cause? init works properly for me now.

matiasgali commented 9 years ago

Hi @hchor, :clap: :clap: for actually looking into it instead of just reporting unexpected behavior.

Now, there's a reason why the _init() call comes before and I suspect that it is also the cause of your issue.

Guillotine stores image width and height as relative measures (1 being 100% of the window), that's what makes it responsive. That condition is checking whether the image is smaller than the window (or cropping area) in any dimension and if it is the plugin will fit and center it to make sure that there's no blank spaces.

So, that snippet makes sure that there's no blank spaces even if you input inconsistent data with the init option, that's why it needs to be after the _init() call.

If you see _center() being called and _offset() being called twice my first guess would be that your image is smaller than the cropping area you're setting with width and height (one dimension at least) so Guillotine is adapting the image to cover the whole area.

Please check the dimensions of the images you're trying against the width and height options you set. If dimensions are OK and the issue persists please post a fiddle or something I can review/debug and I'll take a look.

hchor commented 9 years ago

That's correct - I'm stretching the image to fit the area. So I basically had to switch the order of the statements in order for init to properly set. If that was your intention, then you can close the issue.

matiasgali commented 9 years ago

That's the intended behavior because otherwise you could end up with an image that doesn't cover the cropping area and thus blank spaces.

You should pass the complete set of data to the init option to build the image you want (like the output you'd get from getData). That way you won't be killing the size check.

Basically add the right scale to the init option. The condition will be false and the image won't be fitted or centered leaving you with your initial offset of y: 0.

picture.guillotine({
  width: glltWidth, 
  height: glltHeight, 
  init: { y: 0, x: 0, scale: glltWidth/imgNaturalWidth } // Scale up to fit the smaller image
});