Fix #106993: Slowness with Orbiting around select + Mesh Symmetry #107203

Germano Cavalcante · 2023-04-21T04:45:57+02:00

2023-04-21 04:45:57 +02:00

The issue happens because the algorithm used to calculate the center of
the selection first needs to create a TransData array. In this array,
the code calculates the "mirrored" elements which can be quite slow in
dense meshes.

So replace this slow algorithm used for calculating the pivot point with
the fast algorithm used to calculate the position of transform gizmos.

A simpler solution would be to skip creating those "mirrored" TransData. But I noticed that transform code has two algorithm to calculate selection center:

One is fast and is used for gizmos
The other is the one already mentioned and used in several other parts of the Blender code

Therefore, the solution proposed here is to separate the specific part of the code to calculate the position of the gizmos and use this code instead.

The gizmo's code is intertwined with calculating bounds and orientation, so to separate it out it was necessary to create a "foreach" function that takes a callback.

The original code of transform Gizmos is from @ideasman42. Therefore adding as a reviewer.
I'm also adding @HooglyBoogly as reviewer for C++ styling questions (if needed).

The issue happens because the algorithm used to calculate the center of the selection first needs to create a TransData array. In this array, the code calculates the "mirrored" elements which can be quite slow in dense meshes. So replace this slow algorithm used for calculating the pivot point with the fast algorithm used to calculate the position of transform gizmos. ---- A simpler solution would be to skip creating those "mirrored" TransData. But I noticed that transform code has two algorithm to calculate selection center: - One is fast and is used for gizmos - The other is the one already mentioned and used in several other parts of the Blender code Therefore, the solution proposed here is to separate the specific part of the code to calculate the position of the gizmos and use this code instead. The gizmo's code is intertwined with calculating bounds and orientation, so to separate it out it was necessary to create a "foreach" function that takes a callback. The original code of transform Gizmos is from @ideasman42. Therefore adding as a reviewer. I'm also adding @HooglyBoogly as reviewer for C++ styling questions (if needed).

Germano Cavalcante requested review from Campbell Barton 2023-04-21 04:46:26 +02:00

Germano Cavalcante requested review from Hans Goudey 2023-04-21 04:46:27 +02:00

Campbell Barton requested changes 2023-04-21 05:32:11 +02:00

Campbell Barton left a comment

Made an initial review, although I didn't dig into details.

Please note in the commit log why there was poor performance and what was changed to resolve this.
(this is noted below the --- so it may be enough to include this in the commit log).

Unless you have concerns or there is an area you think calls for a second opinion, I'll trust your judgement on this change.

Made an initial review, although I didn't dig into details. Please note in the commit log why there was poor performance and what was changed to resolve this. _(this is noted below the `---` so it may be enough to include this in the commit log)._ Unless you have concerns or there is an area you think calls for a second opinion, I'll trust your judgement on this change.

source/blender/editors/transform/transform_gizmo_3d.cc Outdated

						
				@ -488,3 +474,1 @@

				static void protectflag_to_drawflags_pchan(RegionView3D *rv3d,

				                                           const bPoseChannel *pchan,

				                                           short orientation_index)

				static int gizmo_3d_foreach_selected(const bContext *C,

Campbell Barton commented

2023-04-21 05:30:29 +02:00

As this function is reasonably involved, it would be good to add a doc-string giving an overview of the function and the parameters.

source/blender/editors/transform/transform_gizmo_3d.cc Outdated

						
				@ -491,0 +475,4 @@

				                                     const short orient_index,

				                                     const bool use_curve_handles,

				                                     const bool use_only_center,

				                                     const float (**r_mat)[4],

Campbell Barton commented

2023-04-21 05:28:36 +02:00

Pass r_ prefixed arguments last.

Pass `r_` prefixed arguments last.

source/blender/editors/transform/transform_gizmo_3d.cc Outdated

						
				@ -491,0 +477,4 @@

				                                     const bool use_only_center,

				                                     const float (**r_mat)[4],

				                                     short *r_drawflags,

				                                     void *data,

Campbell Barton commented

2023-04-21 05:28:16 +02:00

By convention, the data pointer is typically the after the callback that uses it (although this kind of thing might be worth documenting).

By convention, the `data` pointer is typically the after the callback that uses it (although this kind of thing might be worth documenting).

source/blender/editors/transform/transform_gizmo_3d.cc Outdated

						
				@ -503,3 +485,1 @@

				    protectflag_to_drawflags(OB_LOCK_LOC | OB_LOCK_ROT | OB_LOCK_SCALE, &rv3d->twdrawflag);

				  }

				}

				  const auto calc_tw_center_with_matrix = [](const float co[3],

Campbell Barton commented

2023-04-21 05:26:36 +02:00

This name doesn't seem correct anymore, calc_tw_center_with_matrix is simply calling callback which might calculate anything.

This name doesn't seem correct anymore, `calc_tw_center_with_matrix` is simply calling `callback` which might calculate anything.

source/blender/editors/transform/transform_gizmo_3d.cc Outdated

						
				@ -506,0 +485,4 @@

				  const auto calc_tw_center_with_matrix = [](const float co[3],

				                                             const bool use_matrix,

				                                             const float matrix[4][4],

				                                             void (*callback)(const float co[3], void *data),