Page MenuHome

Modifiers: Subsurf modifier and Multires modifier add option to preserve and interpolate custom normals
Needs ReviewPublic

Authored by Cody Winchester (CodyWinch) on Feb 25 2020, 10:19 PM.
Tokens
"Love" token, awarded by weasel."Love" token, awarded by pablovazquez."100" token, awarded by JayE01."Love" token, awarded by realeyez."Love" token, awarded by lightbwk."Love" token, awarded by wilBr."100" token, awarded by Frozen_Death_Knight."Love" token, awarded by asmitty."Love" token, awarded by mywa880."Love" token, awarded by jeacom256."Like" token, awarded by dillongoo.

Details

Summary

This patch adds the ability to the subsurf modifier and the multires modifier to preserve and interpolate a models custom normals.

  • It checks that the object has custom normals, the option turned on, and auto_smooth checked.
  • If it does it calculates the custom normals on the input mesh and removes the CD_FLAG_TEMPORARY flag form the CD_NORMAL custom data layer.
  • The subsurf calculation then takes care of the interpolation.
  • Then at the end it assigns the custom normals to the result mesh and sets the CD_FLAG_TEMPORARY back onto both meshes.
  • For multires modifier it turns off the custom normals when in sculpt mode.

Diff Detail

Event Timeline

Moved the check for existing custom normals into the use_cnorms bool.

Having it outside was causing a crash if you check preserve custom normals option on a model with no custom normals data.

Yes!! heck Yes!, Its a extremely welcome feature.

Sergey is rewriting Multires modifier in a local branch according to his weekly report . So you may want to submit the changes to each modifier separately?

Sergey is rewriting Multires modifier in a local branch according to his weekly report . So you may want to submit the changes to each modifier separately?

The multires modifier is not being rewritten, and this likely won't conflict with current improvements being done there.

I'm not sure that the multires modifier needs this feature at all, or if it may conflict with future design changes.

For subsurf, custom normals may not work with OpenSubdiv GPU acceleration that is planned to be added, but I guess that acceleration could just be disabled if the option is used.

source/blender/makesdna/DNA_modifier_types.h
154

The name here should include CustomNormals instead of Normals. In general, use the same internal and API names, don't abbreviate arbitrarily.

source/blender/makesrna/intern/rna_modifier.c
1738

Change "Preserve" to "Use".

source/blender/modifiers/intern/MOD_multires.c
243–244

Follow comment style convention:

/* If custom normals are present and the option is turned on calculate the split
 * normals and clear flag so the normals get interpolated to the result mesh. */
259

Why mark vertex normals as dirty here?

Cody Winchester (CodyWinch) marked 4 inline comments as done.Feb 26 2020, 6:47 PM
Cody Winchester (CodyWinch) added inline comments.
source/blender/modifiers/intern/MOD_multires.c
259

I thought it was needed because the triangulate modifier had it after the custom normals were set. It seems it is not needed so I will take it out.

Cody Winchester (CodyWinch) marked an inline comment as done.

Updated to have consistent naming and comment styling.

Also removed the unnecessary setting of dirty_verts.

Here is a demonstration of the difference this patch makes and why it is needed for NPR renderings.

source/blender/modifiers/intern/MOD_multires.c
80

Shouldn't this check for whether cutom normals are enabled?
Same for the Subsurf modifier.

Cody Winchester (CodyWinch) marked an inline comment as done.

Updated so that DependsOnNormals checks that the Custom Normals option is enabled.

source/blender/modifiers/intern/MOD_multires.c
80

Good point. I will make the change.

Updated to match new master.

Jay Edwards (JayE01) rescinded a token.
Jay Edwards (JayE01) awarded a token.

This will be such a life a saver, great work as always Cody !

This patch is much needed for NPR and game development.
Please add it to 2.83. Thanks.

Updated for new master.

@Bastien Montagne (mont29) @Campbell Barton (campbellbarton) @Brecht Van Lommel (brecht) @Sergey Sharybin (sergey)

Any chance of this getting a review soon? Would removing multires and only doing subsurf in this patch help?