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

Rendering into a framebuffer with .mipmaps set crashes if depth is used #30745

Closed
haxiomic opened this issue Mar 17, 2025 · 4 comments · Fixed by #30746
Closed

Rendering into a framebuffer with .mipmaps set crashes if depth is used #30745

haxiomic opened this issue Mar 17, 2025 · 4 comments · Fixed by #30746
Milestone

Comments

@haxiomic
Copy link
Contributor

haxiomic commented Mar 17, 2025

Description

It's possible render into specific mipmap levels when we set the target.texture.mipmaps array. This works great for color-only render targets, however if we give this target a depth buffer (either through depthBuffer: true or an explicit depthTexture), rendering will fail with a crash when rendering

The crash is caused by three.js assuming .__webglFramebuffer is a single framebuffer, however, when .mipmaps is set it becomes an array:

if (texture.mipmaps && texture.mipmaps.length > 0) {
  for (let level = 0; level < texture.mipmaps.length; level++) {
    setupFrameBufferTexture(renderTargetProperties.__webglFramebuffer[level], renderTarget, texture, _gl.COLOR_ATTACHMENT0, glTextureType, level);
  }
} else {
  setupFrameBufferTexture(renderTargetProperties.__webglFramebuffer, renderTarget, texture, _gl.COLOR_ATTACHMENT0, glTextureType, 0);
}

This causes a crash on this line

state.bindFramebuffer( _gl.FRAMEBUFFER, renderTargetProperties.__webglFramebuffer );

And

state.bindFramebuffer( _gl.FRAMEBUFFER, framebuffer );

Maybe other places too

See related

Reproduction steps

  1. Create a WebGLRenderTarget with depthBuffer: true or depthTexture set

  2. Try to render to this render target (any mipmap level)

  3. Crash during initialization of the target

Code

const target = new WebGLRenderTarget(2, 2, {
        depthBuffer: true,
});
target.mipmaps = [{}, {}];
renderer.setRenderTarget(target);
renderer.render(scene, camera);

Crash :[

Live example

latest

Screenshots

Image

Version

174

Device

Desktop

Browser

Chrome

OS

MacOS

@haxiomic
Copy link
Contributor Author

haxiomic commented Mar 17, 2025

I believe a solution is to use mip 0 for depth attachment

setupDepthTexture(
    Array.isArray(renderTargetProperties.__webglFramebuffer) ?
        renderTargetProperties.__webglFramebuffer[0] : 
        renderTargetProperties.__webglFramebuffer,
    renderTarget
);

We could also setup a depth texture for each of the mipmap framebuffers too. I need to think on how likely this is to be needed

@haxiomic
Copy link
Contributor Author

haxiomic commented Mar 17, 2025

Motivation:

I'm implementing fast bloom and reflections that blur with distance – for this I render the scene to mipmap 0, then set the other mipmap levels by progressively gaussian blurring

Unfortunately I need a depth buffer both for rendering the scene into mip 0 and for screen space ray marching

Using mipmaps is much preferable to a set of textures because sample a given blur radius using the miplevel is just

vec4 blurredReflection = textureLod(texture, pos, calculateLodForDistance(d))

And we get linear mipmap blending for free

without mipmapping this becomes many lines and meta coding to support an array of textures of different blur levels

@CodyJasonBennett
Copy link
Contributor

See #29844 (comment).

mipmaps is not a well supported field and crashes whereas generateMipmaps to enable mipmap storage works fine.

@haxiomic
Copy link
Contributor Author

haxiomic commented Mar 31, 2025

Indeed but it's very close! Motivating example: mipmap bloom & AO, we render the scene and blur color and depth progressively into the mipmap chain.

This way we can take advantage of mipmap blending when combining blur textures for bloom and sampling the blurred depth buffer for AO

This was made using my patch in PR #30746

Screen.Recording.2025-03-31.at.10__90pct_smaller.mp4
Image

@Mugen87 Mugen87 added this to the r176 milestone Mar 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants