takahirox / three-gltf-extensions

Unofficial Three.js glTF loader/exporter plugins
MIT License
123 stars 20 forks source link

EXT_mesh_gpu_instancing - Support multiple primitives mesh #56

Closed Pourfex closed 2 years ago

Pourfex commented 2 years ago

Hello ! Using a glb and transforming it with gltf-transform (https://github.com/donmccurdy/glTF-Transform) in 2 steps :

We are able to see correct result in babylon.js (in the sandbox) and unity (using GLTFast : https://github.com/atteneder/glTFast). image

unity_glb_instancing

We we're not able to see the correct results in threejs using your code : image

Here is the glb that uses EXT_mesh_gpu_instancing: 01_batiment_instance.glb.zip

And thanks for developing this extension for threejs users :)

takahirox commented 2 years ago

Thanks for the bug report. Let me look into it...

takahirox commented 2 years ago

Perhaps this is the cause.

https://github.com/takahirox/three-gltf-extensions/blob/99af44c234b61378cbfbc11dd34fd9831c8af5a4/loaders/EXT_mesh_gpu_instancing/EXT_mesh_gpu_instancing.js#L46-L49

The loader extension plugin doesn't support multiple primitives mesh yet. If we implement multiple primitive mesh support, probably the model will be able to be rendered correctly.

takahirox commented 2 years ago

I started to work on the multiple primitives mesh support.

@Pourfex I have a question for your 01_batiment_instance.glb model. The values of the TRANSLATION attribute seem to be very huge like (1351130.625, 62.75, -6240384). Do you know if it's intentional? Or may it be a Three.js glTF loader bug?

Pourfex commented 2 years ago

Thanks for looking at it.

Indeed the model was georeferenced (real world coordinates). I uploaded a 0-0-0 centered version of it here :

01_batiment_instance.glb.zip

Now you'll see all elements that are not using GPU Instancing because they are not duplicates at the correct position. However I think that elements using GPU Instancing are not in the correct position (they are centered at 0,0,0 position I think).

If you need any more test files, I can provide a lot more.

Thanks again to develop this for community. Best.

takahirox commented 2 years ago

I merged #58. Does it look good?

image

Pourfex commented 2 years ago

Looks great for this file !

We tested the latest code (merge #58 ) on some files and we couldn't see anything.

I can provide you this one file that is not showing yet : house_berlin_instance.glb.zip

And thanks again to work on this !

takahirox commented 2 years ago

The TRANSLATION attribute of that model seems very huge like [-4962.7080078125, -10806.2001953125, 10240.83203125]. If it's intentional, probably the model can be displayed if you adjust camera?

Pourfex commented 2 years ago

Had to manipulate the file a little to have correct translation, but yeah it's working with your merge.

Tried 2 others files and they seems to work as expected. File tested : 5_house_berlin_instance

Thanks for the merge, closing issue now.