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: Removed gl_FragDepth-based logarithmic depth buffer code #22041

Closed
wants to merge 1 commit into from

Conversation

mrdoob
Copy link
Owner

@mrdoob mrdoob commented Jun 24, 2021

Related issue: #22017

Description

This PR removes all gl_FragDepth-based logarithmic depth buffer related code.

@mrdoob mrdoob added this to the r130 milestone Jun 24, 2021
@mrdoob
Copy link
Owner Author

mrdoob commented Jun 24, 2021

@jbaicoianu what do you think about this? reason in #22017

@gkjohnson
Copy link
Collaborator

Is this previous issue related? #17384

It looks like this PR is changing the behavior of log depth buffer to only apply to vertices rather than fragments which I don't believe is correct.

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 25, 2021

Sidenote: If we decide to keep the current implementation, we should document the MSAA restrictions at the doc page of WebGLRenderer.

@mrdoob mrdoob modified the milestones: r130, r131 Jun 30, 2021
@sam6321
Copy link
Contributor

sam6321 commented Jul 2, 2021

Would it be possible maintain both log depth buffer implementations and leave it up to the user to decide which they want to use?

I use the fragment log depth buffer implementation as part of a 3D globe application, and there are quite a lot of rendering artifacts when I swap to the vertex method.

I would suggest using the vertex method by default, with an option to use the fragment method if desired (Basically the option that was described here).

EDIT: I made a PR for this here: #22082

@mrdoob mrdoob modified the milestones: r131, r132 Jul 28, 2021
@mrdoob mrdoob modified the milestones: r132, r133 Aug 26, 2021
@sam6321
Copy link
Contributor

sam6321 commented Aug 29, 2021

Just a note, after the alternative PR I made was declined, that I have an application that relies on threejs' current fragment log depth implementation that would break if this PR was merged and that implementation was removed.

Are there any other acceptable alternatives that can keep fragment log depth around?

@Mugen87
Copy link
Collaborator

Mugen87 commented Aug 29, 2021

I vote to keep the current logarithmic depth buffer code until a new solution/replacement (reverse depth buffer) is available.

@mrdoob mrdoob modified the milestones: r133, r134 Sep 30, 2021
@mrdoob mrdoob modified the milestones: r134, r135 Oct 28, 2021
@mrdoob mrdoob modified the milestones: r135, r136 Nov 26, 2021
@mrdoob mrdoob modified the milestones: r136, r137 Dec 24, 2021
@mrdoob mrdoob modified the milestones: r137, r138 Jan 26, 2022
@mrdoob mrdoob modified the milestones: r138, r139 Feb 23, 2022
@mrdoob mrdoob modified the milestones: r139, r140 Mar 24, 2022
@mrdoob mrdoob modified the milestones: r145, r146 Sep 29, 2022
@mrdoob mrdoob modified the milestones: r146, r147 Oct 27, 2022
@mrdoob mrdoob modified the milestones: r147, r148 Nov 30, 2022
@mrdoob mrdoob modified the milestones: r148, r149 Dec 22, 2022
@mrdoob mrdoob modified the milestones: r149, r150 Jan 26, 2023
@mrdoob mrdoob modified the milestones: r150, r151 Feb 23, 2023
@mrdoob mrdoob modified the milestones: r151, r152 Mar 30, 2023
@mrdoob mrdoob modified the milestones: r152, r153 Apr 27, 2023
@Mugen87 Mugen87 modified the milestones: r153, r154 Jun 1, 2023
@mrdoob mrdoob modified the milestones: r154, r155 Jun 29, 2023
@mrdoob mrdoob modified the milestones: r155, r156 Jul 27, 2023
@mrdoob mrdoob modified the milestones: r156, r157 Aug 31, 2023
@mrdoob mrdoob modified the milestones: r157, r158 Sep 28, 2023
@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 6, 2023

As mentioned in #22041 (comment), we can't sacrifice per-pixel depth values since this potentially break applications. Until an alternative is found, let's keep using GL_EXT_frag_depth with its emulated fallback.

@Mugen87 Mugen87 closed this Oct 6, 2023
@Mugen87 Mugen87 removed this from the r158 milestone Oct 6, 2023
@mrdoob mrdoob deleted the logdepthbuf branch October 11, 2023 05:09
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.

4 participants