Cleanup: Reduce unnecessary mesh position copying #111313

Jesse Yurkovich · 2023-08-20T07:04:43+02:00

Jesse Yurkovich commented

2023-08-20 07:04:43 +02:00

Continuation of #103789 to remove/reduce the copying of mesh position
data.

Notes:
DerivedMesh.cc Still requires a copy, but the work is done directly to the custom data layer, eliminating one memcpy at the end (1 copy instead of 2)
MOD_uvproject.cc Instead of iterating over the verts 3 times (during the vert copy, the world-space conversion, and the uv-project), only 2 iterations are done now (world-space conversion and the uv-project)

scenario	old	new
UVProject onto a 31658 vert Suzanne (10 times)	Timer 'MOD_uvproject': (Average: 0.5 ms, Min: 0.2 ms, Last: 0.2 ms)	Timer 'MOD_uvproject': (Average: 0.2 ms, Min: 0.2 ms, Last: 0.2 ms)
A 16641 vert plane with a Cloth modifier (20 frames)	Timer 'add_orco_mesh': (Average: 41252 ns, Min: 1100 ns, Last: 0.2 ms)	Timer 'add_orco_mesh': (Average: 37633 ns, Min: 1200 ns, Last: 0.3 ms)
Applying Simple Deform Twist to a 24578 vert object (10 times)	Timer 'create_applied_mesh_for_modifier': (Average: 1.0 ms, Min: 0.8 ms, Last: 0.8 ms)	Timer 'create_applied_mesh_for_modifier': (Average: 0.7 ms, Min: 0.3 ms, Last: 0.3 ms)

Continuation of #103789 to remove/reduce the copying of mesh position data. ---- Notes: `DerivedMesh.cc` Still requires a copy, but the work is done directly to the custom data layer, eliminating one `memcpy` at the end (1 copy instead of 2) `MOD_uvproject.cc` Instead of iterating over the verts 3 times (during the vert copy, the world-space conversion, and the uv-project), only 2 iterations are done now (world-space conversion and the uv-project) | scenario | old | new | | -- | -- | -- | | UVProject onto a 31658 vert Suzanne (10 times) | Timer 'MOD_uvproject': (Average: 0.5 ms, Min: 0.2 ms, Last: 0.2 ms) | Timer 'MOD_uvproject': (Average: 0.2 ms, Min: 0.2 ms, Last: 0.2 ms) | | A 16641 vert plane with a Cloth modifier (20 frames) | Timer 'add_orco_mesh': (Average: 41252 ns, Min: 1100 ns, Last: 0.2 ms) | Timer 'add_orco_mesh': (Average: 37633 ns, Min: 1200 ns, Last: 0.3 ms) | | Applying Simple Deform Twist to a 24578 vert object (10 times) | Timer 'create_applied_mesh_for_modifier': (Average: 1.0 ms, Min: 0.8 ms, Last: 0.8 ms) | Timer 'create_applied_mesh_for_modifier': (Average: 0.7 ms, Min: 0.3 ms, Last: 0.3 ms) |

Jesse Yurkovich added 2 commits 2023-08-20 07:04:57 +02:00

f666d2c101 Cleanup: Reduce unnecessary mesh position copying

buildbot/vexp-code-patch-coordinator Build done. Details

ae25ed5a8c Ensure header is added

Iliya Katushenock requested review from Hans Goudey 2023-08-20 14:55:11 +02:00

Hans Goudey commented

2023-08-20 16:06:46 +02:00

@blender-bot build

Hans Goudey approved these changes 2023-08-20 16:16:22 +02:00

Hans Goudey left a comment

Thanks! Just two comments inline, but I don't think they require another review.

source/blender/modifiers/intern/MOD_uvproject.cc Outdated

						
				@ -100,3 +100,2 @@

				  using namespace blender;

				  float(*coords)[3], (*co)[3];

				  int i, verts_num;

Hans Goudey commented

2023-08-20 16:15:55 +02:00

Whitespace after the "using namespace" lines usually isn't necessary IMO

deadpin marked this conversation as resolved

Jesse Yurkovich commented

2023-08-21 20:16:12 +02:00

I only see 1 comment? Just checking as I want to make sure I get all your feedback :)

Hans Goudey reviewed 2023-08-21 20:19:28 +02:00

Hans Goudey left a comment

Oop, not sure what happened there, thanks for following up!

source/blender/blenkernel/intern/DerivedMesh.cc Outdated

						
				@ -408,2 +408,3 @@

				  const int totvert = mesh->totvert;

				  totvert = mesh->totvert;

				  auto make_layer_orco = [mesh, layer]() -> MutableSpan<float3> {

Hans Goudey commented

2023-08-21 20:19:15 +02:00

Looks like this might make more sense as a separate function called something like "orco_coord_layer_ensure", just to make the use of data clearer and make this easier to read.

deadpin marked this conversation as resolved

Jesse Yurkovich added 2 commits 2023-08-21 22:07:23 +02:00

5b14ddc59d Feedback

buildbot/vexp-code-patch-coordinator Build done. Details

e382025671 Merge remote-tracking branch 'upstream/main' into cleanup-meshcopy

Jesse Yurkovich commented

2023-08-21 22:07:39 +02:00

@blender-bot build

Jesse Yurkovich merged commit f30ac938de into main

2023-08-22 03:52:00 +02:00

Jesse Yurkovich deleted branch cleanup-meshcopy

2023-08-22 03:52:02 +02:00