arkavo-org / arkavo-org/VRMMetalKit
MToon debug mode 15 uses Half-Lambert while production path uses raw Lambert
- Dominant language
- Swift
- Stars
- 6
- Forks
- 2
- Avg merge
- 18h 51m
- Merged PRs (30d)
- 26
Description
## Summary
Surfaced by the PR #207 review (vrm-conformance issue #205 follow-up).
`MToonShader.metal` debug mode 15 (`lightingFactor` visualization, also debug mode 14 `shadowStep`) still applies a Half-Lambert remap to `NdotL` before the linearstep:
\`\`\`metal
float rawNdotL = dot(normal, -uniforms.lightDirection.xyz);
float NdotL = rawNdotL * 0.5 + 0.5; // half-Lambert remap
...
float shadowStep = linearstep(-1.0 + toony, 1.0 - toony, shading);
\`\`\`
The inline comment at line 356 literally says `// Half-Lambert to match main lighting`, but #183 removed Half-Lambert from the main production path (line 605 now uses raw `dot(normal, -lightDir)`). The debug visualization is therefore showing a different formula than the live shader runs — a faithful debug overlay should mirror production.
## Why it matters
`Tests/VRMMetalKitTests/MToonLightingFactorTests.swift` drives `debugUVs=15` to verify lighting behaviour. Because the debug path uses Half-Lambert, the test currently validates a formula production doesn't run. It can pass while the real lighting is broken (or vice versa).
## Repro
\`\`\`bash
grep -n "Half-Lambert" Sources/VRMMetalKit/Shaders/MToonShader.metal
# Debug modes 14 and 15 still have the remap; main path (around line 605) does not.
\`\`\`
## Suggested fix
Replace the `rawNdotL * 0.5 + 0.5` remap in debug modes 14 and 15 with the raw `dot(normal, -lightDirection.xyz)` so the debug visualization mirrors the production `shadowStep` / `lightingFactor`. Update the misleading comment at line 356. Re-baseline any `MToonLightingFactorTests` assertions that were tuned against the Half-Lambert range as needed.
## Out-of-scope context
This was discovered while reviewing #207 (1/π BRDF Lambert normalization for #205) — the debug paths are bypassed by the production fix, so #207 didn't need to touch them. Filing here so the debug ↔ production parity stays an explicit task.
Contributor guide
Research direction
Start in Sources/VRMMetalKit/Shaders/MToonShader.metal, comparing debug modes 14 and 15 with the production lighting path around line 605. Run Tests/VRMMetalKitTests/MToonLightingFactorTests.swift and review assertions affected by the current remap. Done means the debug visualization uses the same lighting formula as production and the tests pass with accurate expectations.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- swift
- Domain
- computer-graphics
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100