Skip to content

Fix for selection off by 1px#24

Merged
w32blaster merged 2 commits intow32blaster:masterfrom
thoepfner:fix-selection
Aug 24, 2015
Merged

Fix for selection off by 1px#24
w32blaster merged 2 commits intow32blaster:masterfrom
thoepfner:fix-selection

Conversation

@thoepfner
Copy link
Contributor

Currently, it's possible to create a selection that extends across the borders of the canvas by one pixel. See attached screenshot of the official demo below, where the image to be cropped is 600x398px and the selection is 601x399px, which shouldn't be possible.

Also, getSelectionXCoordinate() basically returns nInnerX + SELECTION_BORDER_SIZE, but setInitialSelection(...) doesn't take SELECTION_BORDER_SIZE into account. So, when you e.g. create a selection that covers the whole image and later open the cropper again setting the initial selection, the coordinates are deemed wrong by the cropper and it will reset the selection.

While trying to fix this, I found that the code introduced for issue #18 makes it quite complicated to get this right. That's why in this pull request, the internal coordinates are made zero based again and a different approach is taken to compensate for the border width.

cropper

@w32blaster
Copy link
Owner

Thanks, Timo, for your work! At the moment I'm staying abroad so I will be
able to check your pull request after one week I'm afraid. But thanks
anyway!
On 14 Aug 2015 17:27, "Timo Höpfner" notifications@github.com wrote:

Currently, it's possible to create a selection that extends across the
borders of the canvas by one pixel. See attached screenshot of the official
demo below, where the image to be cropped is 600x398px and the selection is
601x399px, which shouldn't be possible.

Also, getSelectionXCoordinate() basically returns nInnerX +
SELECTION_BORDER_SIZE, but setInitialSelection(...) doesn't take
SELECTION_BORDER_SIZE into account. So, when you e.g. create a selection
that covers the whole image and later open the cropper again setting the
initial selection, the coordinates are deemed wrong by the cropper and it
will reset the selection.

While trying to fix this, I found that the code introduced for issue #18
#18 makes it quite
complicated to get this right. That's why in this pull request, the
internal coordinates are made zero based again and a different approach is
taken to compensate for the border width.

[image: cropper]

https://cloud.githubusercontent.com/assets/77366/9275603/82ba20b2-429e-11e5-98bd-b0c2cc6c20bd.jpg

You can view, comment on, or merge this pull request online at:

#24
Commit Summary

  • Use zero-based coordinates internally
  • Compensate for selection border size

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#24.

w32blaster added a commit that referenced this pull request Aug 24, 2015
@w32blaster w32blaster merged commit 0f8cb79 into w32blaster:master Aug 24, 2015
@w32blaster
Copy link
Owner

Hey, I returned from my holiday and immediately checked your pull request. Thank you for fixing this annoying bug! I have already performed the release 0.5.4, updated ApiDocs. It will appear in Maven Central repo shortly.

Thank you!

@w32blaster
Copy link
Owner

The Gwt-Cropper 0.5.4 is released in Maven Central repository. Thank you for contribution!

@enginer
Copy link
Contributor

enginer commented Aug 25, 2015

Hello Guys, looks we have problem with selection boundaries in this release, issue: #25

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants