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.

Sign up for GitHub

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

Jump to bottom

WebGLRenderer: Add support for changing the assigned render target depth texture #28584

Merged
merged 7 commits into from
Aug 5, 2024
+90 −4
Merged

WebGLRenderer: Add support for changing the assigned render target depth texture #28584

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7b420b1
Add support for swapping depth textures
gkjohnson Jun 7, 2024
bf626e4
Don't swap textures if cube target
gkjohnson Jun 8, 2024
9daaabe
Don't recreate render buffers
gkjohnson Jun 8, 2024
ecc85ea
Only attach the buffers if they have been created already
gkjohnson Jun 8, 2024
e3aabfd
remove unused argument
gkjohnson Jun 8, 2024
929f112
Don't rebind the buffers if we can
gkjohnson Jun 11, 2024
3175f66
Handle cases where the depth texture is disposed and unbound
gkjohnson Jun 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/renderers/WebGLRenderer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2268,6 +2268,22 @@ class WebGLRenderer {
// Color and depth texture must be rebound in order for the swapchain to update.
textures.rebindTextures( renderTarget, properties.get( renderTarget.texture ).__webglTexture, properties.get( renderTarget.depthTexture ).__webglTexture );

} else if ( renderTarget.depthBuffer ) {

// Swap the depth buffer to the currently attached one
const depthTexture = renderTarget.depthTexture;
if (
depthTexture &&
properties.has( depthTexture ) &&
( renderTarget.width !== depthTexture.image.width || renderTarget.height !== depthTexture.image.height )
) {

throw new Error( 'WebGLRenderTarget: Attached DepthTexture is initialized to the incorrect size.' );

}

textures.setupDepthRenderbuffer( renderTarget );

}

const texture = renderTarget.texture;
Expand Down
7 changes: 7 additions & 0 deletions src/renderers/webgl/WebGLProperties.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@ function WebGLProperties() {

let properties = new WeakMap();

function has( object ) {

return properties.has( object );

}

function get( object ) {

let map = properties.get( object );
Expand Down Expand Up @@ -36,6 +42,7 @@ function WebGLProperties() {
}

return {
has: has,
get: get,
remove: remove,
update: update,
Expand Down
45 changes: 40 additions & 5 deletions src/renderers/webgl/WebGLTextures.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1594,6 +1594,15 @@ function WebGLTextures( _gl, extensions, state, properties, capabilities, utils,
const renderTargetProperties = properties.get( renderTarget );
const isCube = ( renderTarget.isWebGLCubeRenderTarget === true );

// check if the depth texture is already bound to the frame buffer and that it's been initialized
if ( renderTargetProperties.__boundDepthTexture === renderTarget.depthTexture && properties.has( renderTarget.depthTexture ) ) {
Copy link
Collaborator

@Mugen87 Mugen87 Jun 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Can we move this check in setRenderTarget() and do it before checking the dimensions of the depth texture? The dimensions check should be only relevant when exchanging depth textures and not in the default case.

Copy link
Collaborator Author

@gkjohnson gkjohnson Jun 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I realized this is a bit more complicated when I was thinking through it again. Hopefully the current change makes sense. We need account for the case the where a depth texture is used and already bound on a render target, then disposed - meaning the depth texture is implicitly unbound so we need to make sure the three.js state reflects this. I tested and pushed the current change to the depth peeling demo page.


return;

}

renderTargetProperties.__boundDepthTexture = renderTarget.depthTexture;

if ( renderTarget.depthTexture && ! renderTargetProperties.__autoAllocateDepthBuffer ) {

if ( isCube ) throw new Error( 'target.depthTexture not supported in Cube render targets' );
Expand All @@ -1609,16 +1618,42 @@ function WebGLTextures( _gl, extensions, state, properties, capabilities, utils,
for ( let i = 0; i < 6; i ++ ) {

state.bindFramebuffer( _gl.FRAMEBUFFER, renderTargetProperties.__webglFramebuffer[ i ] );
renderTargetProperties.__webglDepthbuffer[ i ] = _gl.createRenderbuffer();
setupRenderBufferStorage( renderTargetProperties.__webglDepthbuffer[ i ], renderTarget, false );

if ( renderTargetProperties.__webglDepthbuffer[ i ] === undefined ) {

renderTargetProperties.__webglDepthbuffer[ i ] = _gl.createRenderbuffer();
setupRenderBufferStorage( renderTargetProperties.__webglDepthbuffer[ i ], renderTarget, false );

} else {

// attach buffer if it's been created already
const glAttachmentType = renderTarget.stencilBuffer ? _gl.DEPTH_STENCIL_ATTACHMENT : _gl.DEPTH_ATTACHMENT;
const renderbuffer = renderTargetProperties.__webglDepthbuffer[ i ];
_gl.bindRenderbuffer( _gl.RENDERBUFFER, renderbuffer );
_gl.framebufferRenderbuffer( _gl.FRAMEBUFFER, glAttachmentType, _gl.RENDERBUFFER, renderbuffer );

}

}

} else {

state.bindFramebuffer( _gl.FRAMEBUFFER, renderTargetProperties.__webglFramebuffer );
renderTargetProperties.__webglDepthbuffer = _gl.createRenderbuffer();
setupRenderBufferStorage( renderTargetProperties.__webglDepthbuffer, renderTarget, false );

if ( renderTargetProperties.__webglDepthbuffer === undefined ) {

renderTargetProperties.__webglDepthbuffer = _gl.createRenderbuffer();
setupRenderBufferStorage( renderTargetProperties.__webglDepthbuffer, renderTarget, false );

} else {

// attach buffer if it's been created already
const glAttachmentType = renderTarget.stencilBuffer ? _gl.DEPTH_STENCIL_ATTACHMENT : _gl.DEPTH_ATTACHMENT;
const renderbuffer = renderTargetProperties.__webglDepthbuffer;
_gl.bindRenderbuffer( _gl.RENDERBUFFER, renderbuffer );
_gl.framebufferRenderbuffer( _gl.FRAMEBUFFER, glAttachmentType, _gl.RENDERBUFFER, renderbuffer );

}

}

Expand Down Expand Up @@ -1763,7 +1798,7 @@ function WebGLTextures( _gl, extensions, state, properties, capabilities, utils,

_gl.bindRenderbuffer( _gl.RENDERBUFFER, null );

if ( renderTarget.depthBuffer ) {
if ( renderTarget.depthBuffer && renderTargetProperties.__webglDepthRenderbuffer === undefined ) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand this bit.

setupRenderTarget() is only called when __webglFramebuffer is undefined (so no framebuffer creation happened so far). When is __webglFramebuffer, undefined but not __webglDepthRenderbuffer?

Copy link
Collaborator Author

@gkjohnson gkjohnson Jun 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Just removed this. I added it when checking whether the depth buffer needed to be initialized in other functions but you're right it's not needed here.


renderTargetProperties.__webglDepthRenderbuffer = _gl.createRenderbuffer();
setupRenderBufferStorage( renderTargetProperties.__webglDepthRenderbuffer, renderTarget, true );
Expand Down