google / google/filament

lit shading model produces black faces on Metal (NaN propagation from half-precision NoV)

Open
#10,280 3 comments 0 reactions 1 assignee Claimed by @bejado View on GitHub
Dominant language
C++
Stars
20.5k
Forks
2.3k
Avg merge
2d 16h
Merged PRs (30d)
74

Description

## Summary

The `lit` shading model on the Metal backend produces pure-black fragments on a large fraction of cube faces. The `unlit` model renders correctly on the same geometry. This affects Filament v1.74.1 and current `main` (aaabf2c3c).

## Reproduction

A 6-face cube rendered with the `lit` material on the Metal backend (macOS). Headless pixel readback confirms:

| Shading model | Black faces | Notes |
|---|---|---|
| `unlit` | 0% | Renders correctly |
| `lit` (before fix) | 38–60% | Pure black (RGB 0,0,0), regardless of emissive, baseColor, or lighting |
| `lit` (after fix) | 0% | All 6 faces render with correct face differentiation |

Controlled variables that do **not** affect the artifact:
- Emissive `{5,5,5}` — black persists (emissive is poisoned by NaN)
- IBL on/off — black persists
- MSAA on/off — black persists
- Ray tracing on/off — black persists
- Face culling — black persists (both sides affected)

## Root cause

`CreateConvertRelaxedToHalfPass` in `libs/filamat/src/GLSLPostProcessor.cpp:1026` (Metal-only SPIR-V optimizer pass) converts the `mediump`-decorated `dot(shading_normal, shading_view)` to half-precision (`float16`). For face orientations where the normal and view vector are at a large angle, the half-precision dot product underflows to NaN.

### NaN propagation chain

1. `shading_NoV = clampNoV(dot(N, V))` → NaN (`max(NaN, 1e-4)` is NaN; the guard does not catch NaN)
2. `prefilteredDFG(roughness, NaN)` → DFG LUT sample with NaN coord → returns 0
3. `1.0 / pixel.dfg.y` → `1.0 / 0` → +inf → `energyCompensation = inf`
4. `evaluateIBL` multiplies by inf → `color = NaN/inf`
5. `color.rgb += emissive(5,5,5)` → `NaN + 5 = NaN`
6. `min(NaN, MEDIUMP_FLT_MAX)` under Metal `fastMathEnabled=YES` (`MetalShaderCompiler.mm:116`) → 0 (fast-math flushes NaN to 0)

Result: the entire fragment outputs 0 (pure black), regardless of emissive, baseColor, or lighting.

The key files:
- `shaders/src/surface_shading_parameters.fs:66` — `shading_NoV = clampNoV(dot(shading_normal, shading_view));`
- `shaders/src/surface_material.fs:12` — `clampNoV` does `max(NoV, MIN_N_DOT_V)` with no NaN check
- `shaders/src/surface_shading_lit.fs:237` — `1.0 + pixel.f0 * (1.0 / pixel.dfg.y - 1.0)`
- `filament/backend/src/metal/MetalShaderCompiler.mm:116` — `options.fastMathEnabled = YES`

## Proposed fix (3 files in shaders/src/)

**1. `surface_shading_parameters.fs` — root cause fix:**
Force the NoV dot product to highp (float32) to prevent half-precision underflow:
```glsl
highp vec3 hpNormal = shading_normal;
highp vec3 hpView = shading_view;
shading_NoV = clampNoV(dot(hpNormal, hpView));
```

**2. `surface_material.fs` — defensive guard:**
`clampNoV` checks for NaN before `max()`:
```glsl
NoV = (NoV != NoV) ? 0.0 : NoV; // x != x is true only for NaN
return max(NoV, MIN_N_DOT_V);
```

**3. `surface_shading_lit.fs` — defensive guard:**
Guard the `1.0 / pixel.dfg.y` division against zero/NaN:
```glsl
float dfgY = max(pixel.dfg.y, MIN_N_DOT_V);
pixel.energyCompensation = 1.0 + pixel.f0 * (1.0 / dfgY - 1.0);
```

### Note on the highp cast

The `highp` cast on the local copies is the intended root-cause fix. The defensive guards in `clampNoV` and `getEnergyCompensationPixelParams` are belt-and-suspenders against NaN leaking from other paths. The combination resolves the artifact in practice; we have not isolated which change alone suffices, and the `highp` qualifier's effect on the `ConvertRelaxedToHalfPass` at the SPIR-V level may warrant maintainer scrutiny.

## Environment

- Filament v1.74.1 and `main` (aaabf2c3c)
- macOS, Metal backend
- `fastMathEnabled = YES` (default)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.