albar965 / littlenavmap

Little Navmap is a free flight planner, navigation tool, moving map, airport search and airport information system for Flight Simulator X, Microsoft Flight Simulator 2020, Prepar3D and X-Plane.
https://albar965.github.io/littlenavmap.html
GNU General Public License v3.0
1.3k stars 163 forks source link

Web API cached map features retrieval via Rect param #790

Closed KOKAProduktion closed 2 years ago

KOKAProduktion commented 3 years ago

Hiho @albar965,

just when i thought I'm over the hill... :)

Adding clickable features to the map requires retrieving them by a Rect via the API. I've managed to do that already but I've stumbled upon the fact, that MapQuery applies caching (e.g. airportCache)...

Meaning that I'm able to retrieve features, but only those which are visible inside the in-app map instead of my provided Rect if I omit updating the cache. Doing so would make me feel uncomfortable, since it's the in-apps map cache...

POI: https://github.com/KOKAProduktion/littlenavmap/blob/c89da645a63c8534ca7bef59ab4e8559c3243b09/src/query/mapquery.cpp#L734-L759

Also a MapLayer param is required for updating the cache and I'm afraid i don't understand the MapLayers role in the setup as well as where to get the right one from.

Would you have some insights, hints or suggestions where to look further? Should i implement a separate cache for the API map feature calls?

Thanks for your time and cheers!

albar965 commented 3 years ago

Yeh. These caches. :frowning_face: An early and overblown design decision I'd like to get rid of. Thinking about loading all airport and navaid information into a spatial index in memory in the future.

Would it help to add a cache option to the constructor of MapQuery which disables all caching? Then you can use your own instance of the query object.

The map layer can be taken from MapPaintLayer which is a member of MapPaintWidget.

KOKAProduktion commented 3 years ago

I see. Thanks!

Would it help to add a cache option to the constructor of MapQuery which disables all caching?

Yeah, if it's OK to omit the cache in terms of performance. Is it easy for you to implement that or should I take care of it?

Thanks also for the map layer hint! Cheers!

albar965 commented 3 years ago

Disabling the cache would break MapQuery::getNearestScreenObjects() which is built to avoid database queries. Do you need this method?

KOKAProduktion commented 3 years ago

Not using this method at the moment. I'll worry later on, when it becomes necessary :)

Thanks!

albar965 commented 3 years ago

Ok. I think the map query has to be a member of the MapWidget since its cache is related to it. I'll change this so that there is a separate map query for each MapWidget instance. This way you get the right cached objects from the last map display request to "your" web interface related MapWidget.

albar965 commented 3 years ago

Done. I changed the ownership of all classes that have MapPaintWidget related caches. MapQuery, AirwayTrackQuery and WaypointTrackQuery are now aggregates of MapPaintWidget. If you do not GUI related stuff you have to get the queries from the MapPaintWidget. You should review the usage of all NavApp::getMapWidgetGui(), NavApp::getMapQueryGui(), NavApp::getAirwayTrackQueryGui() and NavApp::getWaypointTrackQueryGui(). Note: I added the GUI suffix to indicate that these are in ownership by the main MapWidget.

What is left:

  1. Refactor the MapPaintWidget related caches and methods out of the query classes
  2. Airspace queries are not adapted yet.
albar965 commented 3 years ago

Hold a bit with the update. It's crashing on Windows.

albar965 commented 3 years ago

Fixed so far.

KOKAProduktion commented 3 years ago

Hi @albar965,

wow, very cool! Thanks! Can't wait to check it out!

Cheers!

albar965 commented 3 years ago

Hope it helps. It is a tiny step for more than one map display in LNM. Something like an inset in the GUI might be possible. I'm wondering why the old approach with the overlapping caches worked at all. :upside_down_face:

KOKAProduktion commented 3 years ago

Works perfectly! Thanks!

KOKAProduktion commented 3 years ago

Hi @albar965,

may i ask you again? I've successfully implemented correct airports retrieval via Rect. However VOR, NDB and Markers retrieval using the same pattern return "weird" results. It appears to me, that they misinterpret the provided Rect and return way too many or no results with coordinates in- and outside the original Rect depending on the distance value. It looks like the Rects coordinates are somehow modulated by distance...

I must add that I'm converting from atools::geo::Rect to a GeoDataLatLonBox

GeoDataLatLonBox latLonBox = GeoDataLatLonBox(rect.getNorth(),rect.getSouth(),rect.getEast(), rect.getWest());

So maybe I have introduced this issue myself... Although this works with the airports retrieval.

Did you encounter anything like this before?

Thanks and cheers!

albar965 commented 3 years ago

The request overloads to allow scrolling and zooming on the map without reloading. So, returning more than the requested results is ok. Make sure that the parameter lazy in the MapQuery call is set to true to avoid incomplete results.

And keep in mind that the GeoDataLatLonBox uses radian instead of degree (I prefer degree since it is easier to read while debugging). You have to add GeoDataCoordinates::Degree to the constructor in your example. This also applies for most setters and getters in the Marble geometry classes.

KOKAProduktion commented 2 years ago

You have to add GeoDataCoordinates::Degree to the constructor in your example

Thanks, sounds plausible! I'll try that right away! Cheers

KOKAProduktion commented 2 years ago

Looks good! Thanks!