Can't scale things down, only up

[jywarren]

In MapKnitter now, on this map, i'm using the "r" key to switch to rotate+scale, but can only seem to scale up, not down: https://mapknitter.org/maps/irish-uk-border-mapping/

As this is in production, I'm marking it as priority -- let's try to pin this down! 🙌

[ebarry]

Hi, @steviepubliclab and myself have just experienced this same bug on https://mapknitter.org/maps/the-warehouse-bywater . Cannot scale down. (Can distort to make something smaller as a last resort)

[jywarren]

I believe this will be solved in Leaflet.DistortableImage 0.5.0 which is slated to be published later today. But in case not, noting that this bug seems to only affect some images - this map for example doesn't show it: https://mapknitter.org/maps/cranston-test

[sashadev-sky]

@jywarren @ebarry This is really exciting I managed to replicate this bug in LDI and I think it is a big realization about an implementation detail that we were confused about before:

• in the 2nd map from jeff (https://mapknitter.org/maps/cranston-test), you can make the image smaller up until it is a certain size (the same size that the images currently are in the 1st map (https://mapknitter.org/maps/irish-uk-border-mapping/).

• We added an edgeMinWidth to rotateScale to prevent it from scaling below a certain size and just assumed that, for some reason, it wasn't doing its job bc in LDI it was never really stopping our scaling down. (The idea was to prevent a bug where it scales so small it deletes itself)

• But, I found now if you set the zoom level in LDI to a more zoomed in level, you can see the result is the same as in MK:
  

• What further shows this is that we didn't use a function limiting scaling with edgeMinWidth for the scale handles, and at the same zoom level they continue to scale down:

• I tested this further by adding the same function to scale and now the result is the same as for rotateScale

So the answer to #219 - what is ideal edgeMinWidth and edgeMinHeight property value is a dynamic one based on zoom! I am also still convinced if we calculate it dynamically to also always match the image proportions we can try to use it to prevent unwanted distortion.

[sashadev-sky]

Also really cool, watch how the size of the image is recalculated at different zoom levels. We need to figure out what this calculation is and if its the same for all browsers (here it looks like its about doubled for each zoom).:

And to go with that the following notes from leaflet docs - they give us the formula:

But also a note about scaling inconsistency with different zooms - meaning their formula would maybe work like a heuristic:

[jywarren]

oh wow, what an obscure issue!

Is there a short-term fix that gets basic scaling working again, with the more subtle projection issues solvable in a follow-up? Thanks Sasha!

[jywarren]

Leaflet.DistortableImage/src/edit/RotateScaleHandle.js

Lines 26 to 32 in 83e3371

	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);
	}

[jywarren]

So in that section we should convert overlay.getCorner(0)).distanceTo(overlay.getCorner(1) to screen space, not world-space, right?

latLngToContainerPoint() - https://leafletjs.com/reference-1.4.0.html#map-latlngtocontainerpoint

[jywarren]

var corner1 = map.latLngToContainerPoint(overlay.getCorner(0)),
      corner2 = map.latLngToContainerPoint(overlay.getCorner(1)),
      w, h;
w = Math.abs(corner1.x - corner2.x);
h = Math.abs(corner1.y - corner2.y);

[jywarren]

And, edgeMinWidth should be something like 10px

[sashadev-sky]

updating these lines allows for the image to 'self delete':

Will continue debugging

[sashadev-sky]

couldn't get this to work. Noting other solutions i'm thinking to try next / just notes -

• error thrown In a call to _reset after setting new corners in _scaleBy, it's throwing an error for an invalid latLng.
• I attempted to overrwrite the _scaleBy function to update the scale using a scalar matrix based on points instead of current implementation where we setCorner(s) based on calculations. This worked, but could not figure out how to update the handles' location properly without updating the _corners on the image. getBoundingClientRect() was not sufficient because we are not working rectangles only. Trashed this attempt.
• Tried to get the map to zoom in whenever the image was being scaled too low. Would be a good feature idea! Got stuck on many issues with this - likely would be a feature to add after fixing this bug not a means to fixing it
• potentially just make a predefined object with different edgeMinWidths at different zoom levels (create formula?)
  • start from most zoomed in level figure out the ideal #, from there it might just be a matter of doubling it for every lower zoomDelta of 1.

@jywarren what are your thoughts?

[jywarren]

Hmm, I'm going to look at this error:

error thrown In a call to _reset after setting new corners in _scaleBy, it's throwing an error for an invalid latLng.

first!

[jywarren]

OK, i think i got it. Basically it's working, but when we do this comparison (w > edgeMinWidth && h > edgeMinWidth) that's not right - we shouldn't veto scaling down just because /either/ dimension is under the limit. It's easy for 2 adjacent corners to be at very similar y values but different x values, but still not be close to each other. We need to use the pythagorean theorem to find their actual distance and limit by that, rather than BOTH limiting difference in x and y.

[jywarren]

so:

var w = Math.abs(corner1.x - corner2.x);
var h = Math.abs(corner1.y - corner2.y);
var distance = Math.srqt(w * w + h * h);

[jywarren]

See #340!

[jywarren]

You can still brute-force it somehow... spinning around until you can get it to run. And there are some distortion issues happening. But I think bumping the min value to 30px can be a relatively good fix.

[jywarren]

[jywarren]

Here's limiting to 50:

[jywarren]

50 seems good! Now I think we're just facing this distortion issue which is it's own thing. (#85 #301)

[jywarren]

You can get past it if you are scaling really fast, like, if the change in scale is close to 50px change per animation frame, but it's pretty tough.

[jywarren]

Confirmed thsi is fixed and published to MapKnitter! Thanks all!