nerfstudio-project / nerfacc

A General NeRF Acceleration Toolbox in PyTorch.
https://www.nerfacc.com/
Other
1.39k stars 115 forks source link

Some marching details related to `advance_to_next_voxel` function #72

Closed ventusff closed 2 years ago

ventusff commented 2 years ago

Hi, First of all, thanks for the great repo! During code reading, I noticed that there's a slight difference between raymarching here and ngp's implementation when applying dt:

When cone_angle is set to nonzero, in ngp's implementation, ngp uses proportional stepping to march to the next voxel, while in here it uses minimum step size linear stepping.

  1. Is this difference made out of consideration of marching's safety ? Are there concerns of steps too large that even marches across the next voxel ?
  2. If you are already using linear stepping rather than proportional one, the number of while loop cycles is actually deterministic. _t can be directly calculated from _t = t + dt_min * (int ((t_target-t) / dt_min) + 1);. No do-while loop is needed anymore. The efficiency difference between do-while loop and calculating number of cycles in advance is negligible. During my test, the time difference is within measurement error when N_rays=4096,120k, 1920k.
liruilong940607 commented 2 years ago

Hi in ngp's original impl, the dt calculated by calc_dt is bounded by a maximum dt, which is determinate by the voxel size. So it can safely use calc_dt to march to the next voxel. (it won't skip any voxel).

But in our case, the occupancy grid might be a contracted grid. As our calc_dt function is shared in both cases (non-contracted & contracted grid), we can't use the voxel size to bound the dt (it's not correct for the contracted case). In other words, our calc_dt might return a dt that larger than a voxel size. So here we simplly use the dt_min to safely march to the next voxel.

Actually this is not ideal. A better solution is to use the voxel size to bound the dt in the non-contracted case, but no bound (or estimate a bound based on the contraction method) in the contracted case. But as you have noticed this part of logic doen't affect the overall speed so we didn't make it too complicated.

ventusff commented 2 years ago

Considering contraction makes perfect sense, thanks for your fast reply! Closing now