Cube UV projection correct aspect option does not work correctly with multiple assigned materials #86358

New Issue

Ludvik Koutny · 2021-03-07T11:29:15+01:00

Ludvik Koutny commented

2021-03-07 11:29:15 +01:00

System Information
Operating system: Windows-10-10.0.19041-SP0 64 Bits
Graphics card: GeForce GTX 1080 Ti/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 461.40

Blender Version
Broken: version: 2.92.0, branch: master (modified), commit date: 2021-02-24 16:25, hash: 02948a2cab
Worked: (newest version of Blender that worked as expected)

Short description of error
When UV mapping meshes with multiple assigned materials containing textures of different aspect ratios, the "Correct" aspect option is not used individually, per assigned material, resulting in incorrectly stretched UV maps.

Exact steps for others to reproduce the error

Create a cube in a new, empty .blend file (make sure to delete the default cube and then re-add the new one straight away).
Create a new material and assign it a texture of a square aspect ratio as based color, then assign it to the cube.
Create another new material, and assign it a texture of non square aspect ratio (some visibly elongated rectangle).
Enter edit mode, select a few faces of the cube, and assign it tne other material, so that you end up with a cube which as two different materials in some of its faces.
Perform "Cube Projection" UV mapping operator and make sure the "Correct Aspect" is checked.
Expected: The aspect ratio should be corrected per material.
Result: The aspect ratio of textures in incorrect on one of the two materials.

**System Information** Operating system: Windows-10-10.0.19041-SP0 64 Bits Graphics card: GeForce GTX 1080 Ti/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 461.40 **Blender Version** Broken: version: 2.92.0, branch: master (modified), commit date: 2021-02-24 16:25, hash: `02948a2cab` Worked: (newest version of Blender that worked as expected) **Short description of error** When UV mapping meshes with multiple assigned materials containing textures of different aspect ratios, the "Correct" aspect option is not used individually, per assigned material, resulting in incorrectly stretched UV maps. **Exact steps for others to reproduce the error** 1. Create a cube in a new, empty .blend file (make sure to delete the default cube and then re-add the new one straight away). 2. Create a new material and assign it a texture of a square aspect ratio as based color, then assign it to the cube. 3. Create another new material, and assign it a texture of non square aspect ratio (some visibly elongated rectangle). 4. Enter edit mode, select a few faces of the cube, and assign it tne other material, so that you end up with a cube which as two different materials in some of its faces. 5. Perform "Cube Projection" UV mapping operator and make sure the "Correct Aspect" is checked. Expected: The aspect ratio should be corrected per material. Result: The aspect ratio of textures in incorrect on one of the two materials.

Ludvik Koutny commented

2021-03-07 11:29:15 +01:00

Added subscriber: @Rawalanche

Philipp Oeser commented

2021-03-07 13:52:30 +01:00

Added subscriber: @lichtwerk

Philipp Oeser commented

2021-03-07 13:52:30 +01:00

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

Philipp Oeser self-assigned this 2021-03-07 13:52:30 +01:00

Philipp Oeser commented

2021-03-07 13:52:30 +01:00

Hi @Rawalanche , looks wrong indeed, will check.

Philipp Oeser removed their assignment 2021-03-09 22:57:50 +01:00

Philipp Oeser commented

2021-03-09 22:57:50 +01:00

Added subscriber: @ideasman42

Philipp Oeser commented

2021-03-09 22:57:50 +01:00

Changed status from 'Confirmed' to: 'Needs Developer To Reproduce'

Philipp Oeser commented

2021-03-09 22:57:50 +01:00

Checked on this, and atm. this works based on the active selected face.

So if a face with material 1 is selected it will get the aspect from that associated image.
Likewise, if a face with material 2 is selected, it will base the unwrap on the aspect of that image.

So, this might already help in your workflow.
(we should probably mention this in the manual here https://docs.blender.org/manual/en/dev/modeling/meshes/editing/uv.html#id4)
This also means that in the end, this is working as intended [would not be considered a bug I am afraid]

We could of course make this automatically consider all faces individually [which will have a performance impact of course, so if interesting, this would have to be some sort of option], but this would do it:
P2024: T86358_snippet



diff --git a/source/blender/editors/uvedit/uvedit_unwrap_ops.c b/source/blender/editors/uvedit/uvedit_unwrap_ops.c
index fc5f41e8ed5..3e2d439cd91 100644
--- a/source/blender/editors/uvedit/uvedit_unwrap_ops.c
+++ b/source/blender/editors/uvedit/uvedit_unwrap_ops.c
@@ -221,6 +221,15 @@ static bool uvedit_have_selection_multi(const Scene *scene,
   return have_select;
 }
 
+static void ED_uvedit_face_get_aspect(Object *ob, BMFace *efa, float *r_aspx, float *r_aspy)
+{
+  Image *ima;
+
+  ED_object_get_active_image(ob, efa->mat_nr + 1, &ima, NULL, NULL, NULL);
+
+  ED_image_get_uv_aspect(ima, NULL, r_aspx, r_aspy);
+}
+
 void ED_uvedit_get_aspect(Object *ob, float *r_aspx, float *r_aspy)
 {
   BMEditMesh *em = BKE_editmesh_from_object(ob);
@@ -1456,33 +1465,26 @@ static void correct_uv_aspect(Object *ob, BMEditMesh *em)
 
   const int cd_loop_uv_offset = CustomData_get_offset(&em->bm->ldata, CD_MLOOPUV);
 
-  ED_uvedit_get_aspect(ob, &aspx, &aspy);
-
-  if (aspx == aspy) {
-    return;
-  }
+  BM_ITER_MESH (efa, &iter, em->bm, BM_FACES_OF_MESH) {
+    if (!BM_elem_flag_test(efa, BM_ELEM_SELECT)) {
+      continue;
+    }
 
-  if (aspx > aspy) {
-    scale = aspy / aspx;
+    ED_uvedit_face_get_aspect(ob, efa, &aspx, &aspy);
 
-    BM_ITER_MESH (efa, &iter, em->bm, BM_FACES_OF_MESH) {
-      if (!BM_elem_flag_test(efa, BM_ELEM_SELECT)) {
-        continue;
-      }
+    if (aspx == aspy) {
+      continue;
+    }
+    if (aspx > aspy) {
+      scale = aspy / aspx;
 
       BM_ITER_ELEM (l, &liter, efa, BM_LOOPS_OF_FACE) {
         luv = BM_ELEM_CD_GET_VOID_P(l, cd_loop_uv_offset);
         luv->uv- [x] = ((luv->uv- [x] - 0.5f) * scale) + 0.5f;
       }
     }
-  }
-  else {
-    scale = aspx / aspy;
-
-    BM_ITER_MESH (efa, &iter, em->bm, BM_FACES_OF_MESH) {
-      if (!BM_elem_flag_test(efa, BM_ELEM_SELECT)) {
-        continue;
-      }
+    else {
+      scale = aspx / aspy;
 
       BM_ITER_ELEM (l, &liter, efa, BM_LOOPS_OF_FACE) {
         luv = BM_ELEM_CD_GET_VOID_P(l, cd_loop_uv_offset);

I am keeping this open if this is interesting for the UV Editing module CC @ideasman42 , but will step down until a decision about this has been made from the module.

Checked on this, and atm. this works based on the active selected face. So if a face with material 1 is selected it will get the aspect from that associated image. Likewise, if a face with material 2 is selected, it will base the unwrap on the aspect of that image. So, this might already help in your workflow. (we should probably mention this in the manual here https://docs.blender.org/manual/en/dev/modeling/meshes/editing/uv.html#id4) This also means that in the end, this is working as intended [would not be considered a bug I am afraid] We could of course make this automatically consider all faces individually [which will have a performance impact of course, so if interesting, this would have to be some sort of option], but this would do it: [P2024: T86358_snippet](https://archive.blender.org/developer/P2024.txt) ``` diff --git a/source/blender/editors/uvedit/uvedit_unwrap_ops.c b/source/blender/editors/uvedit/uvedit_unwrap_ops.c index fc5f41e8ed5..3e2d439cd91 100644 --- a/source/blender/editors/uvedit/uvedit_unwrap_ops.c +++ b/source/blender/editors/uvedit/uvedit_unwrap_ops.c @@ -221,6 +221,15 @@ static bool uvedit_have_selection_multi(const Scene *scene, return have_select; } +static void ED_uvedit_face_get_aspect(Object *ob, BMFace *efa, float *r_aspx, float *r_aspy) +{ + Image *ima; + + ED_object_get_active_image(ob, efa->mat_nr + 1, &ima, NULL, NULL, NULL); + + ED_image_get_uv_aspect(ima, NULL, r_aspx, r_aspy); +} + void ED_uvedit_get_aspect(Object *ob, float *r_aspx, float *r_aspy) { BMEditMesh *em = BKE_editmesh_from_object(ob); @@ -1456,33 +1465,26 @@ static void correct_uv_aspect(Object *ob, BMEditMesh *em) const int cd_loop_uv_offset = CustomData_get_offset(&em->bm->ldata, CD_MLOOPUV); - ED_uvedit_get_aspect(ob, &aspx, &aspy); - - if (aspx == aspy) { - return; - } + BM_ITER_MESH (efa, &iter, em->bm, BM_FACES_OF_MESH) { + if (!BM_elem_flag_test(efa, BM_ELEM_SELECT)) { + continue; + } - if (aspx > aspy) { - scale = aspy / aspx; + ED_uvedit_face_get_aspect(ob, efa, &aspx, &aspy); - BM_ITER_MESH (efa, &iter, em->bm, BM_FACES_OF_MESH) { - if (!BM_elem_flag_test(efa, BM_ELEM_SELECT)) { - continue; - } + if (aspx == aspy) { + continue; + } + if (aspx > aspy) { + scale = aspy / aspx; BM_ITER_ELEM (l, &liter, efa, BM_LOOPS_OF_FACE) { luv = BM_ELEM_CD_GET_VOID_P(l, cd_loop_uv_offset); luv->uv- [x] = ((luv->uv- [x] - 0.5f) * scale) + 0.5f; } } - } - else { - scale = aspx / aspy; - - BM_ITER_MESH (efa, &iter, em->bm, BM_FACES_OF_MESH) { - if (!BM_elem_flag_test(efa, BM_ELEM_SELECT)) { - continue; - } + else { + scale = aspx / aspy; BM_ITER_ELEM (l, &liter, efa, BM_LOOPS_OF_FACE) { luv = BM_ELEM_CD_GET_VOID_P(l, cd_loop_uv_offset); ``` I am keeping this open if this is interesting for the `UV Editing` module CC @ideasman42 , but will step down until a decision about this has been made from the module.

Campbell Barton commented

2022-05-03 05:02:50 +02:00

@lichtwerk an issue with having per-face aspect is one UV island may use two different materials (which you would typically want to treat as a single UV island).

Possible options:

No aspect correction.
Aspect correction: average (where each island is assigned an aspect based on the materials of the faces it uses).
Aspect correction: per-face (where every face is scaled based on it's materials which may separate UV islands).

@lichtwerk an issue with having per-face aspect is one UV island may use two different materials (which you would typically want to treat as a single UV island). Possible options: - No aspect correction. - Aspect correction: average (where each island is assigned an aspect based on the materials of the faces it uses). - Aspect correction: per-face (where every face is scaled based on it's materials which may separate UV islands).

Philipp Oeser commented

2022-05-03 06:22:13 +02:00

uv_editing is part of #modeling, no?

- uv_editing is part of #modeling, no?

Campbell Barton commented

2022-05-03 06:26:55 +02:00

@lichtwerk yes, as this is a TODO, it doesn't seem so useful to keep in both workboards (modelling workboard is currently overloaded), but if it's important to keep in both I don't mind.

Philipp Oeser commented

2022-05-03 06:36:16 +02:00

@ideasman42: would prefer to keep #modeling (since only that is considered a real "top-level" module in many discussions, if we change that - which is of course possible - it would just need a couple of tweaks in the way we comunnicate this)