X Tutup
Skip to content

Perf: conditional execution for extract_skins system#23278

Open
aevyrie wants to merge 1 commit intobevyengine:mainfrom
aevyrie:skinned-mesh-run-criteria
Open

Perf: conditional execution for extract_skins system#23278
aevyrie wants to merge 1 commit intobevyengine:mainfrom
aevyrie:skinned-mesh-run-criteria

Conversation

@aevyrie
Copy link
Member

@aevyrie aevyrie commented Mar 9, 2026

Objective

  • In an app with no skinned meshes and many entities, traces were dominated by extract_skins. This was significant - 10ms in a 30ms frame.
image

Solution

  • Don't run the system if there are no skinned meshes. There is some other performance pathology going on here, and you will hit it as soon as you add one skinned mesh, but for those of us with none, this is a simple way to avoid it at all.

Testing

  • Skinned mesh examples still work.

@aevyrie aevyrie added this to the 0.18.2 milestone Mar 9, 2026
@aevyrie aevyrie requested a review from pcwalton March 9, 2026 08:52
@IceSentry IceSentry added A-Rendering Drawing game state to the screen C-Performance A change motivated by improving speed, memory usage or compile times S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it labels Mar 9, 2026
@github-project-automation github-project-automation bot moved this to Needs SME Triage in Rendering Mar 9, 2026
@pcwalton
Copy link
Contributor

pcwalton commented Mar 9, 2026

I think this is wrong because it doesn't handle the case in which you had skins but they went away (see add_or_delete_skins). Give me a second to prepare an alternative patch.

pcwalton added a commit to pcwalton/bevy that referenced this pull request Mar 9, 2026
Currently, we try to use `Changed<GlobalTransform>` as a way to
accelerate extraction of joints. This was a valiant attempt, but it's
thwarted by the fact that we don't have change indexes, and so it incurs
full table scans of all entities with `GlobalTransform` components. This
patch changes `extract_skins` to simply iterate through all the joints
and fetch their transforms.

When we have change indexes, we can restore the original code, or
something like it.

This PR is an alternative to bevyengine#23278 that should be correct when skins
are removed.
@alice-i-cecile
Copy link
Member

I much prefer #23283 because it's a more principled fix; review that please?

@alice-i-cecile alice-i-cecile added S-Nominated-To-Close A triage team member thinks this PR or issue should be closed out. and removed S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it labels Mar 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Rendering Drawing game state to the screen C-Performance A change motivated by improving speed, memory usage or compile times S-Nominated-To-Close A triage team member thinks this PR or issue should be closed out.

Projects

Status: Needs SME Triage

Development

Successfully merging this pull request may close these issues.

5 participants

X Tutup