Fix T88111: Ensure face normals are correctly updated with the skin modifies internal calculations
ClosedPublic
Actions

Authored by Campbell Barton (campbellbarton) on May 26 2021, 8:06 AM.

Details

Reviewers

Bastien Montagne (mont29)

Maniphest Tasks

T88111: Skin Modifier asserts on invalid normals

Summary

The skin modifier was moving vertices without updating normals for the connected faces,
this happened when smoothing and welding vertices.

This asserted in debug builds, giving incorrect results in release builds (since calculations are performed which involved the face normals).

Note I think this is safe put for 2.93.

Diff Detail

Repository

rB Blender

Branch

TEMP-SKIN-MOD-FIX-T88111 (branched from master)

Build Status

Buildable 14774
Build 14774: arc lint + arc unit

Event Timeline

Campbell Barton (campbellbarton) requested review of this revision.May 26 2021, 8:06 AM

Campbell Barton (campbellbarton) created this revision.

Harbormaster completed remote builds in B14773: Diff 37496.May 26 2021, 8:06 AM

Campbell Barton (campbellbarton) retitled this revision from Reviewers: to Fix T88111: Ensure face normals are correctly updated with the skin modifies internal calculations.May 26 2021, 8:11 AM

Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)

Campbell Barton (campbellbarton) added a reviewer: Bastien Montagne (mont29).

Campbell Barton (campbellbarton) added a project: Modeling.

Campbell Barton (campbellbarton) added inline comments.May 26 2021, 8:14 AM

source/blender/modifiers/intern/MOD_skin.c
1398	This could reuse `vert_buf`, I don't have a strong opinion either way.

Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)May 26 2021, 8:15 AM

Campbell Barton (campbellbarton) added a task: T88111: Skin Modifier asserts on invalid normals.

Remove alloca usasge

Harbormaster completed remote builds in B14774: Diff 37497.May 26 2021, 8:22 AM

patch LGTM, thanks.

Some minor comments below, not blocking though.

Not sure re 2.93 however, in bcon4 our policy is fairly strict afaik, only critical fixes and/or regressions are expected? This one looks fairly safe, but personally I would rather wait for 2.93.1 (so that it gets a bit more testing in master first), since 2.93 is LTS?

source/blender/modifiers/intern/MOD_skin.c
1395	Well, not done with it if face is not a quad ;)
1396	Not sure why the assert here if the case is dealt with anyway? Maybe better to get a CLOG error instead then?