Facepunch / garrysmod-requests

Feature requests for Garry's Mod
84 stars 24 forks source link

Option to supply a matrix object to Entity:GetBoneMatrix() #2300

Open 0RadicalLarry0 opened 7 months ago

0RadicalLarry0 commented 7 months ago

Details

Currently calling GetBoneMatrix() in a rendering hook or the BuildBonePositions callback will cause the game to stutter if used a lot. For example, looping through the bones of a lot of ragdolls.

If we could supply our own matrix object to be modified and reused so it doesn't constantly create matrix objects for the GC to remove, it would likely boost performance a lot.

robotboy655 commented 7 months ago

Can you provide a code example (a working one) that reproduces the performance issue?

0RadicalLarry0 commented 7 months ago

Run this on the client and you should see the performance degrade as you add more ragdolls. I spawned serverside ragdolls from the spawnlist for my tests.

if !CLIENT then return end

local is_ragdoll = {
    ["prop_ragdoll"] = true
}

hook.Add("OnEntityCreated", "ragdoll_setupbones", function(ent)
    if !is_ragdoll[ent:GetClass()] then return end

    local bones = {} -- cache bones
    for i = 0, ent:GetBoneCount() - 1 do
        bones[i] = true
    end

    hook.Add("PostDrawOpaqueRenderables", "ragdoll_setupbones"..tostring(ent), function()
        for i in pairs(bones) do
            ent:GetBoneMatrix(i) -- comment this out and the stuttering will go away
        end
    end)
end)

hook.Add("EntityRemoved", "ragdoll_setupbones_remove", function(ent)
    if !is_ragdoll[ent:GetClass()] then return end

    hook.Remove("PostDrawOpaqueRenderables", "ragdoll_setupbones"..tostring(ent))
end)
0RadicalLarry0 commented 7 months ago

With Entity:GetBoneMatrix()

Screenshot_1061

Without Entity:GetBoneMatrix()

Screenshot_1062

0RadicalLarry0 commented 7 months ago

My idea for how I'd supply a matrix object would look something like this. And for cases where the bone matrix cannot be retrieved the return value could instead be a bool signifying whether or not the matrix was able to be modified.

hook.Add("OnEntityCreated", "ragdoll_setupbones", function(ent)
    if !is_ragdoll[ent:GetClass()] then return end

    local bones = {} -- cache bones
    for i = 0, ent:GetBoneCount() - 1 do
        bones[i] = Matrix() -- or ent:GetBoneMatrix(i)
    end

    hook.Add("PostDrawOpaqueRenderables", "ragdoll_setupbones"..tostring(ent), function()
        for i, mat in pairs(bones) do
            local valid = ent:GetBoneMatrix(i, mat) -- reuse existing matrix object, return value is signifying whether or not the matrix was able to be modified
        end
    end)
end)
robotboy655 commented 4 months ago

I don't think changing return type based on arguments is a good idea.

The stutters are coming from GC as far as I can tell, and switching Matrices to the same scheme as Vectors and Angles already use seems to have eliminated the stuttering in my tests. So that's the proper solution in my head, rather than the proposed band-aid for a single function.

There IS still a performance improvement with your proposition, but I am not sure adding a matrix argument to every function that returns one is something I want to do.

0RadicalLarry0 commented 4 months ago

I think a separate Entity:GetBoneMatrix function could be added instead with a matrix argument and my proposed return value. I don't think it'd be necessary to add a matrix argument everywhere else, I reckon the performance impact of reusing matrices is crucial for Entity:GetBoneMatrix in particular due to the immense usage of the function every frame.

However, if you think your solution would suffice I'd be happy either way!

0RadicalLarry0 commented 4 months ago

I feel I should mention, in my use case I was working on a dismemberment system (as one does for ragdolls) and I have to create a new ragdoll for every dismembered limb. With around 10 ragdolls completely dismembered, around 15 ragdolls per ragdoll, this comes out to 150 ragdolls in total. I imagine most players would want to dismember more than 10 ragdolls at once and 10 ragdolls is already a lot for this. Perhaps the performance gap between your solution and my proposal would widen significantly given over 150 ragdolls.

Although I don't know how many ragdolls you have tested with at once.

In the final iteration of my system, I would destroy limbs given some condition like crushing, so there probably wouldn't be as many ragdolls per ragdoll in practice.

0RadicalLarry0 commented 4 months ago

Testing on dev shows reduced spikes in frametimes compared to x86-64 but still low framerates compared to no bone matrix modifying/rendering at all.

138 ragdolls for these tests I'm using my aforementioned dismemberment code

Dev - With rendering: ~25 fps reduced spikes in frametimes when compared to x86-64 Dev - Without rendering: ~65 fps

x86-64 - With rendering: ~27 fps large spikes in frametimes x86-64 - Without rendering: ~73 fps

I don't know how much reusing matrices would improve performance here, there's probably other rendering or code overhead causing more performance to be lost.