geosolutions-it / MapStore2

The solution to create and share maps, dashboards, geostories with 3D support on the web. And it is open-source too!
https://mapstore.geosolutionsgroup.com/
Other
487 stars 383 forks source link

Identify popup for Marker and Test annotations #7449

Open ElenaGallo opened 2 years ago

ElenaGallo commented 2 years ago

The Identify popup does not open for Marker and Test annotations annotations.gif

How to Reproduce

Current result The Identify popup does not open

tdipisa commented 2 years ago

@ElenaGallo the popup opens but it is really difficult for this kind of annotations to catch the exact point in map.

image

Anyway, yes, this is an issue to fix. Thank you.

ale-cristofori commented 2 years ago

The cause of the bug is the fixed buffer used to intersect the buffered query point and the annotation feature (which is also buffered in case it is a geometry point).

There is a TODO in the code MapStore2\web\client\reducers\mapInfo.js line 370 suggesting we adapt buffering to the geometry style, just for clarification @tdipisa when we say 'geometry style' do we mean the geometry type? (Point, Polygon, etc). In that case the buffer could be made bigger in case the intersecting feature geometry is of type Point. If there could be a more intelligent approach I would be open to suggestions.

tdipisa commented 2 years ago

The cause of the bug is the fixed buffer used to intersect the buffered query point and the annotation feature (which is also buffered in case it is a geometry point).

Thank you.

There is a TODO in the code MapStore2\web\client\reducers\mapInfo.js line 370 suggesting we adapt buffering to the geometry style, just for clarification @tdipisa when we say 'geometry style' do we mean the geometry type? (Point, Polygon, etc). In that case the buffer could be made bigger in case the intersecting feature geometry is of type Point. If there could be a more intelligent approach I would be open to suggestions.

I'm not sure, please clarify that part with @offtherailz. Thank you. For what concern the estimate, you can proceed with the fix.

ale-cristofori commented 2 years ago

Issue investigation findings.

Cause of the bug: When performing an identify with hover event on an overlay layer we do not use the in-built functionalities of our mapping libraries (OL and Leaflet) we listen the pointermove (Openlayers) and mousemove (Leaflet) events and we proxy the found coordinates to a redux action featureInfoClick that in turn triggers getVectorInfo, returning the annotations attribute to display on pop-ups, see below the Redux actions stack

image

This approach does not consider the markers and text as objects on the map, differently from polygons which geometries are directly represented on the map, markers and text are un-georeferenced icons attached to an anchor point invisible in the map. This is why the identify works for such features when we are at map large scales or we try to hover near this anchor point, placed at the bottom centre of the marker icon or text.

Temporary Solution for Openlayers: (this approach is used already for the annotations identify on click) when coordinates are received from the pointermove event before being sent to the processing described above we use OL forEachFeatureAtPixel (which intersects markers) to modify the coordinates at which the event generated (see MapStore2\web\client\components\map\leaflet\Map.jsx lines 202 and 474) to 'lock' the clicked or hovered event coordinates to the coordinates of the anchor point.

Temporary Solution for Leaflet: The mousemove event would be the 'pointermove' counterpart of Leaflet. The same process of proxying and locking the event coordinates to the feature applies but differently from the OL map this method is part of the Layer class rather than the Map class and is already implemented for the click event (see MapStore2\web\client\components\map\leaflet\Feature.jsx line 106). In our help we could create a listener for a mouseover event, attached to the layer instance and recalculate the event coordinates same as we do for the click event.

Potential alternative/ideal solution We could redesign the identify functionality for markers, using the methods of forEachFeatureAtPixel in Openlayers and mousemove in Leaflet to extract the attributes directly from the feature (letting the libraries do the job) and plug the results in the existing workflow to fill the texts for the popups and panels displaying the identify results. This solution should be thoroughly discussed because it would involve a relevant chunk of existing code investigation/refactoring besides its feasibility assessment, taking into account the reasons why we have the current approach in place.

offtherailz commented 2 years ago

@tdipisa I've seen that should have solved the part with OpenLayers. We can

tdipisa commented 2 years ago

@offtherailz the second one as I have told to @ale-cristofori. Thank you.