Skip to content

fix(splat): SPLATFileLoader constructor to merge with defaults instead of entirely replacing them#18438

Open
raymondyfei wants to merge 2 commits intoBabylonJS:masterfrom
raymondyfei:yfei/fix_splat_loader_option_override
Open

fix(splat): SPLATFileLoader constructor to merge with defaults instead of entirely replacing them#18438
raymondyfei wants to merge 2 commits intoBabylonJS:masterfrom
raymondyfei:yfei/fix_splat_loader_option_override

Conversation

@raymondyfei
Copy link
Copy Markdown
Contributor

@raymondyfei raymondyfei commented May 8, 2026

Problem

The SPLATFileLoader constructor accepts Partial<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:

constructor(loadingOptions: Partial<Readonly<SPLATLoadingOptions>> = SPLATFileLoader._DefaultLoadingOptions) {
    this._loadingOptions = loadingOptions;
}

The default parameter (= _DefaultLoadingOptions) only applies when no argument is passed. When any partial options are provided, _DefaultLoadingOptions is bypassed entirely, and all unspecified fields are dropped.

For example, createPlugin calls new SPLATFileLoader(options[SPLATFileLoaderMetadata.name]). When a caller passes pluginOptions: { splat: { flipY: true } }, as the Babylon.js Viewer does for .spz files, the loader receives only { flipY: true }, losing spzLibraryUrl and falling back to the built-in JS parser instead of the WASM path.

Fix

Merge the caller's options on top of _DefaultLoadingOptions:

constructor(loadingOptions: Partial<Readonly<SPLATLoadingOptions>> = {}) {
    this._loadingOptions = { ...SPLATFileLoader._DefaultLoadingOptions, ...loadingOptions };
}

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 extension SafeOrbitCameraAdobe was correctly applied to the assets when using the WASM loader.

@Popov72
Copy link
Copy Markdown
Contributor

Popov72 commented May 8, 2026

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@bjsplat
Copy link
Copy Markdown
Collaborator

bjsplat commented May 8, 2026

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
Copy Markdown
Collaborator

bjsplat commented May 8, 2026

Snapshot stored with reference name:
refs/pull/18438/merge

Test environment:
https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/18438/merge/index.html

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
https://sandbox.babylonjs.com/?snapshot=refs/pull/18438/merge
https://gui.babylonjs.com/?snapshot=refs/pull/18438/merge
https://nme.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.

@bjsplat
Copy link
Copy Markdown
Collaborator

bjsplat commented May 8, 2026

@bjsplat
Copy link
Copy Markdown
Collaborator

bjsplat commented May 8, 2026

@bjsplat
Copy link
Copy Markdown
Collaborator

bjsplat commented May 8, 2026

⚡ Performance Test Results

🟢 All performance tests passed — no regressions detected.

@bjsplat
Copy link
Copy Markdown
Collaborator

bjsplat commented May 8, 2026

🟢 Memory Leak Test Results

13 passed, 0 leaked out of 13 scenarios

🟢 All memory leak tests passed — no leaks detected.

Passed Scenarios (13)
Scenario Package
Core Feature Stack @babylonjs/core
Core Rendering Materials Shadows Stack @babylonjs/core
Core Textures Render Targets PostProcess Stack @babylonjs/core
GUI Fullscreen UI Controls @babylonjs/gui
GUI Mesh ADT Controls @babylonjs/gui
Loaders Boombox Import @babylonjs/loaders
Loaders OBJ Direct Load @babylonjs/loaders
Loaders STL Direct Load @babylonjs/loaders
Materials Library Stack @babylonjs/materials
Serializers glTF Export @babylonjs/serializers
Serializers GLB Export @babylonjs/serializers
PostProcesses Digital Rain Stack @babylonjs/post-processes
Procedural Textures Stack @babylonjs/procedural-textures

@bjsplat
Copy link
Copy Markdown
Collaborator

bjsplat commented May 8, 2026

@azure-pipelines
Copy link
Copy Markdown

Commenter does not have sufficient privileges for PR 18438 in repo BabylonJS/Babylon.js

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.

4 participants