Skip to content

Support reversed Z rendering and offscreen 32 bit float depth buffers for improved camera depth accuracy#948

Closed
aftersomemath wants to merge 2 commits into
google-deepmind:mainfrom
aftersomemath:depth-precision
Closed

Support reversed Z rendering and offscreen 32 bit float depth buffers for improved camera depth accuracy#948
aftersomemath wants to merge 2 commits into
google-deepmind:mainfrom
aftersomemath:depth-precision

Conversation

@aftersomemath

Copy link
Copy Markdown
Contributor

This PR adds flags and logic to allow using reversed Z rendering (for details see here and here) and allows using 32 bit floating point for offscreen depth buffers (MuJoCo is currently using 24 bit normalized integers for the depth buffer which are converted to 32 bit floats during mjr_readPixels).

The difference in accuracy is significant. The plots below show the mean, std deviation, and maximum error of depth at all pixels as a randomly tilted plane moves away from the camera along the Z axis. Random tilting is essential for seeing the errors introduced by low precision Z buffers. The discussion and investigation leading to this is in #688.

Figure_1

For room scale scenes, reversed Z rendering and a 32 bit float depth buffer brings the error down from 100 micrometers to 1 micrometer. 100's of micrometers is probably fine if MuJoCo is being used for simulating a sensor, but not very good if it is being used as a source of ground truth (not good enough for my work at least).

Details

Reversed Z rendering requires the ARB_clip_control extension and the floating point Z buffer requires ARB_depth_buffer_float. Support should be very widespread.

Supporting these flags with mjr_makeContext is difficult without a breaking change to the signature or the behavior. I have done it in a way that breaks neither, but I would rather make a breaking change to the signature. Based on feedback, I can change it and update the documentation.

I have updated renderer.py to support the reversed Z flags and 32 bit floating point buffer. I have also updated the way window coordinate Z values were being converted to metric depth. The new way is closer to what OpenGL does, and incurs less numerical errors. The improvement is very significant, especially at far distances, and I can produce plots if wanted.

I'm not sure how to bring the 32 bit floating point depth buffer to the window buffer. Depending on the feedback, it can be figured out.

The script to generate the plot above is here. (Note: znear=0.1, zfar=25, model extent=1)

Combined, both options greatly improve camera depth accuracy compared to
OpenGL default Z mapping and int24 depth buffer.

Requires GL_ARB_clip_control and ARB_depth_buffer_float extensions
…flags

By more carefully inverting the operations performed when OpenGL
transforms metric distance to window coordinates, more accurate
depth values can be estimated from the depth buffer

bool PlatformUIAdapter::RefreshMjrContext(const mjModel* m, int fontscale) {
if (m != last_model_ || fontscale != last_fontscale_) {
// con_.depthMapping = mjDB_ONETOZERO;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting this parameter prior to calling mjr_makeContext allows the user to control depthMapping and depthPrecision. I would rather add arguments to mjr_makeContext like fontscale but this would be a breaking API change.

@kevinzakka kevinzakka requested review from saran-t and yuvaltassa June 22, 2023 21:29
@yuvaltassa

Copy link
Copy Markdown
Collaborator

Have not reviewed thoroughly, but I have a general thought. Is there any sense in which mjDB_ONETOZERO and mjDB_FLOAT32 are not a strict improvement? Is there any non-negligible detrimental effect? E.g. slower rendering, low depth resolution for some situations etc?

If this is a strict improvement or very nearly a strict improvement – more precisely if we can't think of a reason for people to choose the old-style depth-buffer – then I think we should consider making these changes in place.

What I propose, if this is indeed the case, is that you make a seperate PR making the changes in place (should be easy). It would then be straightforward for me to pull that in and run it against our internal tests.

WDYT?

@aftersomemath

Copy link
Copy Markdown
Contributor Author

It seems there should be no non-neglible detrimental effects. A test is needed to make sure, which I can write soon.

mjDB_ONETOZERO + mjDB_FLOAT32 definitely improves accuracy for far distances, but I need to carefully check what happens close to znear.

mjDB_FLOAT32 has some impact on memory consumption and memory bandwidth usage because the depth buffer is now 64 bits per pixel instead of 32 bits (32 bits depth, 8 bits stencil, 24 bits wasted versus 24 bits depth, 8 bits stencil). Additionally, the depth tests are now being done in floating point instead of integer formats. It seems like neither of these things should make a noticeable difference on a modern GPU, but a test is needed to be sure.

So if both of these things turn out to be non-issues, I'll make a PR with the changes in place.

@aftersomemath

Copy link
Copy Markdown
Contributor Author

Closed in favor of #978

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants