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

Toon shading add antialiasing #24406

Merged
merged 4 commits into from
Aug 1, 2022
Merged

Toon shading add antialiasing #24406

merged 4 commits into from
Aug 1, 2022

Conversation

RenaudRohlinger
Copy link
Collaborator

@RenaudRohlinger RenaudRohlinger commented Jul 30, 2022

Adds antialiasing to the getGradientIrradiance function.

Before:
Screen Shot 2022-07-30 at 16 14 31
.
After:
Screen Shot 2022-07-30 at 16 14 12
Edit: Added screenshot example.

@gkjohnson
Copy link
Collaborator

Do you have any before / after pictures?

@RenaudRohlinger
Copy link
Collaborator Author

RenaudRohlinger commented Jul 30, 2022

@gkjohnson Updated the post with a screenshot of our game before/after.

The example provided is using the exact same logic:

// before
vec3 res = ( coord.x <  0.7 ) ? backColor :  vec3( 1.0 );
// after
vec3 res = mix( backColor, vec3( 1.0 ), smoothstep( 0.7 - fw.x, 0.7 + fw.x, coord.x));

@@ -18,7 +18,8 @@ vec3 getGradientIrradiance( vec3 normal, vec3 lightDirection ) {

#else

return ( coord.x < 0.7 ) ? vec3( 0.7 ) : vec3( 1.0 );
vec2 fw = fwidth( coord );
return mix( vec3( 0.7 ), vec3( 1. ), smoothstep( 0.7 - fw.x, 0.7 + fw.x, coord.x));
Copy link
Collaborator

@gkjohnson gkjohnson Jul 30, 2022

Choose a reason for hiding this comment

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

Have you considered halving the fw when using it as a step in the smoothstep function? As it is the step will happen over two pixels which will result in a less crisp edge than it could be:

return mix( vec3( 0.7 ), vec3( 1.0 ), smoothstep( 0.7 - fw.x * 0.5, 0.7 + fw.x * 0.5, coord.x ) );

Also nit: can we use 1.0 instead of 1. for consistency and make sure we have consistent spaces inside the parens.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed! Updated the PR with the changes.

@Mugen87 Mugen87 added this to the r144 milestone Jul 30, 2022
Copy link
Collaborator

@gkjohnson gkjohnson left a comment

Choose a reason for hiding this comment

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

Code looks good to me! I trust @RenaudRohlinger has tested it.

@mrdoob mrdoob merged commit ecf065f into mrdoob:dev Aug 1, 2022
@mrdoob
Copy link
Owner

mrdoob commented Aug 1, 2022

Thanks!

abernier pushed a commit to abernier/three.js that referenced this pull request Sep 16, 2022
* Toon shading add antialiasing

* Update gradientmap_pars_fragment.glsl.js

* Update gradientmap_pars_fragment.glsl.js

* Update gradientmap_pars_fragment.glsl.js

Co-authored-by: Michael Herzog <[email protected]>
snagy pushed a commit to snagy/three.js-1 that referenced this pull request Sep 21, 2022
* Toon shading add antialiasing

* Update gradientmap_pars_fragment.glsl.js

* Update gradientmap_pars_fragment.glsl.js

* Update gradientmap_pars_fragment.glsl.js

Co-authored-by: Michael Herzog <[email protected]>
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.

4 participants