jMonkeyEngine / jmonkeyengine

A complete 3-D game development suite written in Java.
http://jmonkeyengine.org
BSD 3-Clause "New" or "Revised" License
3.75k stars 1.12k forks source link

A multithreading issue with GltfLoader #1809

Closed foxcc2021 closed 1 year ago

foxcc2021 commented 2 years ago

I use multithreading to load gltf models, when loaded, all model materials are messed up.

After I debug GltfLoader, I found out that there may be threading issue in the GltfLoader. The variable "defaultMaterialAdapters" cannot be "static". This variable will be shared when all threads load materials from the model.

A simple way to resolve this issue is to remove the "static". Hope this helps. I use jme version: 3.5.2-stable 20220505173134

resolve I use this way to resolve my problem.

err0 err1 The wrong materials before

correct The correct materials after

stephengold commented 2 years ago

Thanks for the bug report and suggested fix.

stephengold commented 2 years ago

By the way ... creating threads to load models is good trick for reducing latency in a game. Another way to reduce latency is to convert the models to JMonkeyEngine's preferred format (j3o) at build time and then load the j3o instead of the glTF. And of course one can combine both tricks.

The expectation is that the glTF loader won't be used at runtime in a production game. Perhaps that's why the original author of GltfLoader didn't worry about thread safety.

Ali-RS commented 1 year ago

Apparently, CustomContentManager which handles Gltf extension loading is also not thread-safe. The defaultExtensionLoaders needs to be made non-static as well.

https://github.com/jMonkeyEngine/jmonkeyengine/blob/ceb356462aeedf7769dced52e1d5572b01deca31/jme3-plugins/src/gltf/java/com/jme3/scene/plugins/gltf/CustomContentManager.java#L54-L60

Ali-RS commented 1 year ago

A fix is submitted: PR #1886

Ali-RS commented 1 year ago

Merged in commit https://github.com/jMonkeyEngine/jmonkeyengine/pull/1886/commits/92f99ef17e614507e473f157e321d4e06d324751