Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

LatLng.wrap() should not wrap max longitude to min value #10767

Closed
osana opened this issue Dec 20, 2017 · 0 comments · Fixed by #10769
Closed

LatLng.wrap() should not wrap max longitude to min value #10767

osana opened this issue Dec 20, 2017 · 0 comments · Fixed by #10769
Assignees
Labels
Android Mapbox Maps SDK for Android

Comments

@osana
Copy link
Contributor

osana commented Dec 20, 2017

* Return a new LatLng object with a wrapped Longitude. This allows original data object

Two issues with this method:

  1. Method comments say that new LatLng will be created. Current implementation returns this
  2. wrap() from telemetry.MathUtils is used which relies on an implementation of wrap from core.
   * Constrains value to the given range (including min, excluding max) via modular arithmetic.
   * <p>
   * Same formula as used in Core GL (wrap.hpp)
   * std::fmod((std::fmod((value - min), d) + d), d) + min;
   *
   * @param value Value to wrap
   * @param min   Minimum value
   * @param max   Maximum value
   * @return Wrapped value
   */
  public static double wrap(double value, double min, double max) {
    double delta = max - min;

    double firstMod = (value - min) % delta;
    double secondMod = (firstMod + delta) % delta;

    return secondMod + min;

excluding max is what is important here.

For longitude 180 -> that will wrap to -180 even though max value is 180.


To fix this:

  1. LatLng.wrap() should return a new LatLng
    double delta = max - min;

    double firstMod = (value - min) % delta;
    double secondMod = (firstMod + delta) % delta;

   if (value > min && second == 0) {
      return max;
   }
    return secondMod + min; 

cc @asheemmamoowala @tobrun @cammace

@osana osana added the Android Mapbox Maps SDK for Android label Dec 20, 2017
@osana osana added this to the android-v6.0.0 milestone Dec 20, 2017
@osana osana self-assigned this Dec 20, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant