fix(splat): SPLATFileLoader constructor to merge with defaults instead of entirely replacing them#18438
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
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/18438/merge/index.html#WGZLGJ#4600 Links to test your changes to core in the published versions of the Babylon tools (does not contain changes you made to the tools themselves): https://playground.babylonjs.com/?snapshot=refs/pull/18438/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/18438/merge#BCU1XR#0 If you made changes to the sandbox or playground in this PR, additional comments will be generated soon containing links to the dev versions of those tools. |
|
WebGL2 visualization test reporter: |
⚡ Performance Test Results🟢 All performance tests passed — no regressions detected. |
🟢 Memory Leak Test Results13 passed, 0 leaked out of 13 scenarios 🟢 All memory leak tests passed — no leaks detected. Passed Scenarios (13)
|
|
Visualization tests for WebGPU |
|
Commenter does not have sufficient privileges for PR 18438 in repo BabylonJS/Babylon.js |
Problem
The
SPLATFileLoaderconstructor acceptsPartial<Readonly<SPLATLoadingOptions>>, which implies callers only need to provide the fields they want to override — the rest should come from_DefaultLoadingOptions. However, the implementation assigns the argument directly:The default parameter (
= _DefaultLoadingOptions) only applies when no argument is passed. When any partial options are provided,_DefaultLoadingOptionsis bypassed entirely, and all unspecified fields are dropped.For example,
createPlugincallsnew SPLATFileLoader(options[SPLATFileLoaderMetadata.name]). When a caller passespluginOptions: { splat: { flipY: true } }, as the Babylon.js Viewer does for.spzfiles, the loader receives only{ flipY: true }, losingspzLibraryUrland falling back to the built-in JS parser instead of the WASM path.Fix
Merge the caller's options on top of
_DefaultLoadingOptions:This makes the implementation consistent with the
Partial<>type contract: callers override only what they need, and all other defaults remain intact.In addition, we updated the reference image for the test
Gaussian Splatting IBL Shadows. There was a very minor camera difference after the extensionSafeOrbitCameraAdobewas correctly applied to the assets when using the WASM loader.