owl-project / NVISII

Apache License 2.0
319 stars 27 forks source link

`set_intrinsics` function documentation #136

Closed ArthurAllshire closed 2 years ago

ArthurAllshire commented 2 years ago

In the docs here, the set_intrinsics function states

fx (float) – X-axis focal length in meters.
fy (float) – Y-axis focal length in meters.

However I believe that this is a typo and they should be in pixels? When I put in F/L in meters I get very strange images, whereas when I put in pixels things work perfectly.

natevm commented 2 years ago

Sorry for the late response. Been drowning in other projects.

We essentially stole from the intrinsics camera code here:

https://pyrender.readthedocs.io/en/latest/generated/pyrender.IntrinsicsCamera.html

Their documentation states that fx and fy should be in meters. Do you also disagree with the pyrender documentation? I'm not sure who to trust to be honest.

natevm commented 2 years ago

I haven't heard back on this issue in a while, so I'm closing for the time being. Feel free to reopen if you find the time. :)

murrdpirate commented 1 year ago

@natevm fx and fy should generally be in pixels (and always match the units used for cx and cy). It looks like pyrender updated their documentation to pixels.

ancientghost commented 1 year ago

@natevm fx and fy should generally be in pixels (and always match the units used for cx and cy). It looks like pyrender updated their documentation to pixels.

Sure, it's updated, fx and fy should be in pixels, refer to this here.