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

WebGLRenderTarget: Support for custom depth and stencil buffers #15490

Closed
wants to merge 8 commits into from

Conversation

pailhead
Copy link
Contributor

@pailhead pailhead commented Dec 29, 2018

Problem

Multiple WebGLRenderTarget can't share the same depth / stencil render buffer because WebGLRenderer creates them in a private cache. This is possible to do with webgl, but not with three.js.

Proposal

Have the WebGLRenderer attempt to use an existing buffer if one is provided. And allow the targets to be used so:

targetA.ownDepthBuffer = buffer
targetB.ownDepthBuffer = buffer

Use case

This depth peel example, goes from not being interactive to working on phones when stencil masking is used (video).

Argument for

There are a few things in three.js that already work like this i think. defines, customDepthMaterial are the two that pop to mind - you could set them, and they would make WebGLRenderer do certain things, but were not present in documentation for a long time. This does not change the behavior of the renderer in any way, and allows WebGL to be fully utilized.

Optional

Document it:

Sets the render buffer on the target. If one is not provided, three creates one internally for this target. You can share render buffers between targets:

const gl = renderer.getContext()

const buffer = gl.createRenderBuffer()

targetA.ownDepthBuffer = buffer
targetB.ownDepthBuffer = buffer

Future (if accepted)

Make an API:

const buffer = new THREE.RenderBuffer(config)

targetA.depthBuffer = buffer
targetB.depthBuffer = buffer

@pailhead
Copy link
Contributor Author

bump

@MEBoo
Copy link

MEBoo commented May 9, 2019

Hi!
So are we getting the Deep Peel Demo officially supported?
After that... is complicated to include the new transparency management?
Could be integrated in threejs as an option?

@pailhead
Copy link
Contributor Author

pailhead commented May 9, 2019

This PR specifically addresses the render buffers of render targets being locked. IMHO three.js is being too opinionated here and would benefit from releasing this lock. Depth peeling is just one of the examples that could leverage this, there are probably more. Not sure what the concern with this is though :)

@Ben-Mack
Copy link

Transparency sorting has always been a headache. Would love to see this PR merged to support the depth peel example.

@mrdoob Is there any obstacle preventing this PR being merged?

@pailhead
Copy link
Contributor Author

I have to admit that this is interesting. Why is three.js so opinionated when it comes to this? WebGL allows for this out of the box.

@anderscognite
Copy link
Contributor

I would love to see this too!

@RandomShaper
Copy link

What about this: e77697f?

I haven't tried it yet, but seems to allow for that.

@pailhead
Copy link
Contributor Author

pailhead commented Oct 3, 2019

@RandomShaper

This looks like it could work. I'll test it. It's a bit confusing that DepthTexture has all these properties like mip maps and filtering, but it looks like they might not even be used.

edit

On further inspection this seems like it would have to rely on an extension while it can work without one. Still worth testing.

@MEBoo
Copy link

MEBoo commented Feb 4, 2020

@pailhead any news about? we everybody need OIT :)

@pailhead
Copy link
Contributor Author

pailhead commented Feb 4, 2020

None unfortunately :(

And I don’t think this might change any time soon. In the meantime I found more examples of three being opinionated. May be best so look into this class mentioned above, I don’t have time atm.

@gkjohnson
Copy link
Collaborator

gkjohnson commented Mar 25, 2020

I'll move my question here since the other discussion was closed:

@pailhead Can you explain a bit more about why using DepthTexture wouldn't work for depth peeling? At least on the surface it seems to be what you're interested in, granted it requires an extension to work.

@DusanBosnjakKodiak
Copy link

DusanBosnjakKodiak commented Mar 26, 2020

@gkjohnson

In all honesty i didn't give it a shot :) It does look like it could get the job done, but it also seemed like it was using a different path. The way it was achieved requires no extensions (i think). It may be worth a shot.
It may also be that i was more comfortable using the stencil buffer on the gl context directly. I'll dig up a cleaner example of depth peeling if you want to give it a go.

(actually the link from the first post seems to be the latest)

Yeah, now that i've jogged my memory, i got intimidated with the mentions of texture like properties (for example related to mip mapping) that made me decide that this is not suitable. However, this could be just a sideffect of piggy backing on the Texture framework, maybe it's not being actually used for this particular type of texture.

@Mugen87

I think i'm more interested in the pattern, than this particular feature. Granted things may have changed and there could be less of this going on. When this was made, my observation was that there is this pattern present in code, and that it could be exploited in other places if it seems alright:

//inside WebGLRenderer:

   _somethingPrivate.foo = foo_from_outside_world || ___super_private_cache___._internal_foo_

@gkjohnson
Copy link
Collaborator

gkjohnson commented Mar 26, 2020

@DusanBosnjakKodiak

The way it was achieved requires no extensions (i think). It may be worth a shot.
...
Yeah, now that i've jogged my memory, i got intimidated with the mentions of texture like properties (for example related to mip mapping) that made me decide that this is not suitable.

The depth texture extenion is supported in every browser so there's no real reason not to use it. And mip map generation isn't required -- it's a really just a depth buffer that can be sampled as a texture in shaders. Other than that it doesn't seem like there's a whole lot that's different so I think it's a good fit for this.

I might interested in depth peeling coming up here, which is why I'm asking. I might give it a go with DepthTexture.

I agree it might be nice to be able to create a shareable depth (and color! #18860) buffer between render targets, though. I think an API like this might fit into three.js' model a bit more:

const renderTarget = new WebGLRenderTarget( 256, 256 );

renderTarget.colorTarget = new ColorBuffer( 256, 256 );
renderTarget.depthTarget = new DepthBuffer( 256, 256 );

// OR

renderTarget.depthTarget = new DepthTexture( 256, 256 );

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 26, 2020

The depth texture extenion is supported in every browser so there's no real reason not to use it.

That does not mean each device supports it. The support of WebGL 1 extensions is something you have to check for each user. Especially on (older) mobile device the support can be still problematic.

@gkjohnson
Copy link
Collaborator

That does not mean each device supports it. The support of WebGL 1 extensions is something you have to check for each user. Especially on (older) mobile device the support can be still problematic.

Oh okay got it -- might be another reason to support setting a custom non-texture the DepthBuffer, then

@DusanBosnjakKodiak
Copy link

I have to admit that i'm not entirely clear on the difference. From the description i seem to recall that it should do the same thing. @Mugen87 if this is not supported on some devices (i know some extenions have greated coverage) would it mean the render buffer would work on all?

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 26, 2020

Renderbuffers always work. Let's recall that a render target represents a custom framebuffer object. Framebuffers have attachments which represents in some sense the actual buffers that hold rendering data. These data can be saved in renderbuffers or textures. The difference between both entities is that you can't sample a renderbuffer like a texture. If you want to implement depth peeling and need to sample depth values in the shader, you need to bind a texture to the framebuffer's depth attachment.

Unfortunately, you can only save depth information directly in a texture with the mentioned extension. If that is missing, you need RGBA depth packing (the primary purpose of MeshDepthMaterial) and definitely a separate render pass.

@gkjohnson
Copy link
Collaborator

@Mugen87

Unfortunately, you can only save depth information directly in a texture with the mentioned extension. If that is missing, you need RGBA depth packing (the primary purpose of MeshDepthMaterial) and definitely a separate render pass.

I don't think the suggestion is that depth peeling requires sampling a depth buffer but rather that it benefits from reusing an existing depthbuffer when rendering to a different color attachment:

const depthTarget = new DepthTexture( ... );
const renderTarget1 = new WebGLRenderTarget( ... );
const renderTarget2 = new WebGLRenderTarget( ... );

renderTarget1.depthTexture = depthTarget;
renderTarget2.depthTexture = depthTarget;

Now renderTarget2 can reuse the depthbuffer data written when renderTarget1 was rendered to. Beyond depth peeling another use case is using a previously rendered to depthbuffer when rendering something like a normal pass to avoid unneeded fragment calculations (like a depth prepass). The addition of DepthTexture inadvertently exposed this ability.

Adding something like a DepthBuffer() API might be pretty simply added by modifying the setupDepthTexture function here.

@pailhead
Copy link
Contributor Author

pailhead commented Mar 26, 2020

I think my example is still using depth packing (there are some issues on ios, which I recall seeing mentioned in another issue). The performance benefit came from being able to write and use the same stencil buffer. I’m still not sure if this is possible with the mentioned class.

Just to clarify, the depth peel itself, I had trouble doing with depth buffers alone, hence rendering to textures. But reusing the stencil allows for flip flopping between textures and log amount of pixel work with each iteration.

@pailhead
Copy link
Contributor Author

pailhead commented May 8, 2020

@mrdoob @Mugen87

Hi guys. This PR has been stuck for a bit with no feedback and no milestone but it’s till open?

What can we expect to happen with this if anything?

@valenn
Copy link

valenn commented Mar 10, 2021

bump

Is this something that is planned to be merged eventually? Use case I came across is rendering opaque objects to one render target, and then rendering transparents to different render target. It would be nice to reuse depth buffer from opaque pass without copying anything, especially on lower end devices

@pailhead
Copy link
Contributor Author

pailhead commented Jan 21, 2022

w00t hopefully all proposed changes have been addressed. I used google translate for the chinese docs. I've put the usage example in the docs as well.

Not sure if the usage example should include how to obtain the render context, just let me know!

@pailhead
Copy link
Contributor Author

Hmm, i'm not actually sure how to make this green, this DeepScan thing doesn't look like it's at all related to these two loc changes.

@Ben-Mack
Copy link

@mrdoob This PR seems good to merge now, could you take a look

@pailhead
Copy link
Contributor Author

With the docs in Chinese now, there is potential for many more conflicts. But i don't mind resolving the conflicts over and over, but does anyone know what this deep scan thing is complaining about?

@mrdoob
Copy link
Owner

mrdoob commented Mar 17, 2022

It's not clear to me what the issue is with the DepthTexture approach.
Is the problem that it required a WebGL 1 extension? Is that still relevant now that we're moving to WebGL 2?

@mrdoob mrdoob changed the title allow render targets to use their own depth/stencil buffers WebGLRenderTarget: Support for custom depth and stencil buffers Mar 17, 2022
@pailhead
Copy link
Contributor Author

pailhead commented Mar 17, 2022

That’s a good question! This was made though before the webgl 2 move I imagine. It may be totally fixed by webgl 3 :)

I'm unfortunately not using three.js in my daily work, so i don't know the answer to this. However, the main point here may be that it's not expected for this to be clear to everyone. If you never had a need to use something like this, it may be pretty complicated to explain how and why it works.

On the other hand, it is completely optional, that's my only argument for why this may be worth trying out.

I've never tried the DepthTexture approach, @Mugen87 and @gkjohnson seem to have brought that up, so it may be worth asking them.

It is possible that only @Ben-Mack ever finds a use and understands this feature, but i feel that they should still be allowed to use it. In my view this should be a door with the sign "free passage" yet someone locked it. We're discussing if we should unlock the door, while no one seems to be sure why the door was locked in the first place :)
(Ie. You can do this with webgl, not threejs, why?)

@mrdoob
Copy link
Owner

mrdoob commented Mar 17, 2022

@Ben-Mack Does the DepthTexture approach solve your use case?

@Ben-Mack
Copy link

My main use case is to implement Depth Peeling by @pailhead , he's said this PR is blocking him from releasing a DepthPeel module.
Transparency sorting has always been a headache problem with many issues raised overtime. This PR will make DepthPeel performance reasonable, thus provide a great solution to fix transparency sorting, many will benefit from this I believe.

@mrdoob
Copy link
Owner

mrdoob commented Mar 17, 2022

I guess we just need someone to try and see if it's possible to do depth peeling using DepthTexture then.

@pailhead
Copy link
Contributor Author

pailhead commented Mar 17, 2022

Oooh, I think this defeats the purpose of the PR then. There was also an issue linked to this with a similar discussion. Depth peeling is not the only thing that this could be used for I imagine.

It’s a simple issue - if I can hand code some webgl call list, there is no theoretical possibility that I can generate it with threejs. It may be possible to generate a different list that would do the same thing. But if we need someone to investigate that, why just not allow the former (for now)?

@pailhead
Copy link
Contributor Author

I guess we just need someone to try and see if it's possible to do depth peeling using DepthTexture then.

@gkjohnson you mentioned this above, have you ever tried it?

@gkjohnson
Copy link
Collaborator

gkjohnson commented Mar 18, 2022

I guess we just need someone to try and see if it's possible to do depth peeling using DepthTexture then.

@gkjohnson you mentioned this above, have you ever tried it?

I had tried this but unfortunately it doesn't just work. Depth peeling (and other effects) are easier when you can swap out / save / share a depth target but once a render target has been rendered once with a particular depth texture it cannot be changed. See #19447. I think adjusting the WebGLRenderer to be able to detect and change which depth attachment is being used with a target render texture would effectively solve this problem and be aligned with existing three.js APIs.

I think my current hesitation with the currently suggested approach is that it requires the user to create a native GL render buffer instance which isn't required anywhere else in three. And for some context this demo that implements translucent materials using depth peeling is how I found the issue.

I would be very supportive of #19447 being addressed, though. I just can't put the effort in myself to make it happen right now.

@pailhead
Copy link
Contributor Author

I think my current hesitation with the currently suggested approach is that it requires the user to create a native GL render buffer instance which isn't required anywhere else in three.

This has been the case with the stencil buffer for years though. Granted it wasn't plugged directly into a property to be handled by the renderer, but it sort of was when it was plugged into onBeforeRender .

There simply was no other way to use the stencil buffer (a webgl feature) unless the gl context was obtained. Per OP this is advocating for the same treatment. To see if it even works, have this experimental feature (it could be documented as such - experimental), which allows to use a webgl feature (just like stencil ops in the past) with three. Unfortunately i think this is much more complicated to setup then just enabling some stencil state before a draw call, using any existing hooks.

@pailhead
Copy link
Contributor Author

pailhead commented Mar 19, 2022

Also, come to think of it, what would the API here even do? Looks to me like it could be a constant like THREE.RenderBuffer or THREE.RenderBufferReference , the default would be THREE.CreateBufferForTarget or something along those lines. Point is, it's just a null, or some ref, if two targets have null everything happens as it did, it's a noop. If two targets have like 1, then the renderer would create one buffer, and reuse the cache on the other. Does not look like it would add a lot of code, just some thought needs to be put into how this would look.

depthBuffer: null | number seems kinda awkward, but it should either be a number or an object that can be compared with ===

myTexture.depthBuffer = THREE.createRenderBufferReference()

Would potentially just return a number.

tl:dr;

If one has gl , the proposal here was to just use gl.createRenderBuffer() to create a reference. Since it's a render buffer already, the code that needs to change in the renderer is minimal. Representing this reference with some other type, would slightly increase the code (which is pretty low as it is though). In even less words - Texture was used directly as cache here.

@gkjohnson
Copy link
Collaborator

Granted it wasn't plugged directly into a property to be handled by the renderer, but it sort of was when it was plugged into onBeforeRender.

I think this is fairly big difference. There was never anything in the API that encouraged running raw gl functions. Stencils were simply never supported and it happened to be the case that you could adjust the stencil flags by hacking the gl context directly.

Also, come to think of it, what would the API here even do?

In #15490 (comment) I lay out how I think this should look and other use cases. Now that WebGL2 seems to be the norm I'd think it would be okay to require using DepthTexture to make reusable depth buffers:

const depthTarget1 = new DepthTexture( ... );
const depthTarget2 = new DepthTexture( ... );
const renderTarget1 = new WebGLRenderTarget( ... );
const renderTarget2 = new WebGLRenderTarget( ... );

renderTarget1.depthTexture = depthTarget;
renderTarget2.depthTexture = depthTarget;

// ...

// this does nothing right now
renderTarget2.depthTexture = depthTarget2;

You should be able to change the depth buffer attached to a render target so it can be depth tested against while color is rendered to a different target and then be able to change that depth texture again later. This doesn't work at the moment. As a user the fact that you can't swap out the depth texture after rendering to a target once feels like a bug (though I understand it may not be by the original implementation intent) and imo that should be addressed.

I think the current proposal in this PR is a bit of a half baked implementation to get a hack in to support this feature. I want this, too, for a few reasons including Depth Peeling but imo it should be done correctly and in alignment with the existing three.js APIs.

@pailhead
Copy link
Contributor Author

pailhead commented Mar 20, 2022

Stencils were simply never supported and it happened to be the case that you could adjust the stencil flags by hacking the gl context directly.

To me this seems like hacking, but much less hacking. You aren’t explicitly setting gl state, you’re just passing in a gl resource.

I guess it can be said that these are valid concerns but it would have been nice if this was voiced earlier. Especially before asking for docs in Chinese :)

I have to admit though, refusing a half baked solution over not having a solution makes me a bit of a sad panda 😢

@gkjohnson
Copy link
Collaborator

I guess it can be said that these are valid concerns but it would have been nice if this was voiced earlier. Especially before asking for docs in Chinese :)

My previous comments on a different API were an effort to suggest that the proposed solution didn't seem adequate. Sorry for not being more clear. Regarding the docs request - I do agree that there has been a tendency to request more work (docs, examples) which gives the impression that everything else looks good before discussing or committing to whether or not the core feature contribution is mergeable. I agree it's frustrating and hopefully that can be considered in the future.

To that end I want to be clear that these are just my opinions on the topic and I can't speak for the project. I think it would be good if @mrdoob or @Mugen87 could comment on whether what I've suggested is a reasonable solution for this feature if something else is required so someone can work on it if they feel inclined. Depth peeling would be a big step toward better transparency and support for multilayer, depth-aware material transmission.

@pailhead
Copy link
Contributor Author

I can take a look at what’s involved to not use gl

@gkjohnson
Copy link
Collaborator

Looks like Babylonjs just added support for depth peeling:

https://doc.babylonjs.com/divingDeeper/materials/advanced/transparent_rendering#order-independent-transparency

@pailhead
Copy link
Contributor Author

pailhead commented Apr 25, 2022

Also looks like someone has figured out a way to do this. I didn't know properties are available, looks like the renderer can be triggered from outside to create the buffer, it doesn't have to be part of the render call.

https://discourse.threejs.org/t/apply-depth-buffer-from-render-target-to-the-next-render/37276/2

Might still be worth to document this, and maybe look into a cleaner API ie. something that doesn't use the double underscore (eg. __webglDepthRenderbuffer).

I'm not a huge fan of setting the gl state right before the call using renderer.getContext() but it looks like it's been like this, by design, for as long as renderer.properties were available.

I intend to look into this PR and what could be done about the API, but i think this thread works for me. Interesting that this was here all these years, yet no one was really sure. I had no idea you could access .properties 😵‍💫

edit

@gkjohnson

I think my current hesitation with the currently suggested approach is that it requires the user to create a native GL render buffer instance which isn't required anywhere else in three.

Looks like this was not needed, three can already do this and does not involve native buffers. The pre/post render call is a bit native, maybe this could be changed? I don't want to mess with the current design before making sure it's going in the right direction.

As long as one doesn't use renderer.getContext() to create resources, it's not a concern, setting state is fine?

@pailhead
Copy link
Contributor Author

pailhead commented Apr 25, 2022

Looks like Babylonjs just added support for depth peeling:

I think theoretically, it's possible that three added this almost 4 years ago 😉 . Definitely not as user friendly as useDepthPeel = true but for all intents and purposes, it could be treated as a part of three as much as any other example?

I think it may be easier to revisit this PR and use the official API like so:

const renderBufferProps = renderer.properties.get(gbuffer)
depthRenderBuffer = renderBufferProps.__webglDepthRenderbuffer || renderBufferProps.__webglDepthbuffer
const _gl = renderer.getContext();
_gl.framebufferRenderbuffer( _gl.FRAMEBUFFER, _gl.DEPTH_ATTACHMENT, _gl.RENDERBUFFER, depthRenderBuffer );

And try to make it work as it was originally written (no dual depth peel)?

#15312

@ingun37
Copy link
Contributor

ingun37 commented Jun 3, 2022

Hello, Is there an depth peeling example that uses DepthTexture approach?

@gkjohnson
Copy link
Collaborator

Closing in favor of #28584

@gkjohnson gkjohnson closed this Sep 18, 2024
@gkjohnson gkjohnson removed this from the r??? milestone Sep 18, 2024
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.