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

MeshToonMaterial: Only sample the red channel of gradientMap. #22911

Merged
merged 1 commit into from
Dec 3, 2021

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Nov 29, 2021

Related issue: -

Description

When we want to use texStorage2D() it's important to understand that gl.LUMINANCE can not be used as an internal format. The idea is to use gl.RED instead which is converted to the sized formats gl.R8, gl.R16F or gl.R32F by the renderer depending on the data type.

When using a texture with e.g. gl.R8, only the r channel can be sampled. This PR refactors the toon material such that the shader only samples the red channel and not RGB. In this way, the gradient map does not break if we use THREE.RedFormat with WebGL 2.

I'm not sure why it was implemented differently since gradient maps for toon materials are usually grayscale maps with a (n,1) dimension.

// Materials

const cubeWidth = 400;
const numberOfSphersPerSide = 5;
const sphereRadius = ( cubeWidth / numberOfSphersPerSide ) * 0.8 * 0.5;
const stepSize = 1.0 / numberOfSphersPerSide;
const format = ( renderer.capabilities.isWebGL2 ) ? THREE.RedFormat : THREE.LuminanceFormat;
Copy link
Owner

Choose a reason for hiding this comment

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

Hmm, is there a way to hide this? 🤔

Copy link
Collaborator Author

@Mugen87 Mugen87 Nov 30, 2021

Choose a reason for hiding this comment

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

We use this approach at a different location, too:

const format = ( renderer.capabilities.isWebGL2 ) ? THREE.RedFormat : THREE.LuminanceFormat;

Hiding this detail in the renderer is problematic since THREE.RedFormat behaves differently than THREE.LuminanceFormat. If we would internally convert luminance to red, users could be confused because it would be only possible to sample the r channel.

IMO, we could leave the code as it is or just use THREE.RedFormat in both examples (which would make them WebGL 2 only though).

@mrdoob mrdoob added this to the r136 milestone Nov 29, 2021
@Mugen87
Copy link
Collaborator Author

Mugen87 commented Dec 2, 2021

@mrdoob This PR is a blocker for using gl.texStorage2D() with data textures. It would be great to merge it so I can move on with my changes.

I'm happy to refactor the code in webgl_materials_variations_toon and webaudio_visualizer at a later point.

@Mugen87 Mugen87 merged commit 3a92e59 into mrdoob:dev Dec 3, 2021
@mrdoob
Copy link
Owner

mrdoob commented Dec 3, 2021

Sounds good 👍
Thanks!

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