Page MenuHome
Differential D13216

Fix T92631: Fix regression with negative thickness in complex solidify
ClosedPublic
Actions

Authored by Henrik Dick (weasel) on Nov 14 2021, 11:11 PM.

Details

Reviewers
Campbell Barton (campbellbarton)
Hans Goudey (HooglyBoogly)
Group Reviewers
Modifiers
Maniphest Tasks
T92631: Solidify modifier with Complex mode and negative thickness not pointing normals outside
Commits
rB819b9bdfa156: Fix T92631: Fix negative thickness regression in complex solidify
Summary

Fix T92631. This regression was introduced by D11832, but there was problems before that as well. I seem to have missed it in review.
Here is a picture which shows the state in 2.93 and with this patch:


The current state has more problems shown here:

This is the testfile:
solidifynegativethickness.blend895 KBDownload

Diff Detail

Repository
rB Blender

Event Timeline

Henrik Dick (weasel) requested review of this revision.Nov 14 2021, 11:11 PM
Henrik Dick (weasel) created this revision.
Phil Stopford (philstopford) added a subscriber: Phil Stopford (philstopford).EditedNov 15 2021, 3:36 AM

If this was missed before, would it make sense to have some comments to explain what's going on? It might otherwise not be obvious to a future contributor

Brecht Van Lommel (brecht) awarded a token.Nov 16 2021, 6:11 PM
Henrik Dick (weasel) updated this revision to Diff 44835.Nov 16 2021, 8:54 PM
  • added comments to the important variables
Phil Stopford (philstopford) awarded a token.Nov 17 2021, 1:54 AM
Germano Cavalcante (mano-wii) added a subscriber: Germano Cavalcante (mano-wii).EditedNov 17 2021, 3:48 PM

I recommend following the style guide for comments. (especially the symbol section).
https://wiki.blender.org/wiki/Style_Guide/C_Cpp#Comments

Henrik Dick (weasel) updated this revision to Diff 44945.Nov 19 2021, 12:13 AM
  • improved comment style
Henrik Dick (weasel) added a comment.Nov 20 2021, 5:04 PM

AFAIK this can be committed now (I still don't have commit rights). This should go into 3.0

Phil Stopford (philstopford) added a comment.Nov 20 2021, 10:29 PM

@Campbell Barton (campbellbarton) : this all looks good to me. Do you think this could go in for 3.0?

Paul Kotelevets (1D_Inc) awarded a token.Nov 22 2021, 3:20 PM
Raimund Klink (Raimund58) awarded a token.Nov 22 2021, 3:24 PM
Hans Goudey (HooglyBoogly) accepted this revision.Nov 22 2021, 3:24 PM
Hans Goudey (HooglyBoogly) added a subscriber: Hans Goudey (HooglyBoogly).

The fix seems simple enough, and well tested, I will commit this.

I still think this file should become a regression test though. @Henrik Dick (weasel), would you be willing to look into that?

This revision is now accepted and ready to land.Nov 22 2021, 3:24 PM
Closed by commit rB819b9bdfa156: Fix T92631: Fix negative thickness regression in complex solidify (authored by Henrik Dick (weasel), committed by Hans Goudey (HooglyBoogly)). · Explain WhyNov 22 2021, 3:34 PM
This revision was automatically updated to reflect the committed changes.
Hans Goudey (HooglyBoogly) added a commit: rB819b9bdfa156: Fix T92631: Fix negative thickness regression in complex solidify.

Revision Contents
Changeset List

PathSize
source/
blender/
modifiers/
intern/
14 lines

Diff 45069

View Options

source/blender/modifiers/intern/MOD_solidify_nonmanifold.c

Loading...