vizicities
vizicities copied to clipboard
UDST
Loaded building data should not clear when map is refreshed
When the map view is moved and the BlueprintHelperTileGrid emits a "moved" event, BlueprintOutputBuildingTiles clears out all its meshes before loading up the new ones. This causes most of the buildings in view to disappear and re-appear.
BlueprintOutputBuildingTiles should probably keep a list of which meshes belong to which grid tiles, and then only remove the ones that are not in the incoming set of tiles.
While we're at it, it would make sense not to request tiles for which we already have data - just the new ones. But you want to be clear about whether the "gridUpdated" trigger provides all the tiles or just the ones that have changed. I could imagine cases where you'd want either one. So maybe it sends a list of all tiles, a list of added tiles and a list of removed tiles and let the receiving module decide what to do with them. Or, it might make more sense to fire two triggers - one that sends the full state, and one that sends just the additional tiles. That way, the decision of which data you want to use can be set up in the configuration and the modules can remain naive about it.
When do you see buildings disappear & re-appear?
I've never seen that. Tested now with the public demo instance, dragged the map around and to new areas, seems to work correctly. And we've been running local instals of git master (+ modifications, but not related this) and AFAIK others here have never seen that either.
Sorry, I should clarify that this is with the 0.2.0 branch. See examples/basic-example/index.html
ok thanks for the clarification - we had missed the new branch, seems interesting but i figure we'll stick with 0.1.0 / master in our dev for now as have a demo in two weeks :) .. will study 0.2.0 after that.
Definitely a smart move, since 0.2.0 is a complete rewrite and not at all compatible.
This is a huge optimization point for #57. Loading data is the single biggest thing slowing down the frame rate, since copying all that data from the XHR happens on the main thread in large chunks.
Yeah, the flickering (hide/show) in the demo is weird and should be fixed. This is not just the demo its affecting.
I think there is lots of room to improve caching of the map tiles and buildings. The web queries might be cached by the browser, but they still take ~100msec each to complete. There is no "real times" polling requirements for vizi. I think once the data is queried, it should be cached and reused from there if unloaded from the GPU. Not to do the whole cycle of requesting it a again from a sevice/API.
This might or might not be related to the flickering issue. A nice and simple cache should be implemented for the blueprint inputs/outputs to store stuff into. I would make a thin wrapped around local storage, not keeping the potentially big data in JS land forever.
Caching is a great idea, but this is a different problem. I'm talking about buildings that are unloaded from the GPU even when they are still needed. The code as is seems to assume that when the center point moves, it moves to a completely new location far enough away from the previous location that there is no overlap in loaded tiles. If you only move a little bit, there will be some tiles that are already loaded and should stay loaded.
The problem happens here: https://github.com/vizicities/vizicities/blob/0.2.0/src/Blueprint/BlueprintOutputBuildingTiles.js#L86-L88
Caching will not solve this problem, because loading up tiles happens asynchronously. So you'll clear out your building tiles and then wait for them to be restored from the cache. Even if it's pretty fast, there will be a noticeable flicker. And even if it's really fast, it'll slow down the main CPU thread enough to slow down the frame rate.
By the way, I have a working implementation that tracks which tiles will be needed after the move, only removes the ones that won't be needed and only requests new ones that haven't already been loaded. It works very well to a) eliminate the flicker, b) avoid unnecessary network requests (even if they're cached) and c) keep the frame rate close to 60fps.
I didn't make a pull request because it's kind of a quick-and-dirty implementation, and I thought we should resolve the questions I asked in my original comment about whether the "gridUpdated" trigger should send all the tiles or just the new ones. I recommend making two triggers - one that sends all the tiles and another that sends just the additional ones. If @robhawkes wants to weigh in, I'm happy to write it up and make a pull request.
Yeah, like I said the caching wont fix this and probably completely unrelated. Good that you found the code entry point where is happens.
I was myself thinking of a solution that would do some kind of tracking and on each mesh add it would check if the bounding box/sphere of the new mesh already contains an existing mesh. If there is an existing mesh and it is of lower quality it should be replaced with the new one. Ofc this bounding box/sphere checks are not 100% reliable and probably wont work that great for all data sets.
But I would guess the building queries will overlap? If you get 2x2 to 0,0,0 origo with higher quality and then 4x4 lower quality, the center 2x2 area meshes should not be added to the scene as there already are buildings there? Is there such logic now or is there always just one "tile" visible with a certain building quality and the rest are just hidden. I would think its nicer to have high quality stuff in the center and then lower poly stuff around it as much as there is data. Then as higher quality stuff is available it should be used even if low poly versions might be there (until there is too much stuff). Here caching the data would help.
Maybe we should wait for @robhawkes to merge the current stuff and you could build a proposal solution on top of it.
btw. I think this building blueprint stuff needs more signaling to the outside world. Lets say my application wants to validate each individual building for 1) if it should be added (merged to the bigger block mesh) 2) i want to set a custom material to it (would require no merging to the bigger tile mesh, as it needs unique material) 3) etc. app use cases. Currently there is no control to do these kind (or I've missed it completely) of things and its really needed. The app cant even define a default material for the buildings without doing some digging into the blueprints and knowing about the source code. There should be some nice API for stuff like this. eg. world.getBlueprintByType(VIZI.BlueprintOutputBuildingTiles).setMaterial(mat).
Cant remember now if this mesh merging is done in the main thread of the WebWorker. In the case of WebWorker it might get a bit more complex in terms of communicating the events in a sane manner.
Right, I've finally had enough time to sit down and read through everything. Thanks for the issue and the suggestions!
I'm going to ignore a few of the side-issues here and focus on the fact that building objects shouldn't be cleared when the grid updates. We can tackle the other issues separately at a later date, or within separate issue threads (most probably deserve that to keep things less confusing anyway) – feel free to set up those other issues or I can when I get around to it.
In short, I completely agree that we need a solution that better manages what buildings have already been loaded and ensuring that they aren't removed unnecessarily. Also, a more intelligent system for tracking updated grid tiles would definitely be welcome.
In my mind 0.2.0 is about keeping as much separately of logic between input and output, so we need to make as few assumptions as possible and handle as much as possible within the config. I'd much rather having 2 grid update events (one for all tiles, one for changed tiles) than trying to make a single update do everything, or compromise it by restricting it to one use-case (other modules would very much benefit from knowing all the grid tiles on move).
I'm happy to see a PR that address these concerns. Specifically, I'm on board with the idea of having two gridUpdated events; one that emits all tiles and another that emits just the changed ones. I'm not strict on what those events are called, so long as it makes sense. Perhaps we can start with the hacky approach and go from there, or go straight in for a more refined PR.
