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: Allow for binding and rendering into a 2d render target mipmap #29844

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

gkjohnson
Copy link
Collaborator

@gkjohnson gkjohnson commented Nov 8, 2024

Related issue: #29779

Description

Adjusts setRenderTarget to afford binding and rendering into a specific mipmap level of a 2d render target texture. A scratch frame buffer is used to avoid cases where there are other attachments on the cached framebuffer without mipmaps (ie a depth buffer) resulting in an error when drawing (which was happening with 3d render targets, as well).

  • Depth buffers & rendering into depth textures, multiple render targets are not supported.
  • Mipmaps are now no longer automatically generated when rendering into a mipmap layer.

cc @CodyJasonBennett

edit looks like this will need some different handling for multisample targets but I'm not sure what the best way is to handle that Fixed by not rebinding mipmap level == 0 MSRT textures

@gkjohnson gkjohnson added this to the r171 milestone Nov 8, 2024
Copy link

github-actions bot commented Nov 8, 2024

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 339.22
79.04
339.41
79.07
+192 B
+37 B
WebGPU 476.9
132.19
476.9
132.19
+0 B
+0 B
WebGPU Nodes 476.2
132.01
476.2
132.01
+0 B
+0 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Before After Diff
WebGL 464.67
111.99
464.87
112.03
+192 B
+38 B
WebGPU 545.93
147.86
545.93
147.86
+0 B
+0 B
WebGPU Nodes 501.81
137.56
501.81
137.56
+0 B
+0 B

@CodyJasonBennett
Copy link
Contributor

I'm very sick right now, so I can't give this my full attention until the end of next week.

Just a quick glance: bullet points are reasonable and code looks nonintrusive so far.

If I want hardware filtering for depth and shadows, I'll add it myself. No need for that burden now.

@gkjohnson
Copy link
Collaborator Author

Hey @CodyJasonBennett - just wanted to ping again. No rush though - hope you're feeling better!

@CodyJasonBennett
Copy link
Contributor

Yes, feeling much better. I'm hopping on a plane in a few hours, but just ping or assign me and I'll go through them first thing when I land. Thanks again for leading this stuff.

@CodyJasonBennett
Copy link
Contributor

How are you testing this? I'm noticing I have to disable depthBuffer or three will crash when looking for a depth attachment when rendering to mipmaps. In the following example, I render unique colors for each mip level, but I only see the first mip when sampling from it with textureLod. Disabling L73 will show the expected result with a unique color per mip level.

https://jsfiddle.net/7jtmzpoL

@gkjohnson
Copy link
Collaborator Author

I'm noticing I have to disable depthBuffer or three will crash when looking for a depth attachment when rendering to mipmaps.

This looks like it's maybe related to setting the mipmaps array - which I am not doing and generally seems not-so-well documented or tested especially for render targets. I'm just setting generateMipmaps to true so the texture storage is available.

In the following example, I render unique colors for each mip level, but I only see the first mip when sampling from it with textureLod. Disabling L73 will show the expected result with a unique color per mip level.

https://jsfiddle.net/7jtmzpoL

If you set renderTarget.texture.minFilter to THREE.LinearMipmapLinearFilter the fiddle works. Looks like textureLod respects the minification sampling filter.

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.

2 participants