Neopallium / bevy_water

Dynamic ocean material for Bevy.
https://neopallium.github.io/bevy_water/
116 stars 11 forks source link

Simplified calls for wave position / height #5

Closed noebm closed 1 year ago

noebm commented 1 year ago

One can simplify the code by using a custom SystemParam that contains WaterSettings and Time data. This will allow one to access the members directly instead of manually specifying the time and height. Calling Time methods repeatedly shouldn't cause any overhead AFAICT.

Neopallium commented 1 year ago

Very nice. I haven't worked with SystemParam before.

One thing to note about the Rust-side wave height calculation. The bevy Time value seems to be off by one frame compared to the shader time. Not sure why, but I think bevy is passing the time from the render World to the non-render World. I think it the time should be passed into the render world, that way any updates to object Transforms can be based on the same time value as what will be used in the shaders during render.

I spent hours trying to make the Rust port of the shader wave logic match what the shader was generating. Just realised I could have forced the FPS down to very low numbers to debug this easier.

noebm commented 1 year ago

Good to know!

I'm not quite sure I understand you correctly. Is Time not set on the CPU side and passed to the shader as a uniform?

Neopallium commented 1 year ago

Good to know!

I'm not quite sure I understand you correctly. Is Time not set on the CPU side and passed to the shader as a uniform?

It is calculated on the CPU and passed into the shader, but Bevy has two Worlds for the entities, one for the render thread and another (main thread) where our entities/systems run. I found that the way the Time value is passed between them seems to be backwards, which causes the Rust code to have an old value.

Found the code: https://github.com/bevyengine/bevy/blob/3d752105644a18fb1dd1220d6f15e0a7eb49f04a/crates/bevy_render/src/renderer/mod.rs#L89 It is the Render App that updates the time and sends it to the main app.

The main app receives it here: https://github.com/bevyengine/bevy/blob/3d752105644a18fb1dd1220d6f15e0a7eb49f04a/crates/bevy_time/src/lib.rs#L96

I don't understand why it goes in that direction.

Neopallium commented 1 year ago

This explains why they changed it: https://github.com/bevyengine/bevy/commit/a1d3f1b3b42c3a6e05a532362ddfccf0be6b5df3

It sounds like the time is updated at the end of the frame, so the value used in Rust-side systems should match with the value used in the shaders. So maybe that wasn't the problem.

The difference was very small, it could still have been an issue with the Rust logic not matching the shader, or a difference in floats on the CPU vs GPU.