The Skinning Fraud, Closed

In Built, Not Adopted there was a section titled “Skinning, on the GPU, finally” that wasn’t, quite. Here is the paragraph that has been bothering me since I wrote it:

The GPU compute skinning shader was correct end to end. The frame-ring ping-pong worked. The previous-frame SSBO retention worked. SkinnedMeshNode::process was already dispatching it. And then the live consumer (RenderSceneNode::process) was reading the CPU-fallback out: Mesh MeshHandle, because the schema flip from “MeshHandle” to “deformed-position SSBO” hadn’t landed yet.

So: you could open Lux, load a character, watch the bones deform a mesh smoothly at 60 fps, and the codebase contained a complete, tested GPU skinning compute shader and a CPU for i in 0..vertex_count loop. The loop was the one driving every frame. The GPU shader ran too. It produced a buffer. It deformed that buffer beautifully, on schedule, every frame, and nobody read it. A shader doing perfect work into the void.

That post landed a parity test and said the schema flip and the deletion would come “in a later post.” This is the later post. It is also, in the meshlet contract, Passes 4 and 5, and they land here together, exactly as that post said they had to.

DrawItem becomes an enum

The blocker was a type. lux_core::scene::DrawItem was a struct with a mesh: MeshHandle field. A static cube and a skinned character were the same shape, and that shape had nowhere to put a deformed-vertex buffer. So the renderer read mesh, and mesh was the rest pose, and the only way to make a rest pose move was to overwrite it CPU-side every frame, which is exactly what skin_cpu did.

DrawItem is an enum now:

pub enum DrawItem {
    Static(StaticDrawItem),
    Skinned(SkinnedDrawItem),
}

StaticDrawItem is the old struct, renamed. SkinnedDrawItem carries what a skinned draw actually needs: the rest-pose mesh, the deformed_pos buffer the compute shader writes, the prev_deformed_pos buffer from last frame (so TAA can compute motion vectors for a moving character), the material, and both the current and previous model matrices.

Every consumer of scene.draws becomes an exhaustive match. wildcard_enum_match_arm, the clippy lint the debt sweep turned on workspace-wide, is denied for DrawItem, so when a third variant lands one day (a procedural draw, a volumetric draw, whatever it turns out to be) it is a compile error at every match site instead of a silent fallthrough I discover in a screenshot. Ten-plus constructor sites across the render crate and the scene plugin migrated to DrawItem::Static. Exactly one place in the whole tree emits DrawItem::Skinned: SkinnedMeshNode::process.

The mesh shader grew a SKINNED permutation that pulls position, normal, and tangent from the deformed SSBOs at vertex fetch instead of from the rest-pose vertex buffer. The pipeline cache provisions a SKINNED-flavoured draw layout. The compute shader’s output is finally an input to something.

skin_cpu, deleted

With the consumer reading the GPU buffer, the CPU path had no readers. So it goes. All of it, in the same commit, because a half-done version of this breaks every skinned mesh in the tree.

skinning.rs, home of the skin_cpu function, is git rm’d. The skin_cpu_fallback Cargo feature is deleted, and so is the default = ["skin_cpu_fallback"] line that turned it on. That feature was a direct tenet violation: a feature flag whose entire job was to preserve legacy behaviour, with a comment in the Cargo.toml that said, out loud, “keep skin_cpu reachable for one release cycle.” There is no release cycle. There is no one. It’s gone.

This is the shape of every honest deletion in this sprint. The legacy thing is not removed because it’s broken. It’s removed because the real thing finally has a consumer, and the moment it does, the legacy thing is just a slower second answer to a question that now has exactly one.

Verifying a thing whose reference oracle you just deleted

Here’s the wrinkle, and it’s a small preview of a much bigger version of the same problem a couple of posts from now.

The old skinning test was skin_gpu_matches_skin_cpu.rs. It rendered the GPU skin and the CPU skin and asserted they matched. That test is structurally impossible now: one of the two things it compares no longer exists. You cannot diff against a function you deleted. This is obvious in hindsight. It was not obvious at cargo test.

So the test gets rebuilt around ground truth instead of around the old implementation. skin_gpu_correctness.rs computes the expected linear-blend-skin result analytically: one matrix multiply per vertex, written inline in the test, three poses across five vertices, and asserts the GPU output matches it within FMA-precision tolerance. It depends on the skinning math, not on a second copy of the skinning code.

That’s the better test anyway. “Matches the old implementation” only ever proved the two implementations agreed. If they were both wrong the same way, the test was green and the picture was broken, and everyone went home happy. “Matches the analytic result” proves the GPU is doing linear-blend skinning correctly, full stop.

Two passes, closed

The meshlet contract listed Pass 4 as “DrawItem becomes an enum” and Pass 5 as “skin_cpu deletion,” and was emphatic that they could not land apart: the moment one ships without the other, every skinned-mesh patch silently breaks. They shipped together. The contract numbered them 4 and 5, and they happen to be the first of its deferred passes to land, ahead of 2 and 3, because the schema flip didn’t depend on the bindless shader and the bindless shader is the longer road.

That road is the next post. The placeholder shader dies and the bindless arm goes live.

I have no idea what I’m doing or if any of this is right, but it’s fun. Follow along.