Flip Mesh Faces Node #93569

New Issue

Hans Goudey · 2021-12-02T14:29:07+01:00

Hans Goudey commented

2021-12-02 14:29:07 +01:00

Currently there is no way to flip normals in geometry nodes. This node would make that possible
by flipping the winding order of selected faces. The node is purposely not called "Flip Normals",
because normals are derived data, changing them is only a side effect.
The real change is that the face corners of every polygon are reversed.

{F12679512 size=full}

Details

The node should have a mesh input and output, as well as a selection input (evaluated on the face domain). Only face corners attributes and indices will be affected by the node.

Implementation

The input mesh should be mutable ("for_write"), since most of the data doesn't need to be changed.
The code should work on each corner attribute separately for better performance.
Making use of MutableSpan::slice and MutableSpan::reverse should be helpful.
A face's MLoop can be simply reversed, but the edge indices will have to be updated.

Further Questions

N/A

Currently there is no way to flip normals in geometry nodes. This node would make that possible by flipping the winding order of selected faces. The node is purposely not called "Flip Normals", because normals are derived data, changing them is only a **side effect**. The real change is that the face corners of every polygon are reversed. {[F12679512](https://archive.blender.org/developer/F12679512/image.png) size=full} ### Details The node should have a mesh input and output, as well as a selection input (evaluated on the face domain). Only face corners attributes and indices will be affected by the node. ### Implementation - The input mesh should be mutable ("for_write"), since most of the data doesn't need to be changed. - The code should work on each corner attribute separately for better performance. - Making use of `MutableSpan::slice` and `MutableSpan::reverse` should be helpful. - A face's `MLoop` can be simply reversed, but the edge indices will have to be updated. ### Further Questions N/A

Hans Goudey commented

2021-12-02 14:29:07 +01:00

Changed status from 'Needs Triage' to: 'Confirmed'

Hans Goudey commented

2021-12-02 14:29:07 +01:00

Added subscriber: @HooglyBoogly

Paul Larson commented

2021-12-03 01:32:40 +01:00

Added subscriber: @GeorgiaPacific

Filippo Fracascia commented

2021-12-07 14:28:22 +01:00

Added subscriber: @filippo_f

Filippo Fracascia commented

2021-12-09 17:01:31 +01:00

Hi, I am in the process of implementing this node. I have a pair of problems I stumbled across, and wanted to hear an experienced dev's opinion on this. I hope these to be legit questions.

1.This question is about performance: my approach to this was to iterate trought the selected faces first, and then inside every face reverse the MLoop's order. This algorithm should take O(n), where n is the vertices, since I need to iterate trought every vertex of the MLoop in order to reverse it. Is this where the MutableSpan::reverse should take place? I didn't find any reference to this method inside the source code, but I'm sure this would help making the algorithm more efficient and definitely shorter.

2.In the last point you said that edges indices need to be updated after reverse. My question is, are edges indices relative to the face they belong to? I.e. every time I move a face further will my index start from 0 again? Or are all edges indices linked togheter inside the same mesh?

Edit: I found the BLI_span header file, my bad!! That gave me the answers I needed

Hi, I am in the process of implementing this node. I have a pair of problems I stumbled across, and wanted to hear an experienced dev's opinion on this. I hope these to be legit questions. 1.This question is about performance: my approach to this was to iterate trought the selected faces first, and then inside every face reverse the MLoop's order. This algorithm should take O(n), where n is the vertices, since I need to iterate trought every vertex of the MLoop in order to reverse it. Is this where the MutableSpan::reverse should take place? I didn't find any reference to this method inside the source code, but I'm sure this would help making the algorithm more efficient and definitely shorter. 2.In the last point you said that edges indices need to be updated after reverse. My question is, are edges indices relative to the face they belong to? I.e. every time I move a face further will my index start from 0 again? Or are all edges indices linked togheter inside the same mesh? Edit: I found the BLI_span header file, my bad!! That gave me the answers I needed

Hans Goudey commented

2021-12-10 18:04:29 +01:00

Hey, great to hear you're working on this!

The FieldEvaluator offers a method called get_evaluated_as_mask. This gives you only the selected indices, so you don't need to iterate over every single face. That's helpful when only a few faces are selected.
The edge indices are into the mesh's entire MEdge array. Each MLoop has an index of a vertex, and then the index of the subsequent edge. When an edge's vertex order is swapped, which edge is the "subsequent" edge also changes.

Hey, great to hear you're working on this! 1. The `FieldEvaluator` offers a method called `get_evaluated_as_mask`. This gives you only the selected indices, so you don't need to iterate over every single face. That's helpful when only a few faces are selected. 2. The edge indices are into the mesh's entire `MEdge` array. Each `MLoop` has an index of a vertex, and then the index of the subsequent edge. When an edge's vertex order is swapped, which edge is the "subsequent" edge also changes.

Filippo Fracascia commented

2021-12-14 15:33:47 +01:00

Thanks! That was clarifying. I still don't comprehend the edges part. I mean, I tried everything after reversing the loop. I tried swapping the edges indices, but the mesh returns to normal after I apply the modifier, like nothing happened. Maybe I'm missing something at the end of the method? Like updating normals or such? It looks like it's working when the modifier is active but when applied its effects disappear and the normals return like before.

Also if I may ask, what's the difference between these two ways of reversing a face's loops? Are both of them valid?
a.

MPoly *poly = &mesh->mpoly[poly_index];
MutableSpan loop_span = loops.slice(poly->loopstart, poly->totloop);
loop_span.reverse();

}```

b.

for(const int poly_index : selection){
MPoly *poly = &mesh->mpoly[poly_index];
BKE_mesh_polygon_flip(poly, mesh->mloop, &mesh->ldata);
}


Sorry for asking so many thing but I feel like I am so close and it would be a waste to give up now.

Thanks! That was clarifying. I still don't comprehend the edges part. I mean, I tried everything after reversing the loop. I tried swapping the edges indices, but the mesh returns to normal after I apply the modifier, like nothing happened. Maybe I'm missing something at the end of the method? Like updating normals or such? It looks like it's working when the modifier is active but when applied its effects disappear and the normals return like before. Also if I may ask, what's the difference between these two ways of reversing a face's loops? Are both of them valid? a. ```for(const int poly_index : selection){ ``` MPoly *poly = &mesh->mpoly[poly_index]; MutableSpan<MLoop> loop_span = loops.slice(poly->loopstart, poly->totloop); loop_span.reverse(); ``` }``` b. ``` for(const int poly_index : selection){ MPoly *poly = &mesh->mpoly[poly_index]; BKE_mesh_polygon_flip(poly, mesh->mloop, &mesh->ldata); } ``` Sorry for asking so many thing but I feel like I am so close and it would be a waste to give up now.

Hans Goudey commented

2021-12-14 15:55:03 +01:00

Maybe these drawings will help:

{F12739505 size=full}	{F12739507 size=full}
Here the normal of the first face is pointing up, and the normal of the second face points into the screen (https://en.wikipedia.org/wiki/Right-hand_rule#Curve_orientation_and_normal_vectors).
Notice that the vertex indices stay the same, but the edge indices are changed, so it's not correct to simply reverse `MLoop`. (It should work for face corner domain attributes though).

There's also a performance aspect.
The problem with BKE_mesh_polygon_flip is that it works on every every single attribute at the same time, see CustomData_swap. This is needlessly slow, mostly because it doesn't play well with caching on CPUs.
The difference is with the loops:
A:

for each face index:
    for each attribute:

B:

for each attribute:
    for each face index:

B should play much better with caching, especially since in this case the operation is quite simple, so the algorithm will hopefully be memory-bound.

Maybe these drawings will help: | {[F12739505](https://archive.blender.org/developer/F12739505/image.png) size=full} | {[F12739507](https://archive.blender.org/developer/F12739507/image.png) size=full} | | -- | -- | Here the normal of the first face is pointing up, and the normal of the second face points into the screen (https://en.wikipedia.org/wiki/Right-hand_rule#Curve_orientation_and_normal_vectors). Notice that the vertex indices stay the same, but the edge indices are changed, so it's not correct to simply reverse `MLoop`. (It should work for face corner domain attributes though). --- There's also a performance aspect. The problem with `BKE_mesh_polygon_flip` is that it works on every every single attribute at the same time, see `CustomData_swap`. This is needlessly slow, mostly because it doesn't play well with caching on CPUs. The difference is with the loops: A: ``` for each face index: for each attribute: ``` B: ``` for each attribute: for each face index: ``` B should play much better with caching, especially since in this case the operation is quite simple, so the algorithm will hopefully be memory-bound.

Filippo Fracascia commented

2021-12-14 16:12:14 +01:00

Enlightening, thanks!!