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

images distort while scaling #85

Open
jywarren opened this issue Jan 19, 2018 · 5 comments · Fixed by #158
Open

images distort while scaling #85

jywarren opened this issue Jan 19, 2018 · 5 comments · Fixed by #158

Comments

@jywarren
Copy link
Member

jywarren commented Jan 19, 2018

via @patcoyle in publiclab/mapknitter#195 - copying this over!

Plus LOTS of detailed info on this bug over here: publiclab/mapknitter#153


See attached screenshot of in-progress map: http://mapknitter.org/maps/livermore-ca-mobius-with-8mm-lens
Image at bottom right is example where when attempting to scale, size, or rotate, the image distorts. Sometimes, to trapezoidal shape, sometimes "squashed" or stretched in one dimension.

what you did just before the problem occurred: Observed problem previously in Safari 8.07, Firefox 39, so tried Chrome 44.0.2403.125.
your browser and browser version: Noted above
your operating system: OSX 10.10.4
any web addresses needed to see the problem (such as the URL of this page): 

http://mapknitter.org/maps/livermore-ca-mobius-with-8mm-lens
your PublicLab.org username (if you have one): patcoyle
describe how urgent the problem is (desperate or just-trying-to-help!): Mapknitter is mainline tool, so if systemic, urgent.

If you are able, it may also help to quickly resolve the issue if you:

test if it happens in a different browser or on another computer: Noted above
test if it happens whether you are logged in or not (if applicable): NA
test if it affects other users, or whether this happens to you only sometimes: Don't know

screenshot skewed image

@ChrisLowe-Takor
Copy link

I've a couple of observations on this. Using OSX and chrome I can reliably reproduce this on the example by:

Grabbing the top right handle and slowly dragging directly upwards you can create trapezoid distortion:

screen shot 2018-08-26 at 12 43 19 pm

By grabbing the top left handle and slowly dragging to the left you can stretch the image horizontally:

screen shot 2018-08-26 at 12 43 33 pm

And by grabbing any handle and very slowly rotating in either direction you can induce a little of both.

screen shot 2018-08-26 at 12 43 57 pm

Another thing to point out is the geographic size plays no role. It behaves the same big or small.

Here is where it gets interesting. When I was writing the react wrapper I found myself wanting to make small changes to either scale or rotate and the results were not what I was looking for so I split them into two seperate actions. The problem with the distortion was completely eliminated and personally I found it easier to use.

Might want to discuss with your users about changing core functionality like that but it will certainly eliminate the problem.

Anyway, the heart of the problem is here:

https://github.com/publiclab/Leaflet.DistortableImage/blob/master/src/edit/RotateHandle.js#L19-L20

This answer suggests scaling first then rotating which would be a cheap idea to try.

Another option would be to combine the matrix transformations as outlined here. The theory is that the rotate -> refresh -> scale -> refresh might be introducing errors because of the way its applied them to the DOM as style.

Third idea would be to put a threshold on the calculations or clamp them with .toFixed() to prevent floating point accuracy issues around cos(x) as x approaches 0. Speaking of floating points another avenue may be to use BigNumbers for the rotation calculations.

@jywarren
Copy link
Member Author

This is really great research, thank you so much! I like your proposed solution of using .toFixed() or scaling first then rotating... worth a try for sure.

I mentioned in #105 but we might also have an options flag to switch between a separate vs. combined scale/rotate interface. This could solve the issue for lots of people and I'd happily accept a PR or work towards this goal and would love to support if anyone's interested.

@sashadev-sky
Copy link
Member

@jywarren @ChrisLowe-Takor I have been working in this library for a bit now and just wanted to give an update on this issue! I think the problem was because we were applying transformations on the images assuming they are squares, but really they don't have the same height and width (difference of around 150px). By setting edgeMinWidth and edgeMinHeight options equal to their w/h when instantiating them and adding some conditional logic in the scaling class, scaling no longer creates distortions:

bug5

Please let me know if you have any thoughts on this!

@sashadev-sky sashadev-sky mentioned this issue Mar 19, 2019
4 tasks
@jywarren
Copy link
Member Author

jywarren commented Mar 19, 2019 via email

@sashadev-sky
Copy link
Member

@jywarren @ChrisLowe-Takor Ok take 2 -- this is now actually fixed! via #341 and #348. The solution was to switch latLngToLayerPoint to project() 🤯meanwhile we've all been rewriting matrix algorithms. I do see though, that when you move the handles slowly the image jitters a bit.
a. luckily this is not the case for rotate so this narrows things down a bit.

Should I close this an open a new issue for that?

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

Successfully merging a pull request may close this issue.

3 participants