mapzen / eraser-map

Privacy-focused mapping application for Android
GNU General Public License v3.0
74 stars 24 forks source link

Single tap produces inconsistent map marker placement. #775

Closed msmollin closed 7 years ago

msmollin commented 7 years ago
msmollin commented 7 years ago

Additional testing shows that this only happens when the marker is placed using a touch. Searching for this school or any other POI and having the marker dropped in from there works fine, and produces none of this effect.

ecgreb commented 7 years ago

Nice video! I am also able to repro the issue in the latest build. Testing the store version does not show this issue.

We should only be reverse geocoding and dropping a pin on a non-marker location in response to long press.

Single tap should only produce a pin when its on a map icon and then the pin should be centered on the icon.

ecgreb commented 7 years ago

As a workaround let's try ignoring invocations of the FeaturePickListener.onFeaturePick(...) callback where the properties argument is empty.

Tangram ES work toward a more permanent fix is tracked being tracked here https://github.com/tangrams/tangram-es/pull/1076

ecgreb commented 7 years ago

Overview

The desired behavior here is to display the search result view on single tap only if the POI currently has an icon and label displayed on the map. In Tangram 0.4.3 the FeaturePickListener.onFeaturePick(Map<String, String> properties, float positionX, float positionY) callback would only fire for such features on a single tap. A single tap anywhere on the map that did not include an icon and label would not fire the callback.

In Tangram 0.4.5 the behavior has changed so that a single tap on any feature will invoke the feature pick callback regardless of whether it has a label or icon currently displayed on the map. This has led to confusion in the app since new a single tap literally anywhere on the screen invokes the feature pick callback and will display a corresponding search result.

All known properties are returned by Tangram for the feature tapped. If there are no known properties then the properties argument returned is an empty HashMap.

In order to fix this POI selection feature in Eraser Map we need to be able to filter results delivered via the feature pick callback to only POIs with an icon and label showing on the map. Currently this does not seem possible using the properties value returned and will likely require a fix in Tangram ES.

For reference I have included notes for several use cases below.

Use cases

Given the following starting position on the map I performed a single tap on 4 different kinds of features.

  1. Building with icon and label (Bucks County Justice Center)
  2. Building without icon and label (NAPA/Fred Beans Auto Parts)
  3. Minor road with label (Harvey Ave.)
  4. Open space with no visible feature (Area between Wood St. and Decatur St.)

image

Building with icon and label (Bucks County Justice Center)

A single tap anywhere on the Bucks County Justice Center produces the following properties and resulting pin is displayed on the map. This the type of feature for which we want this behavior enabled.

The only problem here is the coordinates returned in the feature pick callback are no longer centered on the feature selected. Instead they are the raw coordinates for the location of the single tap which results in the pin appearing off center.

screenshot 2016-11-18 at 3 54 23 pm

image

Building without icon and label (NAPA/Fred Beans Auto Parts)

A single tap anywhere on the NAPA/Fred Beans Auto Parts building at the corner of N. Broad and Doyle also produces a callback with properties despite not displayed an icon and/or label on the map.

We do not want single tap to trigger this behavior for buildings that do not have an icon or label. But since the properties argument is not empty we cannot simply filter results based on that. Is there something else in the properties that will tell us if this building has an icon and label?

screenshot 2016-11-18 at 3 56 11 pm

image

Minor road with label (Harvey Ave.)

Clicking on a street label produces a result similar to the building without label and icon. Again the properties returned are not empty but do include "kind" -> "minor_road". Perhaps this could be used to filter out non-POI results?

screenshot 2016-11-18 at 3 57 29 pm

image

Open space with no visible feature (Area between Wood St. and Decatur St.)

Finally I tested tapping on some open space with no visible feature. This produced a callback and the properties were empty so it would be easy to filter out this type of result. Notice it also produces the strangest behavior since the pin is placed off the visible screen. This is because the coordinates returned are 0, 0 which then get used for a reverse geocode producing unpredictable results.

screenshot 2016-11-18 at 3 58 38 pm

image

image

Questions

  1. Is it possible to implement a workaround for POI selection using the current version of Tangram ES 0.4.5?
  2. What is the desired behavior for FeaturePickListener.onFeaturePick(Map<String, String> properties, float positionX, float positionY) moving forward? Should it even get invoked for features the have 0 properties?
  3. Do we need an additional interface to notify the application when a POI with label and icon is single tapped? If so should the coordinates returned be geographic center of the feature?
ecgreb commented 7 years ago

Relevant code that handles feature pick callback:

https://github.com/mapzen/eraser-map/blob/master/app/src/main/kotlin/com/mapzen/erasermap/controller/MainActivity.kt#L288-L298

https://github.com/mapzen/eraser-map/blob/master/app/src/main/kotlin/com/mapzen/erasermap/presenter/MainPresenterImpl.kt#L707-L714

matteblair commented 7 years ago

Possible answers

  1. Is it possible to implement a workaround for POI selection using the current version of Tangram ES 0.4.5?

    Yes, and it will require changes to the bubble-wrap scene file (cc @nvkelso). If you open up the current bubble-wrap.yaml and control-f for "interactive", you will see that lots of layers are marked as interactive, including, apparently, all roads and all buildings. The scene file should be edited to make only POI labels interactive if that is the application behavior we're aiming for. This is one of those cases where the scene file creeps out of the territory of "styling" and into "app logic". This overlap is a little confusing and I'm open to suggested revisions.

  2. What is the desired behavior for FeaturePickListener.onFeaturePick(Map<String, String> properties, float positionX, float positionY) moving forward?

    After having this interface around for a while, I can think of some changes that seem like improvements to me.

    • MapController$pickFeature should take some kind of Query object as its argument.
    • Rather than having one listener to receive every callback, the Query object would contain the listener to invoke for that result.
    • FeaturePickListener$OnFeaturePick should receive both the original Query object and a Result object which could indicate that no features were picked.

    Should it even get invoked for features the have 0 properties?

    I think we should guarantee that every query produces a callback. If we allow that some queries will never invoke a callback then it seems that "no callback" could either mean "there were no results" or "this query hasn't run yet". I know that this sort of "guaranteed callback" behavior would make a lot of job interviews more clear :)

  3. Do we need an additional interface to notify the application when a POI with label and icon is single tapped? If so should the coordinates returned be geographic center of the feature?

    I can see two reasonable options. Either we keep one interface and the Result object mentioned earlier would indicate the type of result (icon/text/feature) or we create two interfaces with a different Result type for each. Personally I would prefer the second option.