Triangulate modifier (also Triangulate Faces operator) changed behavior with custom normals #124836

New Issue

Servando · 2024-07-17T05:11:43+02:00

Servando commented

2024-07-17 05:11:43 +02:00

System Information
Operating system: Windows-10-10.0.19045-SP0 64 Bits
Graphics card: NVIDIA GeForce GTX 1080/PCIe/SSE2 NVIDIA Corporation 4.6.0 NVIDIA 551.86

Blender Version
Broken: version: 4.2.0, branch: blender-v4.2-release, commit date: 2024-07-16 06:20, hash: a51f293548ad
Worked: blender 4.1 and before

Caused by f3c32a36bc

Short description of error
Triangulate modifier (also Triangulate Faces operator) changed behavior with custom normals

Exact steps for others to reproduce the error

open 124836.blend in 4.1
enable the Triangulate modifier
observe the custom normals are unchanged
open 124836.blend in 4.2
enable the Triangulate modifier
observe the custom normals are now changed

This also affects the Triangulate Faces operator:

open 124836.blend in 4.1
remove the Triangulate modifier
apply the WeightedNormal modifier
hit Shift + Ctrl + T
observe the custom normals are unchanged
now open 124836.blend in 4.2
remove the Triangulate modifier
apply the WeightedNormal modifier
hit Shift + Ctrl + T
observe the custom normals are now changed

**System Information** Operating system: Windows-10-10.0.19045-SP0 64 Bits Graphics card: NVIDIA GeForce GTX 1080/PCIe/SSE2 NVIDIA Corporation 4.6.0 NVIDIA 551.86 **Blender Version** Broken: version: 4.2.0, branch: blender-v4.2-release, commit date: 2024-07-16 06:20, hash: `a51f293548ad` Worked: blender 4.1 and before Caused by f3c32a36bc **Short description of error** Triangulate modifier (also Triangulate Faces operator) changed behavior with custom normals **Exact steps for others to reproduce the error** - open `124836.blend` in 4.1 - enable the `Triangulate` modifier - observe the custom normals are unchanged - open `124836.blend` in 4.2 - enable the `Triangulate` modifier - observe the custom normals are now changed This also affects the `Triangulate Faces` operator: - open `124836.blend` in 4.1 - remove the `Triangulate` modifier - apply the `WeightedNormal` modifier - hit Shift + Ctrl + T - observe the custom normals are unchanged - now open `124836.blend` in 4.2 - remove the `Triangulate` modifier - apply the `WeightedNormal` modifier - hit Shift + Ctrl + T - observe the custom normals are now changed

124836.blend

913 KiB

Servando added the

labels 2024-07-17 05:11:43 +02:00

Jesse Yurkovich commented

2024-07-17 05:16:14 +02:00

The option was removed because it is always "true" now - custom normals will always be preserved during triangulation. Are you seeing otherwise?

Jesse Yurkovich added

Status

Needs Information from User

and removed

Status

Needs Triage

labels 2024-07-17 05:16:23 +02:00

Servando commented

2024-07-17 05:22:26 +02:00

Yes its giving me different result between both blender versions.

Yes its giving me different result between both blender versions. ![image](/attachments/c589ce6d-6a80-4f16-a9fe-90c0120d8c23)

image.png

2.6 MiB

Test_Triangulate_4.2.blend

866 KiB

Jesse Yurkovich commented

2024-07-17 05:39:57 +02:00

I can confirm the difference with the repro file, thanks for that. It's odd because the 4.2 result doesn't match the 4.1 with Keep Normals OFF either. It's doing something, and getting close to 4.1 with Keep normals ON, but the shading is different.

👍 1

Jesse Yurkovich added

and removed

Needs Information from User

labels 2024-07-17 05:40:09 +02:00

Jesse Yurkovich commented

2024-07-17 05:41:51 +02:00

@lichtwerk Did you encounter anything like this while making the changes in f3c32a36bc ?

@lichtwerk Did you encounter anything like this while making the changes in f3c32a36bcfcad4fb948fb1eb205a2790967b1de ?

Philipp Oeser commented

2024-07-17 09:29:03 +02:00

@lichtwerk Did you encounter anything like this while making the changes in f3c32a36bc ?

I did not notice this in the tests I made back then, but will check...
(if is a different method compared to the previous, so if this regressed in quality, we might have to bring back the old method -- or improve upon the one used now...)

> @lichtwerk Did you encounter anything like this while making the changes in f3c32a36bcfcad4fb948fb1eb205a2790967b1de ? I did not notice this in the tests I made back then, but will check... (if is a different method compared to the previous, so if this regressed in quality, we might have to bring back the old method -- or improve upon the one used now...)

Nika Kutsniashvili commented

2024-07-17 10:14:22 +02:00

This is a duplicate (cant find other issue now)

Philipp Oeser commented

2024-07-17 11:01:01 +02:00

Hm, apparently the Triangulate Faces operator in Fixed mode (Shift Ctrl T) also regressed?

Will check further...

Hm, apparently the `Triangulate Faces` operator in `Fixed` mode (Shift Ctrl T) also regressed? Will check further...

Philipp Oeser commented

2024-07-17 11:18:12 +02:00

Will raise priority, since yeah, this is a regression

Philipp Oeser added

and removed

labels 2024-07-17 11:18:24 +02:00

Philipp Oeser changed title from ~~Keep Normals option is missing from Triangulate modifier~~ to Triangulate modifier (also Triangulate Faces operator) changed behavior

2024-07-17 11:19:18 +02:00

Philipp Oeser commented

2024-07-17 13:41:06 +02:00

In case of the operator:

BMO_op_exec ( "triangulate faces" in our case ) calls bmesh_edit_end (which will update normals again with BM_mesh_normals_update), if we call BM_custom_loop_normals_from_vector_layer afterwards, all is fine
so best to put another call of BM_custom_loop_normals_from_vector_layer in edbm_quads_convert_to_tris_exec after BMO_op_exec?

diff --git a/source/blender/editors/mesh/editmesh_tools.cc b/source/blender/editors/mesh/editmesh_tools.cc
index 415a5051cfd..186340f9746 100644
--- a/source/blender/editors/mesh/editmesh_tools.cc
+++ b/source/blender/editors/mesh/editmesh_tools.cc
@@ -5511,6 +5511,9 @@ static int edbm_quads_convert_to_tris_exec(bContext *C, wmOperator *op)
                  ngon_method);
     BMO_op_exec(em->bm, &bmop);
 
+    /* #BM_mesh_triangulate already does the pair of calls to #BM_custom_loop_normals_to_vector_layer & #BM_custom_loop_normals_from_vector_layer, but BMO_op_exec needs to bmesh_edit_end first. */
+    BM_custom_loop_normals_from_vector_layer(em->bm, false);
+
     /* select the output */
     BMO_slot_buffer_hflag_enable(
         em->bm, bmop.slots_out, "faces.out", BM_FACE, BM_ELEM_SELECT, true);

In case of the modifier:

still staring at this, not sure yet... BKE_mesh_from_bmesh_XXX might not get the custom normals over correctly

@HooglyBoogly might be able to help me out here more quickly
(if this needs to be solved quickly, we might just want to revert f3c32a36bc for now)

In case of the operator: - `BMO_op_exec` ( "triangulate faces" in our case ) calls `bmesh_edit_end` (which will update normals again with `BM_mesh_normals_update`), if we call `BM_custom_loop_normals_from_vector_layer` afterwards, all is fine - so best to put another call of `BM_custom_loop_normals_from_vector_layer` in `edbm_quads_convert_to_tris_exec` after `BMO_op_exec`? ```C diff --git a/source/blender/editors/mesh/editmesh_tools.cc b/source/blender/editors/mesh/editmesh_tools.cc index 415a5051cfd..186340f9746 100644 --- a/source/blender/editors/mesh/editmesh_tools.cc +++ b/source/blender/editors/mesh/editmesh_tools.cc @@ -5511,6 +5511,9 @@ static int edbm_quads_convert_to_tris_exec(bContext *C, wmOperator *op) ngon_method); BMO_op_exec(em->bm, &bmop); + /* #BM_mesh_triangulate already does the pair of calls to #BM_custom_loop_normals_to_vector_layer & #BM_custom_loop_normals_from_vector_layer, but BMO_op_exec needs to bmesh_edit_end first. */ + BM_custom_loop_normals_from_vector_layer(em->bm, false); + /* select the output */ BMO_slot_buffer_hflag_enable( em->bm, bmop.slots_out, "faces.out", BM_FACE, BM_ELEM_SELECT, true); ``` In case of the modifier: - still staring at this, not sure yet... BKE_mesh_from_bmesh_XXX might not get the custom normals over correctly @HooglyBoogly might be able to help me out here more quickly (if this needs to be solved quickly, we might just want to revert f3c32a36bc for now)

Philipp Oeser changed title from ~~Triangulate modifier (also Triangulate Faces operator) changed behavior~~ to Triangulate modifier (also Triangulate Faces operator) changed behavior with custom normals

2024-07-17 16:23:10 +02:00

Philipp Oeser referenced a pull request that will close this issue

2024-07-26 16:13:06 +02:00

Fix #124836: Triangulate modifier changed behavior with custom normals #125499

Philipp Oeser commented

2024-07-26 16:22:04 +02:00

PR to revert is up, see !125499

Philipp Oeser referenced this issue from a commit

2024-07-28 10:27:46 +02:00

Fix #124836: Triangulate modifier changed behavior with custom normals

Philipp Oeser closed this issue

2024-07-28 10:27:48 +02:00