Details

Reviewers

Group Reviewers

Commits

rB68586d2c183b: Complex Solidify: improve constraints solver

Summary

This patch adds a least squares solver to solve the constraints for the "Constraints Thickness Mode". This will give much better results in common cases as shown below.

The square in the center was not a square at all in the old version. This was because the algorithm was only made to be used with 3 faces at a vertex.

There is also a little improvement to boundary handling with the constraints thickness mode.

Diff Detail

Repository: rB Blender

Event Timeline

Henrik Dick (weasel) requested review of this revision.Feb 17 2022, 9:22 PM

Henrik Dick (weasel) created this revision.

I have absolutely no idea what this function is doing, but I was curious and noticed a few things when looking at it, maybe it's helpful.

Unless this is some well known technique that I just don't know about, some more comments describing the approach would certainly be helpful here.

source/blender/modifiers/intern/MOD_solidify_nonmanifold.c
77	The style guide mentions that unsigned integer types shouldn't be used to indicate that a value shouldn't be negative, probably best to follow that here.
78	`float in[3]` -> `const float in[3]`
101–104	Maybe I'm missing something, but I don't see where these arrays are freed

free allocated vectors
add paper name

@Hans Goudey (HooglyBoogly) This is a well known method for solving constraints/linear systems. Take a look at the paper if you want:
"An Introduction to the Conjugate Gradient Method Without the Agonizing Pain" by Jonathan Richard Shewchuk

Henrik Dick (weasel) marked 3 inline comments as done.Feb 18 2022, 10:39 AM

Henrik Dick (weasel) added inline comments.

source/blender/modifiers/intern/MOD_solidify_nonmanifold.c
77	This falls under "follow their policy of integer type usage". In particular `MEM_malloc_arrayN` expects an unsigned type if I see that correctly.

Gavriel Lavi (Gavriel5578) awarded a token.Feb 18 2022, 1:49 PM

refactor of the patch.

Turns out I had transposed the constraints matrix on paper and got the wrong thing. Fixed that. It should now be correct.

Fixed some special cases
removed allocation

Noted some comments, I don't think this needs further review though.

source/blender/modifiers/intern/MOD_solidify_nonmanifold.c
1412	A short explanation of behavior in both cases would be useful (or reference more detailed comment if this is better documented elsewhere).
1466	Noticed while checking this patch `normals_queue` could be renamed to `planes_queue` (since these have a 4th value to represent a plane). Can be changed separately from this patch though.
1561–1566	Typically dot product checks use values near 0.0 or 1.0/-1.0 for aligned or perpendicular vectors. 0.7 seems like an unusual value to be using a different code-paths. This could use a comment explaining why the this value is used.
1595	See related comment `greatest_angle_cos > 0.7f`, this could also use an explanation since it seems arbitrary. Also, if they're related, they could be assigned `const` variables with an explanation for what they do.