-
-
Notifications
You must be signed in to change notification settings - Fork 357
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
always return complete ways on getMapDataWithGeometry #4988
always return complete ways on getMapDataWithGeometry #4988
Conversation
MapDataCache tests work now, and a test is added whether a node outside the bbox is returned if it's part of a way that also has nodes inside the bbox. ElementsDaoTest.getAllElementsByBbox still fails, and here some more tests should be added (maybe I'll try tomorrow, but feel free to work on this). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As you say, needs a few tests for the Dao. Additionally, I found some things that could be done to make it faster (again).
app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataCache.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataCache.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataCache.kt
Show resolved
Hide resolved
app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataCache.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataCache.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataCache.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/de/westnordost/streetcomplete/data/osm/mapdata/MapDataCache.kt
Outdated
Show resolved
Hide resolved
I think with the latest test this is complete now |
Awesome, thank you! FYI I released the beta already yesterday to stay true to the announcement in the changelog from v52.1 that the beta will be released on the 3rd but included a note that it is known that the feature dependent on this are not fully functional yet. |
fixes #4980
MapDataCache
can now fetch nodes if for some reason it's neither inSpatialCache
nor innodeCache
. This can be because of trim (simply clearsnodeCache
), but also for other reasons, e.g.SpatialCache
removing tiles unexpectedly (see #4985).Performance:
getMatDataWithGeometry
is slowerupdate
is typically around 10% slower due to properly handlingnodeCache
(25% observed, this was by far the worst case)tilesRect.asBoundingBox(zoom).enclosingTilesRect(zoom)
issuenodeCache
, so even after trim this fetch does not happen oftengetMatDataWithGeometry
IMO the performance impact is acceptable, and likely any improvements will not be worth the effort.
Some work is still to be done: No new tests are added, and some old tests fail now.