Retain skins from frame to frame. #17818
Conversation
Currently, Bevy rebuilds the buffer containing all the transforms for joints every frame, during the extraction phase. This is inefficient in cases in which many skins are present in the scene and their joints don't move, such as the Caldera test scene. To address this problem, this commit switches skin extraction to use a set of retained GPU buffers with allocations managed by the offset allocator. I use fine-grained change detection in order to determine which skins need updating. Note that the granularity is on the level of an entire skin, not individual joints. Using the change detection at that level would yield poor performance in common cases in which an entire skin is animated at once. Also, this patch yields additional performance from the fact that changing joint transforms no longer requires the skinned mesh to be re-extracted. Note that this optimization can be a double-edged sword. In `many_foxes`, fine-grained change detection regressed the performance of `extract_skins` by 3.4x. This is because every joint is updated every frame in that example, so change detection is pointless and is pure overhead. Because the `many_foxes` workload is actually representative of animated scenes, this patch includes a heuristic that disables fine-grained change detection if the number of transformed entities in the frame exceeds a certain fraction of the total number of joints. Currently, this threshold is set to 25%. Note that this is a crude heuristic, because it doesn't distinguish between the number of transformed *joints* and the number of transformed *entities*; however, it should be good enough to yield the optimum code path most of the time. Finally, this patch fixes a bug whereby skinned meshes are actually being incorrectly retained if the buffer offsets of the joints of those skinned meshes changes from frame to frame. To fix this without retaining skins, we would have to re-extract every skinned mesh every frame. Doing this was a significant regression on Caldera. With this PR, by contrast, mesh joints stay at the same buffer offset, so we don't have to update the `MeshInputUniform` containing the buffer offset every frame. This also makes PR bevyengine#17717 easier to implement, because that PR uses the buffer offset from the previous frame, and the logic for calculating that is simplified if the previous frame's buffer offset is guaranteed to be identical to that of the current frame. On Caldera, this patch reduces the time spent in `extract_skins` from 1.79 ms to near zero. On `many_foxes`, this patch regresses the performance of `extract_skins` by approximately 10%-25%, depending on the number of foxes. This has only a small impact on frame rate.
pcwalton
added
A-Animation
|
I hit a panic if I force Repro: |
| /// | ||
| /// This is 256 GiB worth of joint matrices, which we will never hit under any | ||
| /// reasonable circumstances. | ||
| const MAX_TOTAL_JOINTS: u32 = 1024 * 1024 * 1024; |
There was a problem hiding this comment.
Took me a little while to work how this value was derived. Maybe worth doing it explicitly or as a comment?
| const MAX_TOTAL_JOINTS: u32 = 1024 * 1024 * 1024; | |
| const MAX_TOTAL_JOINTS: u32 = (2u64.pow(u32::BITS) / (JOINTS_PER_ALLOCATION_UNIT as u64)) as u32; |
There was a problem hiding this comment.
You're reading more into it than I intended :) It's actually a completely arbitrary number, set to be "higher than you'll ever need but not so high as to be absurd", so as to catch bugs before OOM happens.
There was a problem hiding this comment.
Hah, that makes sense. I also misunderstood what the allocator was doing.
I think it will panic a while before this due to byte_offset: u32? But doesn't seem like a pressing issue.
|
@greeble-dev Fixed that issue. I needed to leave space at the end of the buffer. |
| /// fine-grained detection to determine which skins need extraction. If the | ||
| /// number of changed entities is over this threshold, we skip change detection | ||
| /// and simply re-extract the transforms of all joints. | ||
| const JOINT_EXTRACTION_THRESHOLD_FACTOR: f64 = 0.25; |
There was a problem hiding this comment.
This heuristic intuitively makes sense to me, but I wonder if it should be exposed as configuration. Maybe something for the future with more testing to understand the balance here. I also hope that our change detection (particularly w.r.t. assets) will become cheaper in the future.
There was a problem hiding this comment.
Do you mean plugin configuration? I'm a bit wary of having knobs that we have to maintain unless we know we need them.
There was a problem hiding this comment.
No, I agree we're fine now, let's just revisit this heuristic if, e.g., asset change detection improves in the future.
There was a problem hiding this comment.
Clicking the approve button with some minor suggestions, but take with salt as I don't have much renderer experience.
In case it's relevant, I'm planning to look at #16929 once things have calmed down, and that will involve adding a test for skinning corner cases (missing joints, adding/removing meshes). So that might shake out any subtle issues with this PR.
EDIT: Forgot to mention that I tested many_foxes and custom_skinned_mesh, also with uniform buffers forced on. Win10/Nvidia/Vulkan.
Currently, Bevy rebuilds the buffer containing all the transforms for joints every frame, during the extraction phase. This is inefficient in cases in which many skins are present in the scene and their joints don't move, such as the Caldera test scene.
To address this problem, this commit switches skin extraction to use a set of retained GPU buffers with allocations managed by the offset allocator. I use fine-grained change detection in order to determine which skins need updating. Note that the granularity is on the level of an entire skin, not individual joints. Using the change detection at that level would yield poor performance in common cases in which an entire skin is animated at once. Also, this patch yields additional performance from the fact that changing joint transforms no longer requires the skinned mesh to be re-extracted.
Note that this optimization can be a double-edged sword. In
many_foxes, fine-grained change detection regressed the performance ofextract_skinsby 3.4x. This is because every joint is updated every frame in that example, so change detection is pointless and is pure overhead. Because themany_foxesworkload is actually representative of animated scenes, this patch includes a heuristic that disables fine-grained change detection if the number of transformed entities in the frame exceeds a certain fraction of the total number of joints. Currently, this threshold is set to 25%. Note that this is a crude heuristic, because it doesn't distinguish between the number of transformed joints and the number of transformed entities; however, it should be good enough to yield the optimum code path most of the time.Finally, this patch fixes a bug whereby skinned meshes are actually being incorrectly retained if the buffer offsets of the joints of those skinned meshes changes from frame to frame. To fix this without retaining skins, we would have to re-extract every skinned mesh every frame. Doing this was a significant regression on Caldera. With this PR, by contrast, mesh joints stay at the same buffer offset, so we don't have to update the
MeshInputUniformcontaining the buffer offset every frame. This also makes PR #17717 easier to implement, because that PR uses the buffer offset from the previous frame, and the logic for calculating that is simplified if the previous frame's buffer offset is guaranteed to be identical to that of the current frame.On Caldera, this patch reduces the time spent in
extract_skinsfrom 1.79 ms to near zero. Onmany_foxes, this patch regresses the performance ofextract_skinsby approximately 10%-25%, depending on the number of foxes. This has only a small impact on frame rate.