Image distorted in Scale mode

[flopail]

Describe the bug:
When scale mode is active, image is sometimes distorted (so initial ratio is lost). There is a quite similar issue #85, but this one is closed and bug seems to be still there.

Reproduce the behavior:
To reproduce this bug, do a mousedown on a corner and resize image at min, then extend it (several times if necessary). Please see gif below.

Browser, version, and operating system:

• Leaflet.DistortableImage v0.4.2
• Firefox 67.0, Chrome 74.0
• Windows 10 (64bits)

[jywarren]

Oh thank you, and such good documentation of the issue as well!!!!

…

On Fri, Jun 7, 2019, 1:58 AM flopail ***@***.***> wrote: *Describe the bug:* When scale mode is active, image is sometimes distorted (so initial ratio is lost). There is a quite similar issue #85 <#85>, but this one is closed and bug seems to be still there. *Reproduce the behavior:* To reproduce this bug, do a mousedown on a corner and resize image at min, then extend it (several times if necessary). Please see gif below. [image: Scale-bug] <https://user-images.githubusercontent.com/32390593/59091900-9bed1a00-8910-11e9-9752-dd9177c899fb.gif> *Browser, version, and operating system:* - Leaflet.DistortableImage v0.4.2 - Firefox 67.0, Chrome 74.0 - Windows 10 (64bits) — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#301?email_source=notifications&email_token=AAAF6JYFX2JKK37WVW5PDXTPZIPIRA5CNFSM4HVT7ZQKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4GYGOUIQ>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAF6J4ALXC5AYJJXZEUKULPZIPIRANCNFSM4HVT7ZQA> .

[jywarren]

This is probably slow drift from the continuous recalculation from the previous location. Could be floating point drift? Or something like that? In any case we could preserve the original aspect ratio when in this non distortion mode maybe?

…

On Fri, Jun 7, 2019, 8:19 AM Jeffrey Warren ***@***.***> wrote: Oh thank you, and such good documentation of the issue as well!!!! On Fri, Jun 7, 2019, 1:58 AM flopail ***@***.***> wrote: > *Describe the bug:* > When scale mode is active, image is sometimes distorted (so initial ratio > is lost). There is a quite similar issue #85 > <#85>, but > this one is closed and bug seems to be still there. > > *Reproduce the behavior:* > To reproduce this bug, do a mousedown on a corner and resize image at > min, then extend it (several times if necessary). Please see gif below. > [image: Scale-bug] > <https://user-images.githubusercontent.com/32390593/59091900-9bed1a00-8910-11e9-9752-dd9177c899fb.gif> > > *Browser, version, and operating system:* > > - Leaflet.DistortableImage v0.4.2 > - Firefox 67.0, Chrome 74.0 > - Windows 10 (64bits) > > — > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub > <#301?email_source=notifications&email_token=AAAF6JYFX2JKK37WVW5PDXTPZIPIRA5CNFSM4HVT7ZQKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4GYGOUIQ>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AAAF6J4ALXC5AYJJXZEUKULPZIPIRANCNFSM4HVT7ZQA> > . >

[sashadev-sky]

@flopail the solution I think will correct this is outlined here: #219 - but the problem with it is those numbers are hardcoded based on my own trial and error for a specific image size. (I also moved those properties inside the class and have not documented them as part of the API yet because of aforementioned hardcoding). They were better than nothing, though, because they prevented an error where scaling down enough deleted the image and extreme distortions.

If you wanted to take a shot at it I would love your help!!

Returning to original aspect ratio also seems like a reasonable solution although the first one would be less of a workaround. But, it could also be the most plausible solution! I have already started on a restore function #274 so it could be something extending that.

[sashadev-sky]

Reopening #85 . Also noting we never implemented any solution for pure scale, just rotateScale.

[jywarren]

In looking into #340, I think this is the responsible section for at least some of the distortions:

Leaflet.DistortableImage/src/edit/DistortableImage.Edit.js

Lines 244 to 251 in 70b22a2

	for (i = 0; i < 4; i++) {
	p = map
	.latLngToLayerPoint(overlay.getCorner(i))
	.subtract(center)
	.multiplyBy(scale)
	.add(center);
	overlay.setCorner(i, map.layerPointToLatLng(p));
	}

I think it's because the values we're using to scale are integers and so we lose decimal precision over time as you keep running this math.

But maybe it's more complex as well: looking at @flopail's docs and testing myself, all images will tend to become square over time. I don't know if this makes sense re: the decimal precision issue.

But there may be a second reason for distortion. I noticed I was able to get images to turn from this:

to this:

And I'm not sure why! That's certainly not square.

[jywarren]

Here's an animation of this 2nd type. I don't know if finding a solution to the decimal precision section would solve this too?

[jywarren]

Maybe we could do all the math in latitude/longitude instead of pixels, but that could introduce some strange behavior if lat and lon aren't the same scale -- like, in current math we go to the origin (add and subtract center) i think... not sure if that'd work...

Maybe this could help? bbecquet/Leaflet.PolylineDecorator@00f7db5

[jywarren]

There they use map.project() instead of map.latLngToLayerPoint()... let me try.

[jywarren]

https://leafletjs.com/reference-1.4.0.html#projection

[jywarren]

Whoa! uh... maybe I solved the trapezoidal distortion issue with this, but not the tending-towards-squareness type?

[jywarren]

See #341!

[jywarren]

Hmm. OK, it's even possible there's a third type:

[jywarren]

By increasing the minimum size you can make an image in #340 (set there to 50) we may be able to limit this because the rounding errors may be more dramatic at smaller pixel sizes (since the errors are larger proportionally to the image size).

Another approach could be to essentially remember all corner coordinates (measured as x, y from the image center, perhaps) upon mousedown, and no matter what, scaleBy() and other functions would reproject those original values transformed by a scale and rotation. This would ensure that it preserves the same proportions/shape no matter what. I guess I really think this is the true solution here. That would mean that in the _onHandleDrag() event handler here:

Leaflet.DistortableImage/src/edit/RotateScaleHandle.js

Lines 11 to 37 in 70b22a2

	_onHandleDrag: function() {
	var overlay = this._handled,
	edit = overlay.editing,
	formerLatLng = overlay.getCorner(this._corner),
	newLatLng = this.getLatLng(),
	angle = this.calculateAngleDelta(formerLatLng, newLatLng),
	scale = this._calculateScalingFactor(formerLatLng, newLatLng);
	if (angle !== 0) { edit._rotateBy(angle); }
	/*
	checks whether the "edgeMinWidth" property is set and tracks the minimum edge length;
	this enables preventing scaling to zero, but we might also add an overall scale limit
	*/
	if (overlay.hasOwnProperty('edgeMinWidth')){
	var edgeMinWidth = overlay.edgeMinWidth,
	w = L.latLng(overlay.getCorner(0)).distanceTo(overlay.getCorner(1)),
	h = L.latLng(overlay.getCorner(1)).distanceTo(overlay.getCorner(2));
	if ((w > edgeMinWidth && h > edgeMinWidth) || scale > 1) {
	edit._scaleBy(scale);
	}
	}
	overlay.fire('update');
	edit._updateToolbarPos();
	},

...we'd record original positions in x,y (relative to image center?) on _onHandleDragStart(), and then in _onHandleDrag() we'd update using those originally stored values.

[jywarren]

However, #341's use of project() instead of latLngToLayerPoint() is also important, so as to preserve decimal precision! Just not /as/ important, as with an absolute reference, drift over time won't accumulate.

[sashadev-sky]

@jywarren keeping track of proportions and then setting the image back to them is basically what I implemented for the "Restore" action! But I think its not perfect either, it applies a slight translation. Haven't looked into that yet.

I managed to get it working via Matrix math with no distortions in #339 ! I would love some feedback there - its far from done yet - I still have to figure out how to make the markers update too and it currently works best with "img2" which didn't get initial corners assigned to it.

[sashadev-sky]

@flopail I believe this is fixed for version 0.7.0 now! Please reopen if you find this to not be the case

[sashadev-sky]

@jywarren it turns out switching to project and standardizing our calls to reset & update did the trick. Still a little confused because latLngToLayerPoint is not relative to absolute pixel coords and project is so I thought this was the reason. but also, not documented anywhere, project does have extra decimal precision (uses 9 digits - I just checked in console). But why have two core API methods, with one that is imprecise and not document that if it causes visible distortions? Looking at the source code is even more confusing, bc layerPointToLatLng returns the same precision as project and unproject, but latLngToLayerPoint uses round(), which strips all decimal precision to return an Integer, not sure why that would be desired

Sorry I just want to get this down as it'll continue being relevant still