"Pick Shortest Path" removes first selected from selection history (making following "Merge Vertices > First" to always merge at last) #117625

New Issue

Sergei Sobolev · 2024-01-29T10:41:58+01:00

Sergei Sobolev commented

2024-01-29 10:41:58 +01:00

System Information
Operating system: Windows-10-10.0.22621-SP0 64 Bits
Graphics card: NVIDIA GeForce RTX 3070/PCIe/SSE2 NVIDIA Corporation 4.6.0 NVIDIA 551.23

Blender Version
Broken: version: 4.0.2, branch: blender-v4.0-release, commit date: 2023-12-05 07:41, hash: 9be62e85b727
Worked: (newest version of Blender that worked as expected)

Short description of error
Merge first/last doesn't count which vertex was the first or last.

Exact steps for others to reproduce the error
Select the first vertex.
Hold control and select the last.
Press M and choose merge at first/last

the file and a gif are attached to the bug.

**System Information** Operating system: Windows-10-10.0.22621-SP0 64 Bits Graphics card: NVIDIA GeForce RTX 3070/PCIe/SSE2 NVIDIA Corporation 4.6.0 NVIDIA 551.23 **Blender Version** Broken: version: 4.0.2, branch: blender-v4.0-release, commit date: 2023-12-05 07:41, hash: `9be62e85b727` Worked: (newest version of Blender that worked as expected) **Short description of error** Merge first/last doesn't count which vertex was the first or last. **Exact steps for others to reproduce the error** Select the first vertex. Hold control and select the last. Press M and choose merge at first/last the file and a gif are attached to the bug.

24.gif

352 KiB

24.blend

4.3 MiB

👍 1

Sergei Sobolev added the

labels 2024-01-29 10:41:58 +01:00

Philipp Oeser commented

2024-01-29 12:27:44 +01:00

Seems to work as expected here (if I select the vertices again after opening the file).

Are you doing this right after opening the file?
Or have you actually selected the vertices again after opening the file?

Not sure if this is pure runtime data (does not look like it on first sight) -- if it is, this is probably expected behavior -- but first please answer the above question to make sure we are all on the same page

Seems to work as expected here (if I select the vertices again after opening the file). Are you doing this right after opening the file? Or have you actually selected the vertices again after opening the file? Not sure if this is pure runtime data (does not look like it on first sight) -- if it is, this is probably expected behavior -- but first please answer the above question to make sure we are all on the same page

Philipp Oeser added

Status

Needs Information from User

and removed

Status

Needs Triage

labels 2024-01-29 12:27:53 +01:00

Sergei Sobolev commented

2024-01-30 10:57:48 +01:00

Please, do it again. Also, I've attached a GIF that shows the bug. I reopened the file and reproduced the bug. And here is attached a special video with a little explanation.

24.mkv

4.5 MiB

Philipp Oeser commented

2024-01-30 11:46:40 +01:00

Ah, can confirm now, seems like Pick Shortest Path removes the first selected element from the selection history on purpose, see the following call:

BM_select_history_remove(bm, v_act)

Then the last elem in the path might (or might not) be stored in the selection history [which seems fine].

Not sure exactly why the first element is unconditionally always removed here, maybe @ideasman42 recalls why this was done?
If this was done to ensure the same element is not in the history twice, this seems unnecessary since BM_select_history_store takes care of this?

Ah, can confirm now, seems like `Pick Shortest Path` removes the first selected element from the selection history on purpose, see the following call: `BM_select_history_remove(bm, v_act)` Then the last elem in the path might (or might not) be stored in the selection history [which seems fine]. Not sure exactly why the first element is unconditionally always removed here, maybe @ideasman42 recalls why this was done? If this was done to ensure the same element is not in the history twice, this seems unnecessary since `BM_select_history_store` takes care of this?

Philipp Oeser added

Module

Modeling

Status

Needs Info from Developers

and removed

Status

Needs Information from User

labels 2024-01-30 11:46:57 +01:00

Philipp Oeser commented

2024-01-30 12:05:28 +01:00

Actually, my assumption is that code just swapped active and destination.

For vertices v_act is the first selected vertex, then v_dst is the second vertex selected to calculate the path in between.
To me, it makes more sense to remove the second one from the list (if at all) and then possibly (if PathSelectParams track_active is ON) add it back and make it active. But not "loose" the first one ever?

So, could it be that the intention was to do the following @ideasman42 ?


diff --git a/source/blender/editors/mesh/editmesh_path.cc b/source/blender/editors/mesh/editmesh_path.cc
index e48e106e3e8..297e87baf5a 100644
--- a/source/blender/editors/mesh/editmesh_path.cc
+++ b/source/blender/editors/mesh/editmesh_path.cc
@@ -221,7 +221,7 @@ static void mouse_mesh_shortest_path_vert(Scene * /*scene*/,
 
     if (path) {
       if (op_params->track_active) {
-        BM_select_history_remove(bm, v_act);
+        BM_select_history_remove(bm, v_dst);
       }
     }
   }
@@ -428,7 +428,7 @@ static void mouse_mesh_shortest_path_edge(
 
     if (path) {
       if (op_params->track_active) {
-        BM_select_history_remove(bm, e_act);
+        BM_select_history_remove(bm, e_dst);
       }
     }
   }
@@ -574,7 +574,7 @@ static void mouse_mesh_shortest_path_face(Scene * /*scene*/,
     if (f_act != f_dst) {
       if (path) {
         if (op_params->track_active) {
-          BM_select_history_remove(bm, f_act);
+          BM_select_history_remove(bm, f_dst);
         }
       }
     }

Actually, my assumption is that code just swapped active and destination. For vertices `v_act` is the first selected vertex, then `v_dst` is the second vertex selected to calculate the path in between. To me, it makes more sense to remove the second one from the list (if at all) and then possibly (if `PathSelectParams` `track_active` is ON) add it back and make it active. But not "loose" the first one ever? So, could it be that the intention was to do the following @ideasman42 ? ```diff diff --git a/source/blender/editors/mesh/editmesh_path.cc b/source/blender/editors/mesh/editmesh_path.cc index e48e106e3e8..297e87baf5a 100644 --- a/source/blender/editors/mesh/editmesh_path.cc +++ b/source/blender/editors/mesh/editmesh_path.cc @@ -221,7 +221,7 @@ static void mouse_mesh_shortest_path_vert(Scene * /*scene*/, if (path) { if (op_params->track_active) { - BM_select_history_remove(bm, v_act); + BM_select_history_remove(bm, v_dst); } } } @@ -428,7 +428,7 @@ static void mouse_mesh_shortest_path_edge( if (path) { if (op_params->track_active) { - BM_select_history_remove(bm, e_act); + BM_select_history_remove(bm, e_dst); } } } @@ -574,7 +574,7 @@ static void mouse_mesh_shortest_path_face(Scene * /*scene*/, if (f_act != f_dst) { if (path) { if (op_params->track_active) { - BM_select_history_remove(bm, f_act); + BM_select_history_remove(bm, f_dst); } } } ```

Philipp Oeser changed title from ~~Merge first/last doesn't count which vertex was the first or last.~~ to "Pick Shortest Path" removes first selected from selection history (making following "Merge Vertices > First" to always merge at last)

2024-01-30 12:07:20 +01:00

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

"Pick Shortest Path" removes first selected from selection history (making following "Merge Vertices > First" to always merge at last) #117625