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

When using L.CRS.Simple for the map, drawing a circle creates a really huge unusable circle #611

Open
dcodeIO opened this issue Oct 16, 2016 · 3 comments
Labels

Comments

@dcodeIO
Copy link

dcodeIO commented Oct 16, 2016

It seems that L.Draw.Circle#_drawShape

    _drawShape: function (latlng) {
        if (!this._shape) {
            this._shape = new L.Circle(this._startLatLng, this._startLatLng.distanceTo(latlng), this.options.shapeOptions);
            this._map.addLayer(this._shape);
        } else {
            this._shape.setRadius(this._startLatLng.distanceTo(latlng));
        }
    },

sets the radius as the result of L.LatLng#distanceTo that calculates the distance based on L.CRS.Earth

    // @method distanceTo(otherLatLng: LatLng): Number
    // Returns the distance (in meters) to the given `LatLng` calculated using the [Haversine formula](http://en.wikipedia.org/wiki/Haversine_formula).
    distanceTo: function (other) {
        return L.CRS.Earth.distance(this, L.latLng(other));
    },

which, when the map uses L.CRS.Simple instead, results in a really huge radius, making the circle shape unusable.

Other shapes seem to be working fine with L.CRS.Simple as these do not utilize L.LatLng#distanceTo but, for instance, use bounds instead.

@dcodeIO dcodeIO changed the title When using CRS.Simple for the map, drawing a circle creates a really huge unusable circle When using L.CRS.Simple for the map, drawing a circle creates a really huge unusable circle Oct 16, 2016
@germanjoey
Copy link

Hey,

As coincidence would have it, I'm also using L.CRS.Simple and ran into the same problem. Your diagnosis helped me solve it. The root issue isn't with Leaflet.Draw but with the coordinate system itself. Although the docs say LatLng is fine for L.CRS.Simple - and it does indeed work for many simple cases- the coordinate system actually doesn't make any sense for something like a flat, pixel-based world that has an arbitrary level of zoom, and thus its distance calculation returns what is essentially a nonsense value.

I think we basically have two options here:

1.) We can create a replacement for L.LatLng, and then a replacement for L.Projection.LonLat, and then finally plug that projection into a replacement for L.CRS.Simple. That's what I ended up doing, because my map has a weird wrapping attribute where it wraps up/down in addition to east/west. (it is a map of a torus). I can post this code if you think it will help.

2.) The second option is much easier: you can highjack L.LatLng and replace its distanceTo function like so:

L.LatLng.prototype.distanceTo = function (other) {
    var dx = other.lng - this.lng;
    var dy = other.lat - this.lat;
    return Math.sqrt(dx*dx + dy*dy);
}

Nice and easy, right? Ordinarily I would be aghast at this sort of patching, but once I realized that the root problem is that inappropriateness of LatLng to L.CRS.Simple I decided this approach was fine. Just make sure you include this code after you include leaflet and leaflet draw.

@dcodeIO
Copy link
Author

dcodeIO commented Oct 21, 2016

For completeness, besides disabling all length and area displays as these are also wrong, this is what I ended up with:

function euclidianDistance(a, b) {
    var dlat = b.lat - a.lat,
        dlng = b.lng - a.lng;
    return Math.sqrt(dlat * dlat + dlng * dlng);
}

L.Draw.Circle.prototype._drawShape = function (latlng) {
    if (!this._shape) {
        this._shape = new L.Circle(this._startLatLng, euclidianDistance(this._startLatLng, latlng), this.options.shapeOptions);
        this._map.addLayer(this._shape);
    } else {
        this._shape.setRadius(euclidianDistance(this._startLatLng, latlng));
    }
};

L.Edit.Circle.prototype._resize = function (latlng) {
    var moveLatLng = this._moveMarker.getLatLng(),
        radius = euclidianDistance(moveLatLng, latlng);
    this._shape.setRadius(radius);
    this._map.fire('draw:editresize', {layer: this._shape});
};

L.Draw.Rectangle.prototype._getTooltipText = function() {
    var tooltipText = L.Draw.SimpleShape.prototype._getTooltipText.call(this),
        subtext;
    return {
        text: tooltipText.text,
        subtext: subtext
    };
};

I like your fix more, though :)

@mikes360
Copy link

Hi Guys,

I am still seeing this issue. Has it still not been addressed?

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

No branches or pull requests

4 participants