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: Fix setRenderTarget()'s activeMipmapLevel #26347

Merged
merged 9 commits into from Jul 18, 2023

Conversation

makovkins
Copy link
Contributor

WebGLRenderer.setRenderTarget accepts 'activeMipmapLevel' parameter but actually it doesn't work.

@github-actions
Copy link

github-actions bot commented Jun 28, 2023

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev Filesize PR Diff
644.2 kB (159.7 kB) 645.2 kB (159.9 kB) +968 B

🌳 Bundle size after tree-shaking

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

Filesize dev Filesize PR Diff
437.3 kB (105.8 kB) 438.3 kB (106 kB) +970 B

@mrdoob
Copy link
Owner

mrdoob commented Jul 6, 2023

@Mugen87 What do you think about this one?

@makovkins
Copy link
Contributor Author

WebGL is very limited and you shouldn't limit it even more preventing the ability to render to mip levels. I use this feature in my project to generate custom mip levels for cube reflection captures. Possibly my implementation is not perfect but I can easily improve it if you tell me what to do.

@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 6, 2023

It would be good to provide a live example that demonstrate what use case currently breaks (so we can see that rendering to an active cube face /mip level does not work).

@mrdoob
Copy link
Owner

mrdoob commented Jul 12, 2023

It would be good to provide a live example that demonstrate what use case currently breaks (so we can see that rendering to an active cube face /mip level does not work).

Agreed. This PR would need to include an example of usage in order to get merged.

@makovkins Would you be able to add one?

@mrdoob mrdoob added this to the r155 milestone Jul 12, 2023
@mrdoob mrdoob changed the title Ability to set the mip levels of a texture as a render target WebGLRenderer: Fix setRenderTarget()'s activeMipmapLevel Jul 12, 2023
@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 12, 2023

The example can be similar to webgl_materials_cubemap_mipmaps which was added by #17072. Meaning it does not have to be fancy looking. A functional example which demonstrates the feature is totally fine for the beginning.

@makovkins
Copy link
Contributor Author

makovkins commented Jul 12, 2023

Ok, I will provide an example as soon as possible

@makovkins
Copy link
Contributor Author

Added an example that demonstrates rendering to mip levels of a cube texture. The example is super simple, it just colorizes the mip levels.

@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 14, 2023

Awesome! I review this PR in the upcoming days.

Copy link
Collaborator

@Mugen87 Mugen87 left a comment

Choose a reason for hiding this comment

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

The PR looks good to me! The example really helped to understand the current limitation of the engine and how the PR resolves it.

const rt = new THREE.WebGLCubeRenderTarget( cubeMapSize, params );

const mipLevels = Math.log( cubeMapSize ) * Math.LOG2E + 1.0;
for ( let i = 0; i < mipLevels; i ++ ) rt.texture.mipmaps.push( {} );
Copy link
Collaborator

@Mugen87 Mugen87 Jul 15, 2023

Choose a reason for hiding this comment

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

@mrdoob Explicitly defining the mipmaps of the cube render target like demonstrated here triggers the new code path.

@Mugen87 Mugen87 merged commit a3c4192 into mrdoob:dev Jul 18, 2023
19 checks passed
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.

None yet

4 participants