blender
/
blender
137
418
Fork 600
Code Issues 6.3k Pull Requests 783 Projects 19 Wiki Activity

Anim: thread remake_graph_transdata #119497

Merged
Christoph Lendenfeld merged 13 commits from ChrisLend/blender:thread_remake_transdata into main 2024-04-09 11:46:17 +02:00
Conversation 0 Commits 13 Files Changed 1 +19 -19
1 changed files with 19 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 8206a0edec - Show all commits

38
source/blender/editors/transform/transform_convert_graph.cc
View File

@ -886,32 +886,32 @@ static void beztmap_to_data(TransInfo *t, FCurve *fcu, BeztMap *bezms, int totve
*/
static void remake_graph_transdata(TransInfo *t, const blender::Span<FCurve *> fcurves)
{
SCOPED_TIMER_AVERAGED("remake");
SpaceGraph *sipo = (SpaceGraph *)t->area->spacedata.first;
const bool use_handle = (sipo->flag & SIPO_NOHANDLES) == 0;
Sybren A. Stüvel commented 2024-04-09 11:14:44 +02:00
Outdated
Review
Copy Link

Add a comment that explains how the 8 got here.

Add a comment that explains how the 8 got here.
blender::threading::parallel_for(
fcurves.index_range(), 16, [&](const blender::IndexRange range) {
for (const int i : range) {
FCurve *fcu = fcurves[i];
blender::threading::parallel_for(fcurves.index_range(), 1, [&](const blender::IndexRange range) {
dr.sybren marked this conversation as resolved Outdated
Falk David commented 2024-03-28 14:59:40 +01:00
Outdated
Review
Copy Link

Looks like you don't need the index here, so it might be better to use parallel_for_each. Of course, that's only if you actually want a grain size of one.

Looks like you don't need the index here, so it might be better to use `parallel_for_each`. Of course, that's only if you actually want a grain size of one.
Iliya Katushenock commented 2024-03-28 15:01:10 +01:00
Outdated
Review
Copy Link

Even with grain size 1, range can have any size. But i wonder if this will be better to test parallel_for_weighted here.

Even with grain size `1`, `range` can have any size. But i wonder if this will be better to test `parallel_for_weighted` here.
Hans Goudey commented 2024-03-28 15:07:44 +01:00
Outdated
Review
Copy Link

parallel_for_each uses a different algorithm internally that has more overhead. AFAIK it will always give each element its own thread. I think it's typically only suitable for much more expensive tasks.

`parallel_for_each` uses a different algorithm internally that has more overhead. AFAIK it will always give each element its own thread. I think it's typically only suitable for much more expensive tasks.
for (const int i : range) {
FCurve *fcu = fcurves[i];
Sybren A. Stüvel commented 2024-04-09 11:15:39 +02:00
Review
Copy Link

if (!fcu->bezt) { continue; }

Gitea already doesn't present this as "it's all the same, just indented", so I think it's fine to restructure the code a bit further for readability.

`if (!fcu->bezt) { continue; }` Gitea already doesn't present this as "it's all the same, just indented", so I think it's fine to restructure the code a bit further for readability.
👍 1
if (fcu->bezt) {
BeztMap *bezm;
if (fcu->bezt) {
BeztMap *bezm;
/* Adjust transform-data pointers. */
/* NOTE: none of these functions use 'use_handle', it could be removed. */
bezm = bezt_to_beztmaps(fcu->bezt, fcu->totvert);
sort_time_beztmaps(bezm, fcu->totvert);
beztmap_to_data(t, fcu, bezm, fcu->totvert);
/* Adjust transform-data pointers. */
/* NOTE: none of these functions use 'use_handle', it could be removed. */
bezm = bezt_to_beztmaps(fcu->bezt, fcu->totvert);
sort_time_beztmaps(bezm, fcu->totvert);
beztmap_to_data(t, fcu, bezm, fcu->totvert);
/* Free mapping stuff. */
MEM_freeN(bezm);
/* Free mapping stuff. */
MEM_freeN(bezm);
/* Re-sort actual beztriples
* (perhaps this could be done using the beztmaps to save time?). */
sort_time_fcurve(fcu);
}
}
});
/* Re-sort actual beztriples
* (perhaps this could be done using the beztmaps to save time?). */
sort_time_fcurve(fcu);
}
}
});
/* Run the handle recalculation outside the parallel_for
* because it might use threading itself. */