Skip to content

GPU instances nodes removal fixes#2577

Open
2frac wants to merge 2 commits into
KhronosGroup:mainfrom
2frac:gpu_instancing_deletion_fix
Open

GPU instances nodes removal fixes#2577
2frac wants to merge 2 commits into
KhronosGroup:mainfrom
2frac:gpu_instancing_deletion_fix

Conversation

@2frac

@2frac 2frac commented Jun 25, 2025

Copy link
Copy Markdown
Contributor

It reworks code dealing with nodes instancing - in particular part which removes nodes added to instancing.

What it does

  • Gathers removed nodes from all scenes and applies deletion at once.
    (in old code self.nodes_idx_to_remove is cleaned up in loop(not before it), which will cause to only delete nodes from last processed scene)

  • Uses sets to check if node is deleted, instead of lists.
    This probably has the most effect on speed for big-sized instance collections(should help with glTF Exporter Hangs with Large-Scale Instancing #2199 )

  • Instead of calculating shifts for node indices it uses single dict mapping.
    I haven't checked exact performance effect, but to iterate all deleted objects for every remaining object can result in bad speed for specific scenes. There is still some O(n^2)-ish speed behavior, but i suspect it rather related to node creation and better be investigated separately.
    Also results in simpler code.

  • Applies new node index to animation's target.
    Previously it was just checking if node is deleted, without changing index to new.
    Should solve Exporting the GPU instance together with other simple TRS animations will cause the animation to play incorrectly. #2423

Examples to reproduce

All made in blender 4.4.3. For every example, both GPU instances and GN instancing should be enabled.

amount of instances can be controlled by density
for 40k instances it takes 25 seconds(before fix) vs 10 second(after)for me.
for 100k - 155 vs 48.

When exported, result is looking strange for "Scene", which likely to be caused by bad indices.
Also some "GN Instance" objects can be seen in node list in JSON, even if GN instancing is enabled as option and it should remove it.

When exported, animation target has node ids in JSON, which is too big to be real.

Note

There is more optimizations i would like to try(beside said creation), but better be done as separate pull request[s].

Instancing generally looks like it be better done using direct transform gathering from GN/instancing/particles rather than generating and unifying nodes - but that will require entirely different approach with its own problems.

@CLAassistant

CLAassistant commented Jun 25, 2025

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@2frac

2frac commented Jun 26, 2025

Copy link
Copy Markdown
Contributor Author

CI fail looks like a consequence of that blender 5.0 change

Specifically, that part fails:

if bpy.context.preferences.addons['io_scene_gltf2'].preferences \
and "allow_embedded_format" in bpy.context.preferences.addons['io_scene_gltf2'].preferences \
and bpy.context.preferences.addons['io_scene_gltf2'].preferences['allow_embedded_format']:

I might create separate pull request for that a bit later.

@julienduroure

Copy link
Copy Markdown
Collaborator

Can you please rebase with main branch?
This should solve the tests now

@2frac

2frac commented Jul 3, 2025

Copy link
Copy Markdown
Contributor Author

(i meant to rebase instead of merge main, but result should be the same)

@bandinopla

Copy link
Copy Markdown

This PR solves the issue. I just tested it with the official version, I was exporting an animation that also had GPU instances ON, and the animation was getting destroyed. Replaced the exporter.py with the one in this PR and it was solved!

@julienduroure

Copy link
Copy Markdown
Collaborator

Thanks for the PR, I will have a look soon!

@julienduroure julienduroure added this to the Blender 5.1 milestone Dec 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants