Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Much cleaner edge rendering when enabling logarithmic depth buffer #1788

Merged
merged 1 commit into from
Jan 23, 2025

Conversation

tmarti
Copy link
Contributor

@tmarti tmarti commented Jan 23, 2025

Descrption

Use the same gradient technique has here #1162 (comment) for much better visual quality in edge rendering when the logarithmic depth buffer is enabled.

Screenshots: Before / After

The screenshots below were taken with a far plane distance equal to 1M km!

Before (i)

image

After (i)

image

Before (ii)

image

After (ii)

image

Explanation

Sub-pixel z-fighting (explained here #1162 (comment)) makes edge drawing worse when logarithmic depth buffer is enabled, possibly because the dynamic range of the Z values get compressed (via the log function) together with much bigger far plane distances being used with logarithmic z-buffer.

As the rendering of edged surfaces happens in two stages in xeokit-sdk...

  • a) first, regular surfaces are rendered, and this initializes the Z-buffer
  • b) on top of it, edges are rendered, which then z-fight with the pre-initialized z-buffer in "a)"

... the reduced dynamic range affects more the case where log-depth-buffer is enabled.

This PR makes use of the gradient trick so, during the regular surface rendering, their z-values are "pushed" far away from the camera to the same distance where their immediate furthest neighbour pixel is.

This makes the z-buffer pre-initialization much more friendly with stage "b)", and fixes the sub-pixel z-fighting problem.

@xeolabs xeolabs merged commit 593019a into xeokit:master Jan 23, 2025
2 checks passed
@Amoki
Copy link
Contributor

Amoki commented Jan 23, 2025

It may fix #651, I'll check!

Thanks :D

@tmarti
Copy link
Contributor Author

tmarti commented Jan 23, 2025

Just checked, it should indeed fix it 🎁

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.

3 participants