Fixup normals in add_mesh_solid generated meshes. #105117

gds · 2024-01-13T09:07:02+01:00

gds commented

2024-01-13 09:07:02 +01:00

First-time contributor

Certain of the presets for add_mesh_solid have faces where their normals are inverted. This commit takes each mesh that's added and invokes normals_make_consistent on it to fix the normals.

The presets that had bad normals were the Rhombicuboctahedron, Truncated Cuboctahedron, Snub Cube, Rhombicosidodecahedron, and Snub Dodecahedron.

With this change, these solids are fixed as will be any new ones that happen to be added with faces ordered the wrong way.

https://imgur.com/a/YLhkPhT shows the Rhombicuboctahedron with an embedded light emitting cube before the fix, and https://imgur.com/a/S9pU41I shows it after.

Note that this issue was raised on Blender StackExchange as https://blender.stackexchange.com/questions/309376/refraction-bsdf-shader-showing-as-opaque-for-certain-meshes-and-angles, and the responder pointed out the problem with the normals. This fixes that.

Certain of the presets for add_mesh_solid have faces where their normals are inverted. This commit takes each mesh that's added and invokes `normals_make_consistent` on it to fix the normals. The presets that had bad normals were the Rhombicuboctahedron, Truncated Cuboctahedron, Snub Cube, Rhombicosidodecahedron, and Snub Dodecahedron. With this change, these solids are fixed as will be any new ones that happen to be added with faces ordered the wrong way. https://imgur.com/a/YLhkPhT shows the Rhombicuboctahedron with an embedded light emitting cube before the fix, and https://imgur.com/a/S9pU41I shows it after. Note that this issue was raised on Blender StackExchange as https://blender.stackexchange.com/questions/309376/refraction-bsdf-shader-showing-as-opaque-for-certain-meshes-and-angles, and the responder pointed out the problem with the normals. This fixes that.

normals-fix.blend

976 KiB

gds added 1 commit 2024-01-13 09:07:04 +01:00

Fixup normals in add_mesh_solid generated meshes. a063069270

Certain of the presets for add_mesh_solid have faces where their normals are inverted.  This commit takes each mesh that's added and invokes `normals_make_consistent` on it to fix the normals.

The presets that had bad normals were the Rhombicuboctahedron, Truncated Cuboctahedron, Snub Cube, Rhombicosidodecahedron, and Snub Dodecahedron.  

With this change, these solids are fixed as will be any new ones that happen to be added with faces ordered the wrong way.

Thomas Barlow commented

2024-01-13 21:50:12 +01:00

I think it would be better to try and find the cause of the problem in the existing code and fix that, than to add a bunch of code to 'fix' things afterwards.

Damien Picard commented

2024-01-14 00:28:54 +01:00

I had a look at this. When vertex truncation is > 1.0, the truncated polyhedron is computed using a mirrored (scaled -1.0) dual of the base polyhedron. Duals themselves are mirrored as well.
I think that’s a good strategy, and computing inverted normals during mesh generation seems much more complex than making them consistent after the fact. However I also think this can be made simpler than the proposed solution, by using bmesh:

diff --git a/add_mesh_extra_objects/add_mesh_solid.py b/add_mesh_extra_objects/add_mesh_solid.py
index ef9cc541d..3469e42f6 100644
--- a/add_mesh_extra_objects/add_mesh_solid.py
+++ b/add_mesh_extra_objects/add_mesh_solid.py
@@ -504,10 +504,18 @@ class Solids(bpy.types.Operator):
         mesh = bpy.data.meshes.new("Solid")
 
         # Make a mesh from a list of verts/edges/faces.
-        mesh.from_pydata(verts, [], faces)
+        import bmesh
+        bm = bmesh.new()
+
+        for v in verts:
+            bm.verts.new(v)
+        bm.verts.ensure_lookup_table()
+        for f in faces:
+            bm.faces.new([bm.verts[v] for v in f])
+        bmesh.ops.recalc_face_normals(bm, faces=bm.faces)
 
         # Update mesh geometry after adding stuff.
-        mesh.update()
+        bm.to_mesh(mesh)
 
         object_data_add(context, mesh, operator=None)
         # object generation done

Also noticed that the faces are converted to quads, probably because the add-on was written before the introduction of ngons with bmesh, so this conversion should be removed in my opinion.

I had a look at this. When vertex truncation is > 1.0, the truncated polyhedron is computed using a mirrored (scaled -1.0) dual of the base polyhedron. Duals themselves are mirrored as well. I think that’s a good strategy, and computing inverted normals during mesh generation seems much more complex than making them consistent after the fact. However I also think this can be made simpler than the proposed solution, by using bmesh: ```diff diff --git a/add_mesh_extra_objects/add_mesh_solid.py b/add_mesh_extra_objects/add_mesh_solid.py index ef9cc541d..3469e42f6 100644 --- a/add_mesh_extra_objects/add_mesh_solid.py +++ b/add_mesh_extra_objects/add_mesh_solid.py @@ -504,10 +504,18 @@ class Solids(bpy.types.Operator): mesh = bpy.data.meshes.new("Solid") # Make a mesh from a list of verts/edges/faces. - mesh.from_pydata(verts, [], faces) + import bmesh + bm = bmesh.new() + + for v in verts: + bm.verts.new(v) + bm.verts.ensure_lookup_table() + for f in faces: + bm.faces.new([bm.verts[v] for v in f]) + bmesh.ops.recalc_face_normals(bm, faces=bm.faces) # Update mesh geometry after adding stuff. - mesh.update() + bm.to_mesh(mesh) object_data_add(context, mesh, operator=None) # object generation done ``` Also noticed that the faces are converted to quads, probably because the add-on was written before the introduction of ngons with bmesh, so this conversion should be removed in my opinion.

Thomas Barlow commented

2024-01-14 03:29:26 +01:00

Flipping polygon normals can be done by reversing the order of the vertices of each face, I've done this for each of the scaled * -1 cases in my fork: 93d3b64772

It seems to work and I would prefer this to recalculating normals afterwards, but I'll check more tomorrow because I'm exhausted right now.

Flipping polygon normals can be done by reversing the order of the vertices of each face, I've done this for each of the scaled * -1 cases in my fork: https://projects.blender.org/Mysteryem/blender-addons/commit/93d3b64772396f32b78d60536105f5bff7a0512f It seems to work and I would prefer this to recalculating normals afterwards, but I'll check more tomorrow because I'm exhausted right now.

Damien Picard commented

2024-01-14 12:13:11 +01:00

It seems to work and I would prefer this to recalculating normals afterwards,

You’re right, it’s not as hard as I thought it would be and your fix is better.

I found another issue, which is that the dual tetrahedron gets flipped when it has a vertex truncation of 2.0. I think it needs to be mirrored as well:

diff --git a/add_mesh_extra_objects/add_mesh_solid.py b/add_mesh_extra_objects/add_mesh_solid.py
index 1f478a323..ccf2101cc 100644
--- a/add_mesh_extra_objects/add_mesh_solid.py
+++ b/add_mesh_extra_objects/add_mesh_solid.py
@@ -164,8 +164,8 @@ def createSolid(plato, vtrunc, etrunc, dual, snub):
         if vtrunc == 0:    # no truncation needed
             if dual:
                 vInput, fInput = source(plato)
-                vInput = [i * supposedSize for i in vInput]
-                return vInput, fInput
             vInput = [-i * supposedSize for i in vInput]
             # Inverting vInput turns the mesh inside-out, so normals need to be flipped.
             return vInput, flipped_face_normals(fInput)

but I'll check more tomorrow because I'm exhausted right now.

No hurry for me :)

> It seems to work and I would prefer this to recalculating normals afterwards, You’re right, it’s not as hard as I thought it would be and your fix is better. I found another issue, which is that the dual tetrahedron gets flipped when it has a vertex truncation of 2.0. I think it needs to be mirrored as well: ```diff diff --git a/add_mesh_extra_objects/add_mesh_solid.py b/add_mesh_extra_objects/add_mesh_solid.py index 1f478a323..ccf2101cc 100644 --- a/add_mesh_extra_objects/add_mesh_solid.py +++ b/add_mesh_extra_objects/add_mesh_solid.py @@ -164,8 +164,8 @@ def createSolid(plato, vtrunc, etrunc, dual, snub): if vtrunc == 0: # no truncation needed if dual: vInput, fInput = source(plato) - vInput = [i * supposedSize for i in vInput] - return vInput, fInput vInput = [-i * supposedSize for i in vInput] # Inverting vInput turns the mesh inside-out, so normals need to be flipped. return vInput, flipped_face_normals(fInput) ``` > but I'll check more tomorrow because I'm exhausted right now. No hurry for me :)

gds commented

2024-01-14 22:30:25 +01:00

First-time contributor

Thanks @Mysteryem and @pioverfour for taking a look!

I didn't know about bmesh, and that's definitely a better and cleaner approach than what I was doing with updating and restoring the current selection.

I'm less certain about fixing these normals at the source of their problem. The extra issue that @pioverfour found with the dual tetrahedron is a good example of why it may be a whack-a-mole problem to get all these right, vs. the guaranteed fixup at the end.

Also, the problem is also hidden by the fact that many uses (maybe most uses) don't subject these solids to shaders that care about the sign of the normal.

I'm a Blender pull-request n00b, so don't know the expectation from me at this point. I'm happy to update this pull request to the bmesh route and feel confident in it. This would be my preferred solution due to the whack-a-mole nature I mention, and the fact that the createSolid method is already super complicated with many if-else branches, and adding to those just makes it more complex.

Suggestions?

Thanks @Mysteryem and @pioverfour for taking a look! I didn't know about `bmesh`, and that's _definitely_ a better and cleaner approach than what I was doing with updating and restoring the current selection. I'm less certain about fixing these normals at the source of their problem. The extra issue that @pioverfour found with the dual tetrahedron is a good example of why it may be a whack-a-mole problem to get all these right, vs. the guaranteed fixup at the end. Also, the problem is also hidden by the fact that many uses (maybe most uses) don't subject these solids to shaders that care about the sign of the normal. I'm a Blender pull-request n00b, so don't know the expectation from me at this point. I'm happy to update this pull request to the `bmesh` route and feel confident in it. This would be my preferred solution due to the whack-a-mole nature I mention, and the fact that the createSolid method is already super complicated with many if-else branches, and adding to those just makes it more complex. Suggestions?

Damien Picard commented

2024-01-15 00:02:06 +01:00

I didn't know about bmesh, and that's definitely a better and cleaner approach than what I was doing with updating and restoring the current selection.

Great that you learned something ;)

The extra issue that @pioverfour found with the dual tetrahedron is a good example of why it may be a whack-a-mole problem

Well, I do think @Mysteryem has a point and their fix is solid. The issue I noticed is not really related to the normals, and should be investigated anyway.

I'm happy to update this pull request to the bmesh route and feel confident in it. This would be my preferred solution due to the whack-a-mole nature I mention, and the fact that the createSolid method is already super complicated with many if-else branches, and adding to those just makes it more complex.

IMO Mysteryem’s solution doesn’t add that much more complexity, and there aren’t really that many combinations to test. This script should be comprehensive enough to check the normals:

import bpy

x = 0
for vTrunc in (0.0, 0.5, 1.0, 1.5, 2.0):
    for eTrunc in (0, 0.5, 1.0):
        for snub in ("None", "Left", "Right"):
            for dual in (True, False):
                for y, source in enumerate(("4", "6", "8", "12", "20")):
                    bpy.ops.mesh.primitive_solid_add(
                        source=source, vTrunc=vTrunc, eTrunc=eTrunc,
                        snub=snub, dual=dual, size=0.5
                    )
                    bpy.context.active_object.location = (x, y, 0)
                    bpy.context.active_object.name = f"{source} v={vTrunc} e={eTrunc} snub={snub} dual={dual}"
                x += 1

Before:

After:

> I didn't know about `bmesh`, and that's _definitely_ a better and cleaner approach than what I was doing with updating and restoring the current selection. Great that you learned something ;) > The extra issue that @pioverfour found with the dual tetrahedron is a good example of why it may be a whack-a-mole problem Well, I do think @Mysteryem has a point and their fix is solid. The issue I noticed is not really related to the normals, and should be investigated anyway. > I'm happy to update this pull request to the `bmesh` route and feel confident in it. This would be my preferred solution due to the whack-a-mole nature I mention, and the fact that the createSolid method is already super complicated with many if-else branches, and adding to those just makes it more complex. IMO Mysteryem’s solution doesn’t add that much more complexity, and there aren’t really that many combinations to test. This script should be comprehensive enough to check the normals: ```python import bpy x = 0 for vTrunc in (0.0, 0.5, 1.0, 1.5, 2.0): for eTrunc in (0, 0.5, 1.0): for snub in ("None", "Left", "Right"): for dual in (True, False): for y, source in enumerate(("4", "6", "8", "12", "20")): bpy.ops.mesh.primitive_solid_add( source=source, vTrunc=vTrunc, eTrunc=eTrunc, snub=snub, dual=dual, size=0.5 ) bpy.context.active_object.location = (x, y, 0) bpy.context.active_object.name = f"{source} v={vTrunc} e={eTrunc} snub={snub} dual={dual}" x += 1 ``` Before: ![before.png](/attachments/630a546a-f572-4d51-9355-e4c4c5eefdfc) After: ![after.png](/attachments/3ead933e-f27d-45e9-9fa7-d28201f401be)

before.png

253 KiB

after.png

252 KiB

Thomas Barlow referenced this pull request

2024-01-15 03:14:12 +01:00

Fix: Add Mesh Extra Objects: Inverted normals of solids when vertex positions are inverted #105123

Thomas Barlow commented

2024-01-15 04:33:58 +01:00

I made a PR for my changes (#105123), it doesn't touch the issue of the dual tetrahedron being flipped, it's only for the normals issue.

I made a PR for my changes (https://projects.blender.org/blender/blender-addons/pulls/105123), it doesn't touch the issue of the dual tetrahedron being flipped, it's only for the normals issue.

gds commented

2024-01-15 06:28:54 +01:00

First-time contributor

Abandoning this PR in favor of !105123. Thanks @Mysteryem!

👍 1

gds closed this pull request

2024-01-15 06:28:55 +01:00

Damien Picard referenced this pull request

2024-01-15 11:01:18 +01:00

Fix: Add Mesh Extra Objects: dual tetrahedron is sometimes flipped #105127

Damien Picard referenced this issue from a commit

2024-01-16 22:04:18 +01:00