sibartlett / Geo

A geospatial library for .NET
https://www.nuget.org/packages/Geo/
GNU Lesser General Public License v3.0
179 stars 39 forks source link

Envelope.Combine maybe faulty #5

Closed volkerr closed 10 years ago

volkerr commented 10 years ago

Hi there,

Geo looks like a very convenient lightweight implementation for geo data handling.

I tried to integrate the geo library into my windows store app for simple geographic calculations. However, the following behaviour seems strange to me:

        Point p1 = new Point(49, 49);
        Point p2 = new Point(50, 50);
        MultiPoint mp = new MultiPoint(p1, p2);
        Envelope e = mp.GetBounds();

This code produces an envelope with 49 in all properties of the envelope "e". Though, I expected the envelope to describe an area covering p1 and p2.

Having a look at the source code it seems strange to me, that combine uses Math.Min an all properties to combine two envelopes. Shouldn't it be Math.Max on the Max-properties?

Cheers Volker

    public Envelope Combine(Envelope other)
    {
        if (other == null)
            return this;

        return new Envelope(
            Math.Min(MinLat, other.MinLat),
            Math.Min(MinLon, other.MinLon),
            Math.Min(MaxLat, other.MaxLat),
            Math.Min(MaxLon, other.MaxLon)
        );
    }
sibartlett commented 10 years ago

I have pushed a new version to NuGet which fixes this. Thank you for the in-depth help.

volkerr commented 10 years ago

Hi, thanks for the immediate help. The source code looks fixed on github, however, the updated NuGet-package (0.11.1) still shows the same behaviour (envelope with all properties set to 49). I am using Visual Studio Express 2013.

I have removed the package from

Can you verify this?

Cheers Volker

volkerr commented 10 years ago

... maybe it is also a problem with the Assembly version which always shows 1.0.0.

assembly-properties geo

sibartlett commented 10 years ago

I just downloaded the latest package from NuGet, and decompiled it. The decompiled source matches the GitHub source. So it should work.

I'll write a test app or a unit test, and see what it does.

sibartlett commented 10 years ago

I have pushed a new version to NuGet again, and it now works.

volkerr commented 10 years ago

Thanks, now it works for me, too (0.11.2) Cheers