KhronosGroup / glTF-Blender-IO

Blender glTF 2.0 importer and exporter
https://docs.blender.org/manual/en/latest/addons/import_export/scene_gltf2.html
Apache License 2.0
1.49k stars 317 forks source link

Importer perf when sharing accessor across primitives #695

Closed julienduroure closed 5 years ago

julienduroure commented 5 years ago

@scurest

Think there is still some perf improvement that can be done. I have a mesh, with more than 5000 primitives, sharing an accessor for position, one for normal, and one for UV.Currently, it takes around 19s to load. Same mesh, but each primitive has its own accessor (one for position, one for normal, one for UV, for each primitive). It takes 1.5s to load (exactly same geometry result) Unfortunately, I can't share this glb file, but I will try to create a file that I can share.

julienduroure commented 5 years ago

Ok, this part of code take 15s (on a total of 17s): https://github.com/KhronosGroup/glTF-Blender-IO/blob/517c808ba42709b191e8f355fcd3e474f23ce85b/addons/io_scene_gltf2/blender/imp/gltf2_blender_primitive.py#L73

(line 71 to 78)

julienduroure commented 5 years ago

Seems we can replace :

for pidx in range(0, len(positions)): ..... if pidx in used_pidxs:

by

for pidx in used_pidxs:

perf is now good. Needs to be tested, but seems to be OK

scurest commented 5 years ago

Wow, 5000 :D

Yeah, that looks good. IIRC, I did that so the verts would go in the same order they were in the primitive. That's useful for debugging but it should be okay to do them in any order.

You could also see if this is fast enough

used_pidxs = list(used_pidxs)
used_pidxs.sort()
for pidx in used_pidxs:

That would put them in the same order.

julienduroure commented 5 years ago

What I propose is to perform sorting only in debug mode, adding:

if bpy.app.debug: ....used_pidxs = list(used_pidxs) ....used_pidxs.sort() for pidx in used_pidxs: