BabylonJS / Babylon.js

Babylon.js is a powerful, beautiful, simple, and open game and rendering engine packed into a friendly JavaScript framework.
http://www.babylonjs.com
Apache License 2.0
22.78k stars 3.39k forks source link

Fix erratic glTF progress values #15136

Closed bghgary closed 1 month ago

bghgary commented 1 month ago

See https://forum.babylonjs.com/t/sceneloader-load-progress-rollback/50805.

The main problem is that we were removing the requests from the array which is used for calculating the total, making the total drop as requests complete. But even if we stop removing the requests, it won't always work properly if there are requests with lengthComputable = false. To solve this, we can force lengthComputable to true and set total to loaded when the request is complete to make sure the progress is counted.

bjsplat commented 1 month ago

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 commented 1 month ago

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

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

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

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

Links to test babylon tools with this snapshot:

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

Jeggery commented 1 month ago

we can force lengthComputable to true and set total to loaded when the request is complete to make sure the progress is counted.

Will it still take effect if gzipped is used?

bjsplat commented 1 month ago

WebGL2 visualization test reporter:

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/15136/merge/testResults/webgl2playwright/index.html

bjsplat commented 1 month ago

Visualization tests for WebGPU (Experimental)

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/15136/merge/testResults/webgpuplaywright/index.html

bghgary commented 1 month ago

we can force lengthComputable to true and set total to loaded when the request is complete to make sure the progress is counted.

Will it still take effect if gzipped is used?

I'm not sure what you mean by "take effect". If a file is gzipped on the server, the lengthComputable will likely be false. The total property will not be available. With the new code, all I'm doing is pretending that lengthComputable is true and that the length is the amount of data loaded when the request is complete to aid in computing the total of all the requests. This means that the total progress won't be available while loading a request where lengthComputable is false, but it will be available as soon as that request completes.

Jeggery commented 1 month ago

we can force lengthComputable to true and set total to loaded when the request is complete to make sure the progress is counted.

Will it still take effect if gzipped is used?

I'm not sure what you mean by "take effect". If a file is gzipped on the server, the lengthComputable will likely be false. The total property will not be available. With the new code, all I'm doing is pretending that lengthComputable is true and that the length is the amount of data loaded when the request is complete to aid in computing the total of all the requests. This means that the total progress won't be available while loading a request where lengthComputable is false, but it will be available as soon as that request completes.

Thank you. I get it