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

Annotations are not selected when map is rotated #15082

Closed
cbo1964 opened this issue Jul 9, 2019 · 5 comments · Fixed by #15097
Closed

Annotations are not selected when map is rotated #15082

cbo1964 opened this issue Jul 9, 2019 · 5 comments · Fixed by #15097
Labels
documentation feature iOS Mapbox Maps SDK for iOS

Comments

@cbo1964
Copy link

cbo1964 commented Jul 9, 2019

If you add an Annotation (MGLPointAnnotation) then when the map is rotated the Annotation is not selected (in fact not found by the "- (MGLAnnotationTag)annotationTagAtPoint:(CGPoint)point persistingResults:(BOOL)persist").

Steps to reproduce

  1. Using the demo App
  2. Make a long press gesture and add a pin
  3. Tap on the pin with map not rotated, it is detected and selected ("- (BOOL)mapView:(__unused MGLMapView *)mapView annotationCanShowCallout:(__unused id )annotation" is called)
  4. rotate the map (90# is fine but seems not so important)
  5. tap on the pin and the pin it is NOT detected

Expected behavior

I expect that the same Annotation is found in "any map conditions": rotated, tilted.

Actual behavior

The MGLPointAnnotations are not detected when the map is rotated

Configuration

Mapbox SDK versions: 5.1.0
iOS/macOS versions: iOS 12.3.1
Device/simulator models: iPad Pro
Xcode version: 10.2.1

@julianrex julianrex added feature iOS Mapbox Maps SDK for iOS documentation labels Jul 9, 2019
@julianrex
Copy link
Contributor

Thanks for your feedback @cbo1964.

Our documentation here is misleading and currently slightly inaccurate: See MGLCalloutView.dismissesAutomatically - it states

A Boolean value indicating whether the callout view should be dismissed automatically

This should read whether the annotation is automatically deselected (and in the process dismisses the callout).

Please try implementing dismissesAutomatically so that it returns NO to see if that solves your issue. Also...

Note that a single tap on the map view still dismisses the callout view regardless of the value of this property.

Leaving this issue open as a documentation task and potential feature improvement.

@cbo1964
Copy link
Author

cbo1964 commented Jul 9, 2019

Hi,
maybe I was not able to preperly explain the issue but I think there is a misunderstanding.

The problem is the detection of the MGLAnnotation when you tap (and not the callout).
What I mean is that the

  • (MGLAnnotationTag)annotationTagAtPoint:(CGPoint)point persistingResults:(BOOL)persist
    function in the MGLMapView, when calling the
    std::vector nearbyAnnotations = [self annotationTagsInRect:queryRect];
    does not find any annotation under the tap if the map is rotated, whereas the same function, with the same data found the annotation if the map it is not rotated (rotated using the 2 finger rotation gesture).

Moreover, I was having other problems with the Annotation and I'm debugging a little bit the code and I found something that I'm not sure it is related with above issue: if you zoom (with pinch) at zoom levels that are not perfectly aligned (not an integer value) annotations are also not found.
What I have seen is that in the
std::pair<bool,bool> CollisionIndex::placeFeature(CollisionFeature& feature, ...

function, the calculated projected point
const auto projectedPoint = projectAndGetPerspectiveRatio(posMatrix, box.anchor);
is wrong (sensibly wrong) when you are in this "in between zoom",

Maybe there is something I'm doing wrong (highly probable because I'm not expert in Mapbox) but I worked in the past with RouteMe and Mapnik and other libraries like these and I cannot understand what I'm doing wrong.

Thanks

@julianrex
Copy link
Contributor

Thanks for the clarification @cbo1964!

@cbo1964
Copy link
Author

cbo1964 commented Jul 10, 2019

I think this problem is the same than #14977

astojilj added a commit that referenced this issue Jul 10, 2019
To changelog:
Fixed incorrect center coordinate after pinch regression caused by edge insets fix (#14664).

While working on #14664, missed to understand the logic used in

```
                CLLocationCoordinate2D centerCoordinate = _previousPinchCenterCoordinate;
                mbgl::EdgeInsets padding { centerPoint.y, centerPoint.x, self.size.height - centerPoint.y, self.size.width - centerPoint.x };
                self.mbglMap.jumpTo(mbgl::CameraOptions()
                                        .withCenter(MGLLatLngFromLocationCoordinate2D(centerCoordinate))
                                        .withPadding(padding));

```

Replacing this code by moveBy achieves the required translation.

Fixes: #14977, #15082
@julianrex
Copy link
Contributor

PR for this issue: #15097

friedbunny pushed a commit that referenced this issue Jul 13, 2019
To changelog:
Fixed incorrect center coordinate after pinch regression caused by edge insets fix (#14664).

While working on #14664, missed to understand the logic used in

```
                CLLocationCoordinate2D centerCoordinate = _previousPinchCenterCoordinate;
                mbgl::EdgeInsets padding { centerPoint.y, centerPoint.x, self.size.height - centerPoint.y, self.size.width - centerPoint.x };
                self.mbglMap.jumpTo(mbgl::CameraOptions()
                                        .withCenter(MGLLatLngFromLocationCoordinate2D(centerCoordinate))
                                        .withPadding(padding));

```

Replacing this code by moveBy achieves the required translation.

Fixes: #14977, #15082
astojilj added a commit that referenced this issue Jul 14, 2019
To changelog:
Fixed incorrect center coordinate after pinch regression caused by edge insets fix (#14664).

While working on #14664, missed to understand the logic used in

```
                CLLocationCoordinate2D centerCoordinate = _previousPinchCenterCoordinate;
                mbgl::EdgeInsets padding { centerPoint.y, centerPoint.x, self.size.height - centerPoint.y, self.size.width - centerPoint.x };
                self.mbglMap.jumpTo(mbgl::CameraOptions()
                                        .withCenter(MGLLatLngFromLocationCoordinate2D(centerCoordinate))
                                        .withPadding(padding));

```

Replacing this code by moveBy achieves the required translation.

Fixes: #14977, #15082
@friedbunny friedbunny added this to the release-picklejuice milestone Jul 15, 2019
astojilj added a commit that referenced this issue Jul 19, 2019
To changelog:
Fixed incorrect center coordinate after pinch regression caused by edge insets fix (#14664).

While working on #14664, missed to understand the logic used in

```
                CLLocationCoordinate2D centerCoordinate = _previousPinchCenterCoordinate;
                mbgl::EdgeInsets padding { centerPoint.y, centerPoint.x, self.size.height - centerPoint.y, self.size.width - centerPoint.x };
                self.mbglMap.jumpTo(mbgl::CameraOptions()
                                        .withCenter(MGLLatLngFromLocationCoordinate2D(centerCoordinate))
                                        .withPadding(padding));

```

Replacing this code by moveBy achieves the required translation.

Fixes: #14977, #15082
astojilj added a commit that referenced this issue Jul 23, 2019
To changelog:
Fixed incorrect center coordinate after pinch regression caused by edge insets fix (#14664).

While working on #14664, missed to understand the logic used in

```
                CLLocationCoordinate2D centerCoordinate = _previousPinchCenterCoordinate;
                mbgl::EdgeInsets padding { centerPoint.y, centerPoint.x, self.size.height - centerPoint.y, self.size.width - centerPoint.x };
                self.mbglMap.jumpTo(mbgl::CameraOptions()
                                        .withCenter(MGLLatLngFromLocationCoordinate2D(centerCoordinate))
                                        .withPadding(padding));

```

Replacing this code by moveBy achieves the required translation.

Fixes: #14977, #15082
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation feature iOS Mapbox Maps SDK for iOS
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants