Repository navigation
Commit 2da144e
Fix inverse light transform in convertLightsToEyeSpace (#1571)
Summary:
Pull Request resolved: #1571
The software rasterizer shades each pixel with `dot(light_dir_eye, normal_eye)`, so lights and surface normals must live in the same eye/camera space. Surface normals are taken to eye space with `camera.eyeFromWorld()` (world->eye) via `_normalMatrix` and `_worldToEye_rotation` in `arvr/libraries/momentum/rasterizer/utility.h`, but `convertLightsToEyeSpace` in `arvr/libraries/pymomentum/renderer/rasterizer_utility.h` transformed lights with `camera.worldFromEye()` — the INVERSE (eye->world) transform. Lights were therefore rotated by `R^T` while normals were rotated by `R`, so the shading dot product was only correct when the camera view rotation was symmetric (identity / equator). With any tilted, from-below, or polar camera, a world-space camera "headlight" (light direction = camera forward) left the camera-facing surface DARK, even though that surface was fully facing both the camera and the light.
The fix is one line: transform lights with `camera.eyeFromWorld()` so lights and normals use the same world->eye rotation. `convertLightsToEyeSpace` is the single point where every Python-facing render entry point converts world-space lights to eye space (7 call sites in `software_rasterizer.cpp`, 1 in `rasterizer_primitives.cpp`), so this corrects the whole rasterizer. The local variable was already named `worldToEyeXF`, which now correctly matches what it holds. Directional lights are transformed by `xf.linear()` and point lights by the full affine `xf` (`transformLight`), both correct once `xf` is the world->eye transform. The `Light.transform()` pybind takes a caller-supplied matrix and is unaffected.
The existing `test_lighting` never rotated the camera (identity extrinsics, where `worldFromEye() == eyeFromWorld()`), which is why the bug went unnoticed.
Reviewed By: cstollmeta
Differential Revision: D111853949
fbshipit-source-id: b9af875fa4e6dde5a0d8bac731ea80875fe30e2f1 parent 1a0ad7f commit 2da144e
2 files changed
Lines changed: 81 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
302 | 302 | | |
303 | 303 | | |
304 | 304 | | |
305 | | - | |
| 305 | + | |
306 | 306 | | |
307 | 307 | | |
308 | 308 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
111 | 111 | | |
112 | 112 | | |
113 | 113 | | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
114 | 194 | | |
115 | 195 | | |
116 | 196 | | |
| |||
0 commit comments