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

Draft IBL shadowing #15106

Draft
wants to merge 46 commits into
base: master
Choose a base branch
from
Draft

Draft IBL shadowing #15106

wants to merge 46 commits into from

Conversation

MiiBond
Copy link
Contributor

@MiiBond MiiBond commented May 15, 2024

This is a very early peek at what the shadowing looks like. The quality will still improve a lot.
IBL Shadows

@MiiBond MiiBond marked this pull request as draft May 15, 2024 15:26
@sebavan sebavan requested review from Popov72 and deltakosh May 15, 2024 15:27
@bjsplat
Copy link
Collaborator

bjsplat commented May 15, 2024

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented May 15, 2024

@bjsplat
Copy link
Collaborator

bjsplat commented May 15, 2024

Visualization tests for WebGPU (Experimental)
Important - these might fail sporadically. This is an optional test.

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/15106/merge/testResults/webgpuplaywright/index.html

@bjsplat
Copy link
Collaborator

bjsplat commented May 15, 2024

@bjsplat
Copy link
Collaborator

bjsplat commented May 16, 2024

Visualization tests for WebGPU (Experimental)
Important - these might fail sporadically. This is an optional test.

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/15106/merge/testResults/webgpuplaywright/index.html

@MiiBond
Copy link
Contributor Author

MiiBond commented May 23, 2024

Would it be possible to get some general feedback about whether this is the right approach for implementing something like this in Babylon? I've modeled this after the DepthPeelingRenderer, which is a scene component, but maybe this should be a pipeline?
The effect has many passes and parts so I'm curious whether I'm structuring them in a reasonable way inside Babylon.

@Popov72
Copy link
Contributor

Popov72 commented May 23, 2024

I'll take a look at it early next week.

@Popov72
Copy link
Contributor

Popov72 commented May 27, 2024

Here are some quick comments, but not a full review, as I probably won't be able to follow up because I'll be away for a few months in a couple of days, and it's only in draft mode:

  • "clip" in some variable / define names (like viewDepthClipSpace or PREPASS_CLIPSPACE_DEPTH) would probably be better replaced by "ndc"
  • I would have created a sub-directory under Rendering/ and move all IBL shadows related files to this directory
  • Try to avoid GC in methods called each frame (as the update() method)
  • All shader code should probably be in Shaders/ and not directly in .ts files (cf. iblShadowsImportanceSamplingRenderer.ts, which seems to be the only file concerned, probably because you're still working on the implementation)
  • Perhaps we should find an abbreviation other than "sss" for "screen space shadows", because it conflicts with "sub-surface scattering".
  • I don't think the current code support dynamic update of the scene (adding/removing meshes) after the ibl shadow renderer has been enabled?

As for your question about implementation, I think a rendering pipeline would indeed be better suited, as it would better isolate the code from the rest of the engine.
We're going to try and move away from the existing architecture with scene components, so it's best not to rely on them...

I also wonder about performance...
With the default values of 256 for the voxel resolution, and assuming 8 draw buffers are supported, it means (256/8)*3=96 rendering to regenerate the voxelized scene.
Even with a resolution of 64, it's still 24 passes to generate the voxelized scene.
Have you been able to run tests on different scenes of varying complexity and assess the impact on performance?

@MiiBond
Copy link
Contributor Author

MiiBond commented May 28, 2024

Excellent, feedback, @Popov72! Thank you.

Regarding performance, yes, it's a lot of draw calls to generate the voxel grid and it isn't really suited to dynamic scenes for that reason (at least in it's current form). There are optimizations yet to be made but, to be honest, most of the work I have ahead of me is for improving the quality rather than the performance.

Enjoy your time off!

@MiiBond
Copy link
Contributor Author

MiiBond commented Jun 7, 2024

I turned this into a pipeline but I'm not yet using the addEffect functionality and adding the pipeline to the scene.postProcessRenderPipelineManager.
I think most of the per-camera stuff that I have can be converted to this system (many are ProceduralTextures right now) but I'm unsure of my AccumulationPass. Right now, it relies on a couple of buffers from the previous frame, including the previous result of the accumulation pass. Is there a way to store off RT's to then be used for the next frame for each camera? It doesn't look like the PostProcessRenderPipeline was designed with this in mind.

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

3 participants