Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ocean3D as a tool script (and OceanSurface3D components for rendering options) #14

Draft
wants to merge 25 commits into
base: devel
Choose a base branch
from

Conversation

Tattomoosa
Copy link
Contributor

@Tattomoosa Tattomoosa commented Feb 20, 2024

Adding as a draft PR because I had to do a lot more than I expected to get things mostly playing nicely, and I didn't keep my changes very minimal, and there's still a few edge cases to iron out and code to clean up, but I think it's far along enough that it's worth getting your eyes on.

The biggest change is the addition of a component OceanSurface3D and subclasses for different rendering methods which I mostly added because I wanted to test while diagnosing issues with QuadTree as a tool script and prepare for optional Terrain3D integration. I think there are also plenty of situations when rendering with the simplest PlaneMeshOceanSurface3D makes the most sense, and simplifying the setup of a surface for an end user is useful.

Another change is that Ocean3D copies the default material instead of using it directly. There were a lot of issues with 2 Ocean3D's affecting the same resource so I'm not sure it should be possible. I have been thinking maybe it should use the resource directly as a separate "initial settings" property but then use a different material to write its changes to, or the various initial settings on the material should be properties of Ocean3D itself that it handles applying to the material properly? Still thinking about this.

I think the last big change is that the QuadTree no longer depends on a scene resource.

Also a callout that I understand now the MultiMesh implementation I made might not actually make sense here - MultiMesh doesn't really have performance benefits until we get to waaaaaaay more meshes, and it doesn't support any mesh resolution differences. I'm keeping it around because I think it may be useful as a piece of a simpler LOD solution to test against the quad tree solution later but performance analysis will need to be done.

There are changes committed to the Example scene but it's not in the best state since I was doing experimentation there. I think it's all working properly, just the settings are not great.

I've added a UvBorder.gdshader for debugging QuadTree behavior (because I made a bit of a mess of it for a minute there, haha...). It can be added as a second pass on the ocean material or applied directly to a quadtree and will render just the edges of the plane to make it really obvious what quadtree is doing.

TODO before PR

  • When Ocean3D editor simulation is on, it should pause when a game instance is running
  • QuadTree3D makes all of nodes in-editor but only ever shows the lowest LOD, meaning it can be really slow to open scenes
    • visibility detection doesn't seem to work with the editor camera
  • Ocean3D doesn't handle changing the FFT settings correctly while simulating, maybe others, need to test more
  • General cleanup, documentation, etc.

Some other thoughts

  • QuadTree performance:

    • Working solution in editor that doesn't rely on visibility notification by reading from the camera view directly?
      • Can make this method available during runtime as well and see what's more performant
    • Performance improvements:
      • Instantiate necessary higher detail quads lazily (fairly easy change)
      • See if creating one node then duplicating using instantiation for each other node per LOD level is cheaper. Not finding much documentation on that, it might only be useful for keeping node changes in-sync
      • Re-use high detail quads by moving them around as needed, keep actual mesh count as low as possible (fundamental change)
  • The ocean needs to grab a camera for shader calculations anyway. This should be the "source of truth" on what camera needs to be looked at for OceanSurfaces as well as OceanEnvironment (or an OceanEnvironment replacement)

  • Infinite ocean surface rendering should be possible by moving the ocean surface rendering mesh(es) with the camera

    • Quick test (not committed) shows that this kind of works but is a bit jittery
    • I've done this in a (much worse) ocean shader I built a while back, going to see if I did something there I haven't tried here
    • This was a major consideration behind creating the OceanSurface3D class, as it could handle infinity and maybe other properties that will be common across all surface rendering solutions

Sorry this is all a bit disorganized!

@Tattomoosa
Copy link
Contributor Author

Tattomoosa commented Feb 21, 2024

Found a bug with the QuadTree implementation that I've fixed here, but let me know if I should PR it separately real quick since it also happens in devel

Screenshot 2024-02-20 at 9 43 24 PM

You can see the low detail mesh and high detail mesh are rendering on top of each other.

To reproduce:

  1. Move the first Example scene camera very far away
  2. Switch to camera 2

The fix is to change this bit at line 178

    for subquad in _subquads:
        if not subquad.lod_select(cam_pos):
            subquad.mesh_instance.visible = true

To

    for subquad in _subquads:
        subquad.mesh_instance.visible = false
        if not subquad.lod_select(cam_pos):
            subquad.mesh_instance.visible = true

@Tattomoosa
Copy link
Contributor Author

There's another issue with changing the camera to the second one when the first is very far away, but I'm not sure it's really possible to fix since I think it has to do with how the simulation simplifies at a distance. Basically all the buoyant objects wind up in the wrong place under or over the surface.

@tessarakkt
Copy link
Owner

The biggest change is the addition of a component OceanSurface3D and subclasses for different rendering methods which I mostly added because I wanted to test while diagnosing issues with QuadTree and prepare for optional Terrain3D integration. I think there are also plenty of situations when rendering with the simplest PlaneMeshOceanSurface3D makes the most sense, and simplifying the setup of a surface for an end user is useful.

I like this change, but would rename OceanSurface3DBase to just OceanSurface3D to align with the naming in the engine which does not include "Base" in the names of base classes usually.

Another change is that Ocean3D copies the default material instead of using it directly. There were a lot of issues with 2 Ocean3D's affecting the same resource so I'm not sure it should be possible.

Duplicating the material for each Ocean3D should work.

I have been thinking maybe it should use the resource directly as a separate "initial settings" property but then use a different material to write its changes to, or the various initial settings on the material should be properties of Ocean3D itself that it handles applying to the material properly? Still thinking about this.

I would say have Ocean3D expose and manage the settings on the material.

See if creating one node then duplicating using instantiation for each other node per LOD level is cheaper. Not finding much documentation on that, it might only be useful for keeping node changes in-sync

This may be a good candidate for using the RenderingServer directly to draw the surface as described here.

Quick test (not committed) shows that this kind of works but is a bit jittery

Also had the same jitter when I tried this. I suspect there are so many nodes that moving them triggers it.

Found a bug with the QuadTree implementation that I've fixed here, but let me know if I should PR it separately real quick since it also happens in devel

I've been chasing that for a bit, that's great that you corrected that. I don't think a separate PR is needed.

There's another issue with changing the camera to the second one when the first is very far away, but I'm not sure it's really possible to fix since I think it has to do with how the simulation simplifies at a distance. Basically all the buoyant objects wind up in the wrong place under or over the surface.

The simulation itself does not simulate at a distance, only the visual rendering does. The buoyancy remains the same "resolution" regardless of distance from the camera. I do notice that the domain warp setting causes the buoyancy bodies to not properly follow the wave height for the location they are in. Does what you are seeing here occur with the domain warp strength set to 0? If that fixes it, the issue is how domain warp is handled in Ocean3D.global_to_pixel().

@Tattomoosa
Copy link
Contributor Author

Tattomoosa commented Feb 27, 2024

I like this change, but would rename OceanSurface3DBase to just OceanSurface3D to align with the naming in the engine which does not include "Base" in the names of base classes usually.

Done

I would say have Ocean3D expose and manage the settings on the material.

Cool. Thinking that will be part of this PR.

Also had the same jitter when I tried this. I suspect there are so many nodes that moving them triggers it.

This isn't the cause, even a single plane still displays the jitter. It isn't noticeable at all when the subdivision amount is high enough, so I'm thinking it has to do with the vertices being able to get into the displaced position consistently.

Not sure that's actually solvable but went ahead and implemented basic infinity in the base OceanSurface3D class anyway for further experimentation and because at high enough detail levels its very usable. If the jitter of doing it the simple way isn't solvable, then it can still be done by teleporting meshes around instead of moving them slowly. Probably not going to look at that as part of this PR though.

This may be a good candidate for using the RenderingServer directly

Agreed. Probably not going to look at that as part of this PR either, but it does interest me and I'd be happy to look into it a little later.

The simulation itself does not simulate at a distance, only the visual rendering does. The buoyancy remains the same "resolution" regardless of distance from the camera. I do notice that the domain warp setting causes the buoyancy bodies to not properly follow the wave height for the location they are in. Does what you are seeing here occur with the domain warp strength set to 0? If that fixes it, the issue is how domain warp is handled in Ocean3D.global_to_pixel().

Thanks for clearing that up, but it does appear to still happen with domain warp at 0. I don't have any ideas for the cause here, but I do have some ideas for setting up testing scenes that might help to sort it out.

TODO before PR

When Ocean3D editor simulation is on, it should pause when a game instance is running

I think I've finally sorted out how to do this, looking at implementing it now. EDIT: Found a much easier way, actually. Done

QuadTree3D makes all of nodes in-editor but only ever shows the lowest LOD, meaning it can be really slow to open scenes

I wish I had a better idea how to handle this, but for now I'm just going to stop the quad tree from making all LOD levels in-editor. I think once this PR is ready to merge I'm going to look at the Terrain3D integration and then I might take a deep dive into the quadtree and try to tackle a couple of the things we've talked about. EDIT: Done

Ocean3D doesn't handle changing the FFT settings correctly while simulating, maybe others, need to test more

I think it's just the FFT settings that are a problem, but all the Ocean3D code that's dealing with the RenderingServer, compute shaders, etc, is pretty opaque to me. I know the issue is the change in buffer size if the newer FFT size is bigger than the old one. Not sure how to solve that cleanly just yet though.

I'm also getting this error I've had some issue tracking down:

E 0:00:00:0580   set_texture_rd_rid: Condition "!RD::get_singleton()->texture_is_valid(p_texture_rd_rid)" is true.
  <C++ Source>   scene/resources/texture_rd.cpp:78 @ set_texture_rd_rid()

It seems to happen either not at all, 3 times, or 6 times, when running a scene with Ocean3D which is also simulating Ocean3D within the editor. It looks to me like the error occurs when a texture has an RID and a new RID is assigned to it (?)
https://github.com/godotengine/godot/blob/b09f793f564a6c95dc76acc654b390e68441bd01/scene/resources/texture_rd.cpp#L78

It's definitely behavior introduced by Ocean3D as a tool script, and I assume it's one of the rendering thread textures or compute shaders but the error doesn't stack trace back to GDScript

So for both that error and FFT settings, would appreciate some guidance if you have ideas, or I think the PR is set up so that you can push to it directly. I'll work on get the changes I know how to tackle done before I get back to poking at those

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants