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

WebGLRenderer: Add logarithmicDepthBufferFragment parameter #22082

Closed
wants to merge 2 commits into from

Conversation

sam6321
Copy link
Contributor

@sam6321 sam6321 commented Jul 3, 2021

Related issue: #22017

Description

Added logarithmicDepthBufferFragment parameter to allow the user to choose between vertex shader emulated logarithmic depth and fragment shader logarithmic depth.

This is an alternative option to #22041 which removes fragment shader logarithmic depth entirely.

…hoose between vertex shader logz and fragment shader logz
@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 3, 2021

A third option would be to leave the logarithmic depth buffer implementation as it is and introduce a reverse depth buffer implementation like suggested in #22017 (comment).

@sam6321
Copy link
Contributor Author

sam6321 commented Jul 3, 2021

Yeah that would be good. Would it be a problem to support all 3 depth buffer methods?

@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 3, 2021

Technically yes but it is a matter of maintainability and complexity if we want to support all of them.

I'm also not yet familiar with reverse depth buffer so I can't tell whether it's possible to use it as a full replacement for logarithmic depth buffer or not. Ideally, we have just one implementation for better supporting large-scale view frustums.

@sam6321
Copy link
Contributor Author

sam6321 commented Jul 3, 2021

The current logarithmic depth buffer implementation has worked very well for my use case (earth-scale scene), but I think the drawbacks to using fragment log depth over vertex log depth for some use cases justify letting the user control which implementation they use.

@Mugen87
Copy link
Collaborator

Mugen87 commented Aug 27, 2021

Sorry, but this issue has to be solved differently. As discussed in the morph targets texture PR, such implementation specific flags are not user-friendly (see #22293 (comment)). It's better to provide a solution that just "works". It seems more promising to check out reverse depth buffer (see BabylonJS/Babylon.js#7133).

@Mugen87 Mugen87 closed this Aug 27, 2021
@sam6321
Copy link
Contributor Author

sam6321 commented Aug 29, 2021

The current system in threejs does just work for my use case. The change that I originally referenced (#22041) would break my application if it was merged, so I wanted to present this as an alternative to avoid that.

If this sort of change isn't a route that threejs wants to go down, I understand. I'd just ask that the older fragment logdepth features aren't removed because I rely on them.

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