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

LatLngBounds.contains(LatLong) can return always 'false' #11137

Closed
klblk opened this issue Feb 7, 2018 · 7 comments · Fixed by #11219
Closed

LatLngBounds.contains(LatLong) can return always 'false' #11137

klblk opened this issue Feb 7, 2018 · 7 comments · Fixed by #11219
Assignees
Labels
Android Mapbox Maps SDK for Android

Comments

@klblk
Copy link

klblk commented Feb 7, 2018

Platform: Android
Mapbox SDK version: 5.4.0 (not reproduced on 5.0.2)

Steps to trigger behavior

  1. LatLngBounds latLngBounds = mapboxMap.getProjection().getVisibleRegion().latLngBounds
  2. latLngBounds.toString()
  3. Sametime (change map rotation angle): "N:55.99722274440842; E:92.80566583823929; S:56.00073056968671; W:92.77899862655636"
    4.method: LatLngBounds.contains(ILatLng latLng) return always 'false', because latitudeNorth < latitudeSouth
public boolean contains(final ILatLng latLng) {
    final double latitude = latLng.getLatitude();
    final double longitude = latLng.getLongitude();
    return ((latitude <= this.latitudeNorth)
      && (latitude >= this.latitudeSouth))
      && ((longitude <= this.longitudeEast)
      && (longitude >= this.longitudeWest));
  }
@tobrun tobrun added the Android Mapbox Maps SDK for Android label Feb 8, 2018
@tobrun
Copy link
Member

tobrun commented Feb 8, 2018

@osana would you be able 👀 this issue? You have recently worked on this class and its business logic.

@osana osana self-assigned this Feb 8, 2018
@osana
Copy link
Contributor

osana commented Feb 8, 2018

@klblk what are the LatLng bounds that are passed into contains() method of LatLngBounds object with values that you specified?

@klblk
Copy link
Author

klblk commented Feb 9, 2018

Sorry, I did not understand the question.
My case:
Determine user sees LatLng point in screen or not.

For Mapbox 5.0.2 and less, i use VisibleRegion.latLngBounds, and contains() method. But this broken in 5.4.0 if rotate map (not tested other version).

My current solution for 5.4.0:
I not use VisibleRegion.latLngBounds, and create LatLngBounds through method:

private static LatLngBounds visibleLatLngBounds(VisibleRegion visibleRegion) {
        return new LatLngBounds.Builder()
                .include(visibleRegion.farLeft)
                .include(visibleRegion.farRight)
                .include(visibleRegion.nearLeft)
                .include(visibleRegion.nearRight)
                .build();
    }

@osana
Copy link
Contributor

osana commented Feb 9, 2018

@klblk Thank you for clarifying. I thought that the bug was against incorrectly working contains method.

@osana
Copy link
Contributor

osana commented Feb 15, 2018

@klblk I am confirming that indeed this is reproducible in master.

@osana
Copy link
Contributor

osana commented Feb 16, 2018

@tobrun The source of the problem is a combination of implementation of Projection.getVisibleRegion()

return new VisibleRegion(topLeft, topRight, bottomLeft, bottomRight,

and implementation of
LatLngBounds.fromLatLngs()

If map is rotated topLeft of the VisibleRegion will not be the actual northWest, and so on.

@osana
Copy link
Contributor

osana commented Feb 16, 2018

@tobrun A couple of action items / further discussions out of this

  1. VisibleRegion takes farLeft, farRight, nearLeft, nearRight and latlngBounds which are supposed to be The smallest bounding box that includes the visible region defined in this class. There are no checks to ensure if that is indeed the case. Should we do some checks in the constructor?

  2. We should add a check to LatLngBounds constructor: throw an IAE if north < south

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.

3 participants