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

IBL shadowing #15106

Open
wants to merge 45 commits into
base: master
Choose a base branch
from
Open

IBL shadowing #15106

wants to merge 45 commits into from

Conversation

MiiBond
Copy link
Contributor

@MiiBond MiiBond commented May 15, 2024

This PR adds an IblShadowsRenderPipeline that adds shadowing for image-based lighting (IBL). It's applied as a post process after rendering your scene. You must assign an HDR environment with setIblTexture to enable the shadowing. Both cubemaps and equirectangular projected IBL's work.

This pipeline builds a voxel grid for all the meshes in the scene that you want to cast shadows. You can exclude meshes (e.g. the skybox, ground plane, etc.) by calling addExcludedMesh on the pipeline. The voxel grid needs to be re-generated every time something in the scene moves and this can be an expensive operation (depending on the voxel resolution) so it's most suitable for static scenes in its current form. Note that the camera can move and the IBL environment is free to rotate or change completely without having to rebuild the voxel grid.

Using this voxel grid, shadows are generated by sampling the IBL and then applied to the scene as a greyscale approximation. In combination with this, we can also calculate screen-space shadows to handle shadowing of small details.

Some important exposed settings:

  • voxel resolution - the dimension of the voxel grid, in voxels. Directly affects how sharp the shadows are. Must be power-of-two.
  • sample directions - how many samples of the voxel grid per pixel. Increasing this results in less noise but adds significantly to the per-frame cost of shadows
  • toggling screen-space shadows - SS shadows add small shadow detail close to the shadow caster but may not be needed in all scenes so this is togglable and has various settings to control how they behave.

Updating the voxel grid:

  • When a mesh is added or removed from a scene, the voxelization of the scene is automatically rebuilt.
  • When a mesh moves in the scene, you must call updateSceneBounds to get the shadows to rebuild.

Things that still need work:

  • The voxel tracing is currently done in an extremely naïve way and tends to miss voxels, especially at higher voxel resolutions. I have an algorithm using octrees mostly implemented that will be significantly better quality but I wanted to get this PR in sooner.
  • The shadows are greyscale and are an approximation of the light intensity blocked by an object. They often darken even unshadowed surfaces more than they should. I have plans to improve this and also add better support for coloured lighting.
  • IBL shadows don't currently work with multiple cameras (mostly due to this issue https://forum.babylonjs.com/t/viewport-issue-when-using-prepassrenderer/51276)
  • There are sometimes graphical artifacts when moving the camera. I believe that these may be due to precision in the motion buffer (the prepass "velocity" texture).
  • WebGPU is not yet fully supported.
  • Various performance and quality tweaks. e.g. support downsizing the buffers for faster speed.
    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.

@MiiBond MiiBond marked this pull request as ready for review June 12, 2024 19:34
@MiiBond MiiBond changed the title Draft IBL shadowing IBL shadowing Jun 16, 2024
Copy link
Member

@RaananW RaananW left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few suggestions to get the build running correctly. One task is one me (to fix the post-build script from processing commented code).

import { FreeCamera } from "../../Cameras/freeCamera";
import { PostProcessRenderPipeline } from "../../PostProcesses/RenderPipeline/postProcessRenderPipeline";
import { PostProcessRenderEffect } from "core/PostProcesses/RenderPipeline/postProcessRenderEffect";
import type { Camera } from "core/Cameras";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
import type { Camera } from "core/Cameras";
import type { Camera } from "core/Cameras/camera";

import type { AbstractEngine } from "../../Engines/abstractEngine";
import type { Scene } from "../../scene";
import { Vector4 } from "../../Maths/math.vector";
// import { Logger } from "../Misc/logger";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, this is on me! Should be ignored, but our post-build script doesn't detect it as a comment. I will fix the script for future builds.

Suggested change
// import { Logger } from "../Misc/logger";
// import { Logger } from "../../Misc/logger";

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can ignore these comments (of the commented imports) if you merge from master

import { ProceduralTexture } from "../../Materials/Textures/Procedurals/proceduralTexture";
import type { IProceduralTextureCreationOptions } from "../../Materials/Textures/Procedurals/proceduralTexture";
// import { EffectRenderer, EffectWrapper } from "../Materials/effectRenderer";
// import { Logger } from "../Misc/logger";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same thing here

Suggested change
// import { Logger } from "../Misc/logger";
// import { Logger } from "../../Misc/logger";

import type { AbstractEngine } from "../../Engines/abstractEngine";
import type { Scene } from "../../scene";
import { Vector4 } from "../../Maths/math.vector";
// import { Logger } from "../Misc/logger";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// import { Logger } from "../Misc/logger";
// import { Logger } from "../../Misc/logger";

import type { AbstractEngine } from "../../Engines/abstractEngine";
import type { Scene } from "../../scene";
import { Matrix, Vector2, Vector4 } from "../../Maths/math.vector";
// import { Logger } from "../Misc/logger";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// import { Logger } from "../Misc/logger";
// import { Logger } from "../../Misc/logger";

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

4 participants