wave-harmonic / crest

A class-leading water system implemented in Unity
MIT License
3.49k stars 478 forks source link

Infinite World Setting back to Vector.zero #150

Closed holdingjason closed 5 years ago

holdingjason commented 5 years ago

So how would we handle this. If I want to reset the world back to zero at some point which is a typical way of dealing with the floating point issue in Unity and infinite worlds how would we get crest to handle that seamlessly. So we would be sending the boat, player and all the islands etc back to zero as we travel as we get certain distances away from the origin. My guess is this is not going to look very good depending on the waves etc of crest at the time. Imagine going up a wave, snapping back to zero where maybe a different wave etc is there. Guessing that would look really bad.

So any thoughts? Someway to grab the current values of the various LODs and keep those values when we reset back?

huwb commented 5 years ago

Funny you should ask, this was asked before ad i put my initial thoughts here #131

holdingjason commented 5 years ago

Ok read what you mentioned

worldPos_wt _renderDataPrevFrame _renderData collision readback requests

Are you talking about applying an offset to these guys to basically keep the same values before we moved? If so how do you handle floating point issues btw or at all. My guess we are going to start getting funky values for height etc as we move away from center, in which case the offset would not help that part since the actual value would still be going up and up.

huwb commented 5 years ago

Yes, that is exactly the question I also pose in the other issue. How should the gerstners work?

huwb commented 5 years ago

Oh I suppose each gerstner wave component could have some kind of phase offset which accounts for displacement from origin or something like that. So when teleporting the phases are adjusted to compensate for the jump..?

holdingjason commented 5 years ago

I guess I will need to understand how the algorithm uses worldPos to calc the gerstner values. Hmm wonder what would happen if we just kept pumping in the old world pos and then gradually lerped it to the new position.

holdingjason commented 5 years ago

I would think that would work (ie phase offset) as long as we got rid of it overtime so the values never get to out of wack. So maybe they lerp out slowly back to zero so we don't really notice the adjustment?

holdingjason commented 5 years ago

You just don't want the phase offset values become so huge that you get the floating point error again on your side (I am guessing again that large floating point values would cause trouble with your gerstner calcs but not sure)

huwb commented 5 years ago

Yeah I don't know exactly where the large values would end up causing issues, but since the wave functions repeat periodically every 2pi radians I think the values can be kept in [0,2pi) , even for large world positions.

If I get a chance to test this I'll post back, no promises in the short term though.

holdingjason commented 5 years ago

This is long term issue so low priority. Short term we are just sticking to a fixed world size less then 10,000 units to avoid the issue. I am spinning up a separate project to find out the various issues involved and work on coming up with a long term solution. For us that involves moving the world items and ship back to origin after you reach a certain distance. So was just playing around with that when I realized the ocean would also be a problem so figured I would at least start the discussion.

holdingjason commented 5 years ago

BTW that does make sense what you are saying and because of how your generating a 2pi radian from that world value I cannot think of how that would create any issues.

huwb commented 5 years ago

Are there other things on your list of issues? I was thinking that getting continuity with particle systems would be problematic (maybe only if they are simulated in world space?).

How about rigidbodies, can you just poke new positions into them and they'll keep the same velocity etc?

holdingjason commented 5 years ago

Had not thought about particles, those would be a problem if simulated in world space but I think most of ours our under local so would move with everything else as long as they were created under a movable parent.

Rigidbody should maintain those values but if not should be easy to update them. That guys ref to the floating Origin script shows updates to there sleep values but not sure what that is about. This would have an effect on our boat so will have to test that.

holdingjason commented 5 years ago

BTW updated to latest 2018.3 and everything is working fine. Not sure if anyone had done that yet.

huwb commented 5 years ago

I think i know whats required now. i sketched something out in the commit/branch above.

It doesnt do anything smart with the phase - all that should be required is to add a Mathf.Repeat() to keep it in range. However there seems to be a small pop coming from somewhere which should probably be fixed first. Will post back if i have any luck.

huwb commented 5 years ago

I got a pop free version working, pushed to the branch (enabled in main.unity).

I unfortunately discovered that any world space calculations fall over, and there were two that were generating pops:

I suppose the latter could be addressed by having a UV offset which can be used to compensate shifts in origin (similar to how the phase offset compensate for the shift for gerstners). Not sure i want to venture down this rabbit hole too much further, especially if it involves adding more calculations and code..

holdingjason commented 5 years ago

So how would you sum up where your are at this point. Not sure I totally understood your last sentence.

holdingjason commented 5 years ago

BTW this may sound stupid but would it be possible to physically move the LODs to a new spot along with the player/camera? I am guessing not since we are passing world position into the shaders directly.

holdingjason commented 5 years ago

Ok going to grab that branch and play with it a bit.

holdingjason commented 5 years ago

Ok so played with it and getting rid of the terrible FindObject etc I don't see any spikes in CPU etc. Like you said there is a small pop that I can see during the reset but honestly it is not bad as long as your not doing it very often. We could live with that small little bump once in awhile as it stands right now especially since it does not appear to effect the boat in anyway via physics/height.

huwb commented 5 years ago

Yeah thats with the normals set to a power of 2 size (so the scale param). I think its 16. If you use other scales it will pop. THe normals give the appearance of waves, so it looks like the ocean pops.

I think this is a reasonable constraint though. If foam and other textures were also scaled at a power of 2 it would solve much of the pop.

Still deciding what i want to do with this (for core crest). Guess i might write a section in the docs and decide if i want to merge the code in or not.

holdingjason commented 5 years ago

Ok. Yeah its to bad that unity does not have a better solution for larger worlds and when I say larger anything further out then a few 1000 units, which I understand now but was a bit shocked about. So this will probably effect most production level games I would think at some point. It does not take long doing any sort of ocean travel before you hit those limits. I would vote for some sort of solution/hack but whatever you think is best. What is the downside to this code mod btw? Power for 2 seems reasonable but perhaps I do not understand how that is effecting things so maybe a bit of insight on that. Thanks for all the help btw yet again.

holdingjason commented 5 years ago

Just tested this again adjusting all the scales to powers of two. yep your right I don't even notice the jump anymore at all. I lose a bit of tweaking but like you said I think that is a reasonable constraint. This works very well for what we need that is for sure.

huwb commented 5 years ago

Thats cool. My only hesitation is just a general "less code is better" philosophy, especially with things like this which I would forget to test often, or be too lazy to test often.

Anyway ill take a look at the changes with fresh eyes and think about it.

How did you get rid of the FindObjectsOfType calls btw? There are a few of them for different types, did you get rid of all of them? Do you have user-defined lists, or an interface to register scripts that need to get floating origin messages?

holdingjason commented 5 years ago

Sounds fair. Agree with you on that as a general approach.

As far as the FindObjectsOfType I was doing a user defined list for the test, however typically your idea of an interface and register pattern would be the correct way to go. I do that in many other places for this type of thing. Especially in this case where it is not good enough to do a findobjects at the start of the app or something and never have to touch it again. Here objects could be going and coming out of existence all the time that might need to be updated as well.

So yes I would create a static or Instance object that the scripts that will process the update register themselves with and then do whatever cleanup they need. For something like this a generic system will only work for the most basic of scenarios ie FindObject. We would also need to update things like path finding (paths currently in motion, grid offsets etc) which would require additional specialized code.

huwb commented 5 years ago

Makes sense.

Building that stuff to register components is beyond me for the reasons mentioned above, however i thought the following compromise solution might work - i've added component arrays to the FloatingOrigin script. If you provide them (either by hand or at runtime) it will use them, otherwise it will fall back to the Find() calls. That means the behaviour is flexible and works by default, and when it comes to fine grained optimisation, one can either list the stuff by hand, or write a system around it which does all the registration etc and sets these arrays. Could you take a look at #151 and let me know what you think?

holdingjason commented 5 years ago

Ok yes I would not expect you to have to create that system. All you need to concern yourself with is an example, which you have and the ability to reset origin of the ocean which you have. The specifics as to moving things around outside of the ocean is not your issue in my opinion so what you have is fine for that.

holdingjason commented 5 years ago

Question on this code. Do the children of the ocean every change once the app starts up? If not I would cache that though its not going to make a smidgen of difference given how infrequent this is called.

Otherwise looks fine.

// Notify ocean of origin shift if (OceanRenderer.Instance) { var fos = OceanRenderer.Instance.GetComponentsInChildren(); foreach (var fo in fos) { fo.SetOrigin(newOrigin); } }

huwb commented 5 years ago

Hm.. It can be that the ocean is destroyed (unloaded from a subscene) and then recreated and stuff like that, so i think im inclined to take the safe route here.

huwb commented 5 years ago

Merged! Let me know if you use it and encounter any issues.

holdingjason commented 5 years ago

BTW not sure if you knew this. I did not until a little while ago that some of these math functions are seriously slow. I have not done a deep profile on your stuff in a bit but I did see some of these functions. This only matters when your doing a lot of these over and over again so not sure if any of that applies but figured I would pass it along.

Minimize expensive math functions Transcendental functions (Mathf.Sin, Mathf.Pow, etc), Division, and Square Root all take about 100x the time of a multiplication. (In the grand scheme of things, no time at all, but if you are calling them thousands of times per frame it can add up).

The most common case of this is vector normalization. If you are normalizing the same vector over and over, consider normalizing it once instead and caching the result for use later.

If you are both using the length of a vector and normalizing it, it would be faster to obtain the normalized vector by multiplying the vector by the reciprocal of the length rather than by using the .normalized property.

If you are comparing distances, you don’t have to compare the actual distances. You can compare the squares of the distances instead by using the .sqrMagnitude property and save a square root or two.

Another one, if you are dividing over and over by a constant c, you can multiply by the reciprocal instead. Calculate the reciprocal first by doing 1.0/c.

huwb commented 5 years ago

Yeah these kinds of things are on my radar. If you spot cases where I can avoid this please call it out. As far as I'm aware I'm either calling these things infrequently, or where they need to be called. But I might be missing some optimisation opportunities.

One expensive case is gerstner wave computation on the CPU - this should be moved to the job system at some point, like the lwrp wave racer demo. However this code is not used by default so it's not top of my list right now.

holdingjason commented 5 years ago

Yep. I need to get into the job system myself. Powerful but a bit more obtuse from a coding standpoint.