Babylon.js

Babylon.js copied to clipboard

Hello.

I have been working on updating our electron app to the latest electron version (20.0.1, previously we used 17.3.1) and we had a breakage in our use of babylon.js when the scene is disposed:

TypeError: Failed to execute 'unobserve' on 'ComputePressureObserver': 1 argument required, but only 0 present.

Note that not all electron applications will experience this problem. ComputePressureObserver is only exposed if you use experimentalFeatures: true.

It appears that electron 19 and up is using a Chromium version with an updated ComputePressureObserver API. Here is the relevant change in the working draft:

• https://github.com/WICG/compute-pressure/issues/21
• https://github.com/WICG/compute-pressure/pull/25

The error is ignored in observe but not in unobserve. My first commit here catches the error in unobserve. I first tried to use catch() like in observe but it didn't work so I had to use try { ... } catch {}. Even though it isn't documented in the spec, observe is returning a promise but unobserve is not.

The second commit updates the use of the observer to work with electron >= 19. This disables the observer for electron < 19 (ComputePressureObserver.supportedSources is not defined), but I don't think many will notice since the only thing this is used for is the CPU utilization metric in the Realtime Performance Viewer.

The spec appears to have more updates than what electron supports. Doesn't look like the samplerate property works.

I made this simple Electron Fiddle that can be used to test with: https://gist.github.com/stefansundin/4cee8e88133efb196f54f2d2a9f05155. You need to run npm run start to run the babylon.js server as well.

Please let me know if you want me to change anything! Thanks!

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.

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

Test environment: https://babylonsnapshots.z22.web.core.windows.net/refs/pull/12858/merge/index.html

To test a playground add it to the URL, for example:

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/12858/merge/index.html#WGZLGJ#4600

Links to test babylon tools with this snapshot:

https://playground.babylonjs.com/?snapshot=refs/pull/12858/merge https://sandbox.babylonjs.com/?snapshot=refs/pull/12858/merge https://gui.babylonjs.com/?snapshot=refs/pull/12858/merge https://nme.babylonjs.com/?snapshot=refs/pull/12858/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/12858/merge#BCU1XR#0

LGTM but I'll tag @deltakosh to give a look too

This is a great one! Thanks for updating our support ;)

Please fix the format issue and we can merge

Thanks for the reviews!

I ran npm run format and npm run lint but I got these errors:

$ npm run format

> @babylonjs/[email protected] format
> npm run format:check && npm run format:fix


> @babylonjs/[email protected] format:check
> prettier --check ./packages/**/src/**/*.{ts,tsx,js,json,scss,css}

Checking formatting...
[error] No files matching the pattern were found: "./packages/**/src/**/*.json".
[error] No files matching the pattern were found: "./packages/**/src/**/*.css".
All matched files use Prettier code style!

$ npm run lint

> @babylonjs/[email protected] lint
> npm run lint:check && npm run lint:fix


> @babylonjs/[email protected] lint:check
> eslint ./packages/**/src/**/*.{ts,tsx,js,json}


Oops! Something went wrong! :(

ESLint: 8.11.0

No files matching the pattern "./packages/**/src/**/*.json" were found.
Please check for typing mistakes in the pattern.

The solution was to run the commands with updated patterns, i.e.:

$ prettier --check ./packages/**/src/**/*.{ts,tsx,js,scss}
$ eslint ./packages/**/src/**/*.{ts,tsx,js}

So I guess for some reason my setup doesn't have the missing files so the whole command failed instead of running it on the files that actually do exist. A potential solution to this might be to add a few dummy files that will always exist so that the patterns are always satisfied.

Just thought I should raise this potential issue that might bite babylon.js newcomers! :)

To fix formating: npm run format:fix :)

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.

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

Test environment: https://babylonsnapshots.z22.web.core.windows.net/refs/pull/12858/merge/index.html

To test a playground add it to the URL, for example:

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/12858/merge/index.html#WGZLGJ#4600

Links to test babylon tools with this snapshot:

https://playground.babylonjs.com/?snapshot=refs/pull/12858/merge https://sandbox.babylonjs.com/?snapshot=refs/pull/12858/merge https://gui.babylonjs.com/?snapshot=refs/pull/12858/merge https://nme.babylonjs.com/?snapshot=refs/pull/12858/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/12858/merge#BCU1XR#0