mapbox / mapbox-unity-sdk

Mapbox Unity SDK - https://www.mapbox.com/unity/
Other
720 stars 213 forks source link

Map mesh generation needs to be multi-threaded. #1332

Open rygo6 opened 5 years ago

rygo6 commented 5 years ago

I am using mapbox in a VR application and unfortunately I found mapbox to be very prohibitive to use in VR because it appears like all processing for a tile is done on the main thread. I have many of the 3D map features turned on, and set to be combined, so I assume there is a lot of mesh processing going on. It causes frame hiccups when you scroll. Then when you zoom in enough to require all tiles to be flushed and loaded for that zoom level it completely freezes everything for a good quarter second or more. This makes it unusable in a VR application. My workaround so far has been to just limit the zoom level so that you cannot zoom so far as to require a reloading of every tile. I'll just have to deal with the frame hiccups as you scroll. But really this is an important thing if mapbox is going to carry any relevance in the world of wearable computing. All tile processing and mesh generation needs to be multithreaded so as to not cause frame hiccups. I've written a system like mapbox before for an internal client project and know for certain you can multithread this type of system, then bucket and stagger the mesh/texture uploads to cause zero frame loss. Fortunately and unfortunately mapbox has become way more developed in every regard compared to that system I made, so I would prefer to use mapbox. But if multi-threading doesn't end up on an official roadmap sometime soon, we may end up having to focus on a different solution to maps for wearable computing projects.

abhishektrip commented 5 years ago

@rygo6 Our SDK is built to take advantage of Unity's coroutines and avoid any frame drops and performance issues.

That said, the performance bottleneck we have noticed is due to Unity's Texture2D.LoadImage method which is blocking and runs on the main thread causing dips in frame-rate. This a known issue on the Unity end.

This issue is very important to us and we are constantly looking to improve the performance of our SDK.

rygo6 commented 5 years ago

Even with all texture data set to none it still has this same issue. I think it's just the main thread being dumped with a bunch of mesh generation calc. Maybe also with network requests? Coroutines aren't multithreaded, complex calculation put into a unity coroutine will still disrupt framerate. All network requests and all mesh generation could be put into separate threads, then the main thread only minimally used each frame to upload newly generated or received map data to the GPU.

kjyv commented 4 years ago

I'm also noticing these hiccups and found that it is LoadImage that is causing the delays. Proper multithreading (and as rygo6 said not coroutines) could probably help. However, it doesn't seem like there is much caching going on, there are at max only 9 tiles existing and whenever new tiles need to be shown, LoadImage is called again and again on (possibly cached) image files. Why not keep those tiles loaded and disabled or keep the Texture2D objects? There could be a setting how many tiles at max before old ones (by time of last usage) are destroyed. Also they could asynchronously be preloaded for close zoom levels and locations around the current one. That would already help a lot I think without any multithreading worries.

dblatner commented 4 years ago

Indeed, the proper way is to use async-await instead of coroutines, but I also suggest to instantiate a max of X mesh generations at a time (e.g. X = 4 or 8) and queue the mesh generation so not all meshes start generating at the same time

kjyv commented 4 years ago

@dblatner: async-await still runs on the main thread same as Coroutines and is also not the right choice (probably has the same performance). It is more suited if the main thread is waiting for IO or other things that are not using the CPU much. Trying a separate thread for each LoadImage is probably best for starters

dblatner commented 4 years ago

@kjyv you're right, but let me add a note: check out the AsyncAwaitUtil Plugin for Unity3D. You can run async background tasks with Task.Run( async () => { <your background code here, remember not to call Unity3d features, as they don't work outside the main thread> } );

The AsyncAwaitUtil even allows to switch between a background and main thread via simple 'await' extensions.