richardbiely / Voxelmetric

An efficient voxel framework for Unity3d
GNU General Public License v3.0
158 stars 27 forks source link

Mesh Colliders #22

Closed XeonG closed 8 years ago

XeonG commented 8 years ago

So I was thinking about the problem I have with Mesh Colliders causing a significant delay in user experience when removing/placing blocks rapidly.. there is a noticeable pause everytime compared to pre Mesh Colliders being built.

Maybe a solution would be to just have a setting to control the delay before mesh collider is generated if the last user action was removing/placing a block down.. bit like the double click delay

So if they click the timer starts, if they keep rapidly clicking within certain threshold (I dunno <500ms) the mesh collider is delayed from updating, upto a certain point like maybe after 1-4 seconds if the user is still rapidly clicking, then regardless it will still force a mesh collider update?

richardbiely commented 8 years ago

I know about it and will address it soon. Thank you for mentioning it, though :)

richardbiely commented 8 years ago

I did some optimizations: 842fca68a2f6775cf4bf9f224f7a752ffe13f3f3

XeonG commented 8 years ago

Aight.. so tested it is an improvement in that the spikes aren't as big.. but not sure its perfect.. it seems after removing something from a chunk, that chunk then forever has spike updates every (whatever this is set too.... if (rebuildMask>=0 && now-lastUpdateTime>=200) (fyi can these hardcoded settings 200ms be referenced from some sort of config, or public variable in one of the prefabs like camera or voxel world? it would make tracking them down for tuning easier)

And where as before once you stopped editing any terrain the profiler would level out at 2-3ms for me, now after editing a chunk, it has ~7ms spike every 200ms... up until I move the camera view away from that last edited chunk..

Also the updates you did with this optimization.. this 200ms delay its not just for the collider update is it? because it seems like its encompassing both the geometry update, and collider update?.. my suggestion was that the geometry update work as before ie instantly (or maybe on a seperate delay that is more like 50ms), but have the collider update using the timer delay that is higher like 200-500ms (maybe with it pausing all unity physics updates during the time it updates that collider?)

The other thing is that colliders are only generated in a cross + patch from the camera position.. so in testing performance changes between commits if I was removing blocks in a patch that sits in one of the vertical corners of this cross patch, where colliders aren't being generated, well its faster but then its not doing any collider update :D (it explains why some of the performance discrepancy I was seeing, thinking the colliders were completely surrounding the camera, but I think the number of colliders around the camera needs expanding a bit to cover at least another 4more chunks around it, at least for the current raycast distance that you can use).. I dont think the size setting for colliders around the camera is exposed anywhere like it is for loaded chunk distances.. could this be added or pointed out in the code so I can adjust it.

My other questions on colliders is just one how I envision making a game that uses unity physics.. currently the colliders are generated in realtime around the camera position and removed once not seen.. I'm thinking I'd probably want to sticky certain collider/geometry chunks, so that some of them always remain loaded and visible.. would this be trivial feature to add or not? Obviously I still see the engine you're working on as wip.. so I haven't really dived in to do anything with the code myself its changes a bit with your commit updates anyway :D

richardbiely commented 8 years ago

fyi can these hardcoded settings 200ms be referenced from some sort of config, or public variable in one of the prefabs like camera or voxel world? it would make tracking them down for tuning easier)

They certainly can be configurable. However, I'm still thinking about to do improve this further. If I don't come with something, I'll simply introduce a config param.

And where as before once you stopped editing any terrain the profiler would level out at 2-3ms for me, now after editing a chunk, it has ~7ms spike every 200ms... up until I move the camera view away from that last edited chunk..

That was because the rebuild mask was not properly invalidated. Fixed in: 804976f3ed7fa648ef042db0c44cf43037a5a183

I dont think the size setting for colliders around the camera is exposed anywhere like it is for loaded chunk distances.. could this be added or pointed out in the code so I can adjust it.

Collider area is not exposed. The reason being, colliders should only be generated around object that really need them. In general, there's no reason for generating colliders for the whole world if you don't use them. Of course, this might be a bit different with some games. Therefore, it's completely up to you to turn colliders on/off on a chunk via NeedCollider. I will expose this in some way later when I prepare a demo showing off colliders in action.

currently the colliders are generated in realtime around the camera position and removed once not seen..

Not true. Colliders are not removed when not seen. The are kept active for as long as chunk is loaded (or for as long as CollliderNeeded is not set to false).

XeonG commented 8 years ago

"The reason being, colliders should only be generated around object that really need them."

Well what I was getting at is that its currently only generating colliders around camera position in this pattern... http://i.imgur.com/Zk7sM5G.png

.. given the pattern, might aswel not bother generating the 4 end colliders, as any object needing the colliders in the diagonal corners around the camera would have no collider...especially if the camera was right next to the corner point of the chunk it was sitting in.

but anyway I see where the code for it is chunk.NeedsCollider = xd_xd+zd_zd<=1;

So i still think an exposed setting to increase/descrease the size of that pattern around the camera would be useful.. though I just set it to chunk.NeedsCollider = true .. and thats fine for me aswel, seeing as I'll be wanting colliders on all visible terrain that the camera sees anyway.

But the performance of updating the chunk collider everytime (when adding/removing blocks on a chunk) is where I still think it should be separated so that collider meshes are updated less often, while the geometry update delay is as fast as possible as that is mainly what the player sees, and making that fast as possible feels better when adding/removing blocks in quick succession.. where the visual feedback of those updates works better instead of it being held up by the collision mesh being generated aswel. .. ie seperated delays.. but you say you still have some other ideas for performance improvements on it so I'll wait.

richardbiely commented 8 years ago

Well what I was getting at is that its currently only generating colliders around camera position in this pattern... http://i.imgur.com/Zk7sM5G.png ...

Sure. However, like I said, it's up to devs to decide how they want to use colliders. That's what NeedsCollider is there for. Later, maybe you'll be able to influence this via some sort of "collider range" property but I think it's better to let this in hands of devs. E.g., you either enable colliders for the whole world automatically or generate them in runtime in a specific range around your player and NPCs.

But the performance of updating the chunk collider everytime (when adding/removing blocks on a chunk) is where I still think it should be separated so that collider meshes are updated less often, while the geometry update delay is as fast as possible...

Agreed. Definitelly.

XeonG commented 8 years ago

"Sure. However, like I said, it's up to devs to decide how they want to use colliders. That's what NeedsCollider is there for. Later, maybe you'll be able to influence this via some sort of "collider range" property but I think it's better to let this in hands of devs."

yeh ..well I've just added the below code for myself, but I think I probably will do something else with how chunk colliders work.. I'm personally leaning more towards having specific chunks will always have a collider while others will never get one..

public int ChunkColliderDist

        if (xd * xd + zd * zd <= ChunkColliderDist) {
            chunk.NeedsCollider = true;
        } else {
            chunk.NeedsCollider = false;
        }

seems to be enough, can control the area of colliders directly around the camera now.

richardbiely commented 8 years ago

Collider and render mesh updates are now considered two separate things: 42e338bae1bc0f80c7b59d9e3c5eae7ab2131621

With this I consider this issue closed.

Given this issue grew quite large, if there is anything else bothering you, create a new ticket (or a merge request ;) ), please :)

Thank you!

XeonG commented 8 years ago

Nice.. fyi just eager to test out this latest commit update out with the sperated collider/mesh updates.. but it seems the scene broke..

NullReferenceException: Object reference not set to an instance of an object Voxelmetric.Examples.VoxelmetricExample.Update () (at Assets/Voxelmetric/Examples/VoxelmetricExample.cs:64)

perhaps some more commits still incoming, not sure.

edit. my bad ...seems it was because I had renamed grass_top.psd > grasstop.psd and had another grass_top.png in the same texture folder.. when adding the new commit it brought back grass_top.psd and it seemed to break everything. strange