Viewer fixes for IBL Shadows (Draft) #16659
Conversation
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
|
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/16659/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/16659/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/16659/merge#BCU1XR#0 |
|
WebGL2 visualization test reporter: |
|
Visualization tests for WebGPU |
1 similar comment
|
Visualization tests for WebGPU |
|
WebGL2 visualization test reporter: |
|
Visualization tests for WebGPU |
|
WebGL2 visualization test reporter: |
There was a problem hiding this comment.
I pushed a change to this PR to fix WebGPU snapshot rendering with IBL shadows. The only other thing I think we probably want before exposing IBL shadows in the Viewer is to disallow them when the model is animated (since IBL shadows do not update in real time as the model animates). @alexandremottet do you want to make those changes in this PR?
| @@ -1693,7 +1684,7 @@ export class Viewer implements IDisposable { | |||
|
|
|||
| updateMaterial(); | |||
|
|
|||
| pipeline.onShadowTextureReadyObservable.addOnce(updateMaterial); | |||
| pipeline.onShadowTextureReadyObservable.add(updateMaterial); | |||
There was a problem hiding this comment.
I guess regardless we don't really need to track and remove the observer, since we would do that at exactly the same time as when we dispose the entire pipeline anyway (which clears observers), correct?
There was a problem hiding this comment.
No, I don't think this change is needed.
|
WebGL2 visualization test reporter: |
|
Visualization tests for WebGPU |
Yes. In fact, I already opened a PR for the animation fix on WebGPU. I didn't really intend this PR to also include that... I'm not sure how I managed to do that. |
MiiBond
changed the title
When using the geometry buffer with animated meshes, we're hitting this compile error about
uniforms.mPreviousBonesnot being defined. Turns out there was a slight difference in the WebGPU shader that was preventing compilation when BONETEXTURE was also defined. Now, the behaviour matches WebGL.