Skip to content

Commit

Permalink
make XR renderingg more efficient
Browse files Browse the repository at this point in the history
  • Loading branch information
cabanier committed Jan 29, 2025
1 parent 722487e commit 2fe59c2
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 5 deletions.
8 changes: 8 additions & 0 deletions src/renderers/common/RenderContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,14 @@ class RenderContext {
*/
this.clearStencilValue = 1;

/**
* Whether the depth and stencil should be skipped during copy operations.
*
* @type {Boolean}
* @default true
*/
this.skipDepthStencilCopy = false;

/**
* By default the viewport encloses the entire framebuffer If a smaller
* viewport is manually defined, this property is to `true` by the renderer.
Expand Down
14 changes: 12 additions & 2 deletions src/renderers/common/Renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import { Matrix4 } from '../../math/Matrix4.js';
import { Vector2 } from '../../math/Vector2.js';
import { Vector4 } from '../../math/Vector4.js';
import { RenderTarget } from '../../core/RenderTarget.js';
import { DoubleSide, BackSide, FrontSide, SRGBColorSpace, NoToneMapping, LinearFilter, LinearSRGBColorSpace, HalfFloatType, RGBAFormat, PCFShadowMap } from '../../constants.js';
import { DoubleSide, BackSide, FrontSide, SRGBColorSpace, NoToneMapping, LinearFilter, LinearSRGBColorSpace, HalfFloatType, RGBAFormat, PCFShadowMap, UnsignedByteType } from '../../constants.js';

/** @module Renderer **/

Expand Down Expand Up @@ -1128,7 +1128,7 @@ class Renderer {
frameBufferTarget = new RenderTarget( width, height, {
depthBuffer: depth,
stencilBuffer: stencil,
type: HalfFloatType, // FloatType
type: UnsignedByteType,
format: RGBAFormat,
colorSpace: LinearSRGBColorSpace,
generateMipmaps: false,
Expand Down Expand Up @@ -1255,6 +1255,8 @@ class Renderer {

}

if ( outputRenderTarget ) renderContext.skipDepthStencilCopy = outputRenderTarget.needsDepthTexture === false;

//

let viewport = this._viewport;
Expand Down Expand Up @@ -1321,6 +1323,14 @@ class Renderer {

const renderTargetData = this._textures.get( renderTarget );

// Do not attach a depth buffer if XR doesn't need one and we're using MSAA
if ( renderContext.skipDepthStencilCopy && ( renderContext.sampleCount > 1 ) ) {

This comment has been minimized.

Copy link
@Mugen87

Mugen87 Jan 29, 2025

Owner

Why does the skip require MSAA? Can't we do that for the non-MSAA code path as well?

This comment has been minimized.

Copy link
@cabanier

cabanier Jan 30, 2025

Author

if we directly render to the non-msaa buffer, we need the depth buffer


renderTarget.depthBuffer = null;
renderTarget.stencilBuffer = null;

}

renderContext.textures = renderTargetData.textures;
renderContext.depthTexture = renderTargetData.depthTexture;
renderContext.width = renderTargetData.width;
Expand Down
1 change: 1 addition & 0 deletions src/renderers/common/XRManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,7 @@ class XRManager extends EventDispatcher {
} );

this._xrRenderTarget.hasExternalTextures = true;
this._xrRenderTarget.needsDepthTexture = glBinding.usesDepthValues;

This comment has been minimized.

Copy link
@Mugen87

Mugen87 Jan 29, 2025

Owner

What is the difference between glBinding.usesDepthValues and glProjLayer.ignoreDepthValues? Is it basically the same information just on different objects?

This comment has been minimized.

Copy link
@cabanier

cabanier Jan 30, 2025

Author

yes. The original intent was to make ignoreDepthValues read/write but no headset has signed up for that yet


} else {

Expand Down
8 changes: 8 additions & 0 deletions src/renderers/common/XRRenderTarget.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,14 @@ class XRRenderTarget extends RenderTarget {
*/
this.hasExternalTextures = false;

/**
* Whether the system compositor needs depth textures.
*
* @type {Boolean}
* @default true
*/
this.needsDepthTexture = true;

This comment has been minimized.

Copy link
@Mugen87

Mugen87 Jan 29, 2025

Owner

When introducing needsDepthTexture, do we still need autoAllocateDepthBuffer? I'm not sure I understand the difference between both properties.

I thought a depth renderbuffer is required if we don't use glSubImage.depthStencilTexture. And this texture is only used when glProjLayer.ignoreDepthValues evaluates to false.

This comment has been minimized.

Copy link
@cabanier

cabanier Jan 30, 2025

Author

I don't think this extra flag is needed. I added it because needsDepthTexture is confusing because we still need a depth texture


/**
* Whether a depth buffer should automatically be allocated
* for this XR render target or not.
Expand Down
9 changes: 7 additions & 2 deletions src/renderers/webgl-fallback/WebGLBackend.js
Original file line number Diff line number Diff line change
Expand Up @@ -1956,7 +1956,7 @@ class WebGLBackend extends Backend {

}

if ( descriptor.depthTexture !== null ) {
if (( descriptor.depthTexture !== null ) && (!descriptor.skipDepthStencilCopy || ( samples === 0 ))) {

const textureData = this.get( descriptor.depthTexture );
const depthStyle = stencilBuffer ? gl.DEPTH_STENCIL_ATTACHMENT : gl.DEPTH_ATTACHMENT;
Expand Down Expand Up @@ -2044,7 +2044,12 @@ class WebGLBackend extends Backend {
renderTargetContextData.depthRenderbuffer = depthRenderbuffer;

const depthStyle = stencilBuffer ? gl.DEPTH_STENCIL_ATTACHMENT : gl.DEPTH_ATTACHMENT;
invalidationArray.push( depthStyle );

if ( invalidationArray.indexOf( depthStyle ) === -1 ) {

invalidationArray.push( depthStyle );

}

}

Expand Down
2 changes: 1 addition & 1 deletion src/renderers/webgl-fallback/utils/WebGLTextureUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ class WebGLTextureUtils {

if ( glFormat === gl.DEPTH_COMPONENT ) {

if ( glType === gl.UNSIGNED_INT ) internalFormat = gl.DEPTH24_STENCIL8;
if ( glType === gl.UNSIGNED_INT ) internalFormat = gl.DEPTH_COMPONENT24;
if ( glType === gl.FLOAT ) internalFormat = gl.DEPTH_COMPONENT32F;

}
Expand Down

0 comments on commit 2fe59c2

Please sign in to comment.