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

Envelope.Combine maybe faulty #5

Closed
volkerr opened this issue Dec 29, 2013 · 6 comments
Closed

Envelope.Combine maybe faulty #5

volkerr opened this issue Dec 29, 2013 · 6 comments

Comments

@volkerr
Copy link

volkerr commented Dec 29, 2013

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
Copy link
Owner

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

@volkerr
Copy link
Author

volkerr commented Dec 29, 2013

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

  • package.info
  • references
  • the filesystems and
  • cleared the NuGet package cache.
    Still, the newly downloaded package seems to contain the reported problem.

Can you verify this?

Cheers
Volker

@volkerr
Copy link
Author

volkerr commented Dec 29, 2013

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

assembly-properties geo

@sibartlett
Copy link
Owner

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 added a commit that referenced this issue Dec 29, 2013
@sibartlett
Copy link
Owner

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

@volkerr
Copy link
Author

volkerr commented Dec 29, 2013

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

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

No branches or pull requests

2 participants