Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sometimes the map doesn't show all the markers #378

Open
wbazant opened this issue May 25, 2024 · 6 comments
Open

sometimes the map doesn't show all the markers #378

wbazant opened this issue May 25, 2024 · 6 comments
Assignees
Labels
enhancement New feature request epic Very important big issue performance Related to interface speed and responsiveness

Comments

@wbazant
Copy link
Contributor

wbazant commented May 25, 2024

Spotted with maples in Clemson University (again when testing something else).

First observation, these are different dots:
http://beta.fallingfruit.org/map/@34.676566103490984,-82.83368184902955,16z
http://fallingfruit.org/?z=16&y=34.67628&x=-82.83776&m=true&t=roadmap&l=false&locale=en&c=forager,freegan

Secondly, playing around with the filter suggests that a map has a quiet maximum number of markers that it's willing to display, and the "Tree inventories" feature is enough to trigger it. See these selections of various maples:
Screenshot from 2024-05-25 12-43-52
Screenshot from 2024-05-25 12-43-34
Screenshot from 2024-05-25 12-43-13

@wbazant
Copy link
Contributor Author

wbazant commented May 25, 2024

In src/redux/viewChange.js we decide whether to show clusters or not by:

export const getIsShowingClusters = (state) =>
  state.map.view.zoom <= VISIBLE_CLUSTER_ZOOM_LIMIT

Proposed resolution strategy:

  • find out where the marker limit comes from and whether it can be configured
  • find out how to recognise when the app gets more data than it can show on the map: maybe the result of /counts?
  • if the app the realises it has too much data to show, it asks for clusters instead of locations

Actually it could also work entirely on counts. For rare species, we don't need to show clusters in a zoomed out location, for example this could just be a dot instead of a cluster with one location:
Screenshot from 2024-05-25 13-19-49
but the UX of zooming into the cluster after clicking on it is very helpful, so some consideration would need to be given to that.

@wbazant
Copy link
Contributor Author

wbazant commented May 26, 2024

related to #133

@ezwelty
Copy link
Collaborator

ezwelty commented May 31, 2024

@wbazant The logic for this is that the tree inventory data can be so dense that transferring all the locations in a request, and rendering the locations on the map, would slow or crash the app. So we devised the pre-computed clusters, which reduce the locations in an area to a count and a center of mass. They are computed on a nested quadtree down currently to zoom level 13.

Theoretically I might be able to precompute them at a variable depth, but easiest might be to first see down to what zoom level I would need to go for it to be sufficient in even the densest areas. Then maybe the response from types/counts endpoint could be used to decide whether to request clusters or locations?

@wbazant
Copy link
Contributor Author

wbazant commented Jun 1, 2024

I think what's left of the problem is now the presentation aspect - the map can't draw all the markers, or they are placed so densely they are hard to read. I was thinking about having the client display some of the received locations as markers, until I realised it's not possible to do that on the edge of the map, it might be slow, and the result would move as the map moves, which is not good

Then maybe the response from types/counts endpoint could be used to decide whether to request clusters or locations?

Does the problem get easier if we allow the response to produce a mix of clusters and locations? It could overall be nice also for sparsely annotated areas, and even on the furthest zoom there are sometimes clusters of size 1, which could be just a location marker.

@ezwelty
Copy link
Collaborator

ezwelty commented Jun 9, 2024

Does the problem get easier if we allow the response to produce a mix of clusters and locations? It could overall be nice also for sparsely annotated areas, and even on the furthest zoom there are sometimes clusters of size 1, which could be just a location marker.

The clusters are computed on the same (but I believe denser by a factor of 2) quad-tree which is used for the square base map tiles in web maps. I can imagine a scheme where, for each grid-cell, the API returns either locations or the single cluster. I should run some tests with the database to see what is possible performance-wise.

@ezwelty ezwelty self-assigned this Jul 3, 2024
@ezwelty ezwelty added enhancement New feature request epic Very important big issue performance Related to interface speed and responsiveness labels Jul 3, 2024
@wbazant
Copy link
Contributor Author

wbazant commented Jul 12, 2024

I still like the dynamic clusters / locations improvement, but I also had a thought about this issue when looking at the code that fetches the locations in the map slice - it uses a limit:

src/redux/mapSlice.js
33:    await getLocations(selectParams(getState(), { limit: 250 })),

The limit should probably stay, because returning arbitrarily large payloads is leading to all sorts of problems, but I realised we could reduce severity of the bug by toasting a warning about the map being incomplete whenever getLocations returns 250 entries - then the user can zoom in or adjust type filters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature request epic Very important big issue performance Related to interface speed and responsiveness
Projects
None yet
Development

No branches or pull requests

2 participants