Refactor: Consistent WM jobs API; avoid function pointers as identifiers

Basically this tries to make the API to stop and kill jobs more explicit &
consistent, so intent is expressed clearly & behavior as expected.

- Remove use of the job start callback address as identifier for the job.
  6887dea786 already removed this pattern from the jobs system internals, this
  commit also removes it from the API.
- Make stop & kill API and implementation consistent. E.g. don't stop/kill jobs
  by either owner **or** type/callback in one function, and by owner (if
  provided) **and** type/callback in another. Causes some small behavior
  changes, documented inline.
- Use the same job type and API for all preview render jobs (change by Brecht).
  There doesn't seem to be a need for the separated types, in fact the
  separation might have caused some issues earlier (and added code complexity).
- Add/improve function documentation.

This does actually have subtle behavior changes that are known, see PR, but
they were investigated carefully and seem like implementing wanted behavior.

Co-authored-by: Brecht Van Lommel <brecht@blender.org>
Pull Request: https://projects.blender.org/blender/blender/pulls/123086

source/blender/draw/engines/eevee_next/eevee_lightcache.cc

@@ -310,7 +310,7 @@ wmJob *EEVEE_NEXT_lightbake_job_create(wmWindowManager *wm,
   }
 
   /* Stop existing baking job. */
-  WM_jobs_stop(wm, nullptr, EEVEE_NEXT_lightbake_job);
+  WM_jobs_stop_type(wm, nullptr, WM_JOB_TYPE_LIGHT_BAKE);
 
   wmJob *wm_job = WM_jobs_get(wm,
                               win,

source/blender/editors/include/ED_node_preview.hh

@@ -46,6 +46,5 @@ ImBuf *node_preview_acquire_ibuf(bNodeTree &ntree,
                                  const bNode &node);
 void node_release_preview_ibuf(NestedTreePreviews &tree_previews);
 NestedTreePreviews *get_nested_previews(const bContext &C, SpaceNode &snode);
-void stop_preview_job(wmWindowManager &wm);
 
 }  // namespace blender::ed::space_node

source/blender/editors/interface/templates/interface_templates.cc

@@ -5966,22 +5966,22 @@ static void do_running_jobs(bContext *C, void * /*arg*/, int event)
       G.is_break = true;
       break;
     case B_STOPCAST:
-      WM_jobs_stop(CTX_wm_manager(C), CTX_wm_screen(C), nullptr);
+      WM_jobs_stop_all_from_owner(CTX_wm_manager(C), CTX_wm_screen(C));
       break;
     case B_STOPANIM:
       WM_operator_name_call(C, "SCREEN_OT_animation_play", WM_OP_INVOKE_SCREEN, nullptr, nullptr);
       break;
     case B_STOPCOMPO:
-      WM_jobs_stop(CTX_wm_manager(C), CTX_data_scene(C), nullptr);
+      WM_jobs_stop_all_from_owner(CTX_wm_manager(C), CTX_data_scene(C));
       break;
     case B_STOPSEQ:
-      WM_jobs_stop(CTX_wm_manager(C), CTX_data_scene(C), nullptr);
+      WM_jobs_stop_all_from_owner(CTX_wm_manager(C), CTX_data_scene(C));
       break;
     case B_STOPCLIP:
-      WM_jobs_stop(CTX_wm_manager(C), CTX_data_scene(C), nullptr);
+      WM_jobs_stop_all_from_owner(CTX_wm_manager(C), CTX_data_scene(C));
       break;
     case B_STOPFILE:
-      WM_jobs_stop(CTX_wm_manager(C), CTX_data_scene(C), nullptr);
+      WM_jobs_stop_all_from_owner(CTX_wm_manager(C), CTX_data_scene(C));
       break;
     case B_STOPOTHER:
       G.is_break = true;

source/blender/editors/render/render_preview.cc

@@ -2113,8 +2113,7 @@ void ED_preview_kill_jobs(wmWindowManager *wm, Main * /*bmain*/)
   if (wm) {
     /* This is called to stop all preview jobs before scene data changes, to
      * avoid invalid memory access. */
-    WM_jobs_kill(wm, nullptr, common_preview_startjob);
-    WM_jobs_kill(wm, nullptr, icon_preview_startjob_all_sizes);
+    WM_jobs_kill_type(wm, nullptr, WM_JOB_TYPE_RENDER_PREVIEW);
   }
 }
 

source/blender/editors/render/render_update.cc

@@ -177,8 +177,8 @@ void ED_render_engine_changed(Main *bmain, const bool update_scene_data)
       ED_render_engine_area_exit(bmain, area);
     }
   }
-  /* Invalidate all shader previews. */
-  blender::ed::space_node::stop_preview_job(*static_cast<wmWindowManager *>(bmain->wm.first));
+  /* Stop and invalidate all shader previews. */
+  ED_preview_kill_jobs(static_cast<wmWindowManager *>(bmain->wm.first), bmain);
   LISTBASE_FOREACH (Material *, ma, &bmain->materials) {
     BKE_material_make_node_previews_dirty(ma);
   }

source/blender/editors/scene/scene_edit.cc

@@ -130,7 +130,7 @@ bool ED_scene_delete(bContext *C, Main *bmain, Scene *scene)
 
   /* kill running jobs */
   wmWindowManager *wm = static_cast<wmWindowManager *>(bmain->wm.first);
-  WM_jobs_kill_type(wm, scene, WM_JOB_TYPE_ANY);
+  WM_jobs_kill_all_from_owner(wm, scene);
 
   if (scene->id.prev) {
     scene_new = static_cast<Scene *>(scene->id.prev);

source/blender/editors/space_node/node_group.cc

@@ -173,7 +173,6 @@ static int node_group_edit_exec(bContext *C, wmOperator *op)
   const bool exit = RNA_boolean_get(op->ptr, "exit");
 
   ED_preview_kill_jobs(CTX_wm_manager(C), CTX_data_main(C));
-  stop_preview_job(*CTX_wm_manager(C));
 
   bNode *gnode = node_group_get_active(C, node_idname);
 
@@ -631,7 +630,6 @@ static int node_group_separate_exec(bContext *C, wmOperator *op)
   int type = RNA_enum_get(op->ptr, "type");
 
   ED_preview_kill_jobs(CTX_wm_manager(C), bmain);
-  stop_preview_job(*CTX_wm_manager(C));
 
   /* are we inside of a group? */
   bNodeTree *ngroup = snode->edittree;
@@ -1238,7 +1236,6 @@ static int node_group_make_exec(bContext *C, wmOperator *op)
   Main *bmain = CTX_data_main(C);
 
   ED_preview_kill_jobs(CTX_wm_manager(C), CTX_data_main(C));
-  stop_preview_job(*CTX_wm_manager(C));
 
   VectorSet<bNode *> nodes_to_group = get_nodes_to_group(ntree, nullptr);
   if (!node_group_make_test_selected(ntree, nodes_to_group, ntree_idname, *op->reports)) {
@@ -1292,7 +1289,6 @@ static int node_group_insert_exec(bContext *C, wmOperator *op)
   const char *node_idname = node_group_idname(C);
 
   ED_preview_kill_jobs(CTX_wm_manager(C), CTX_data_main(C));
-  stop_preview_job(*CTX_wm_manager(C));
 
   bNode *gnode = node_group_get_active(C, node_idname);
   if (!gnode || !gnode->id) {

source/blender/editors/space_node/node_shader_preview.cc

@@ -782,7 +782,7 @@ static void ensure_nodetree_previews(const bContext &C,
     return;
   }
   if (tree_previews.rendering) {
-    WM_jobs_stop(CTX_wm_manager(&C), CTX_wm_space_node(&C), shader_preview_startjob);
+    WM_jobs_stop_type(CTX_wm_manager(&C), CTX_wm_space_node(&C), WM_JOB_TYPE_RENDER_PREVIEW);
     return;
   }
   tree_previews.rendering = true;
@@ -837,15 +837,10 @@ static void ensure_nodetree_previews(const bContext &C,
   WM_jobs_start(CTX_wm_manager(&C), wm_job);
 }
 
-void stop_preview_job(wmWindowManager &wm)
-{
-  WM_jobs_stop(&wm, nullptr, shader_preview_startjob);
-}
-
 void free_previews(wmWindowManager &wm, SpaceNode &snode)
 {
   /* This should not be called from the drawing pass, because it will result in a deadlock. */
-  WM_jobs_kill(&wm, &snode, shader_preview_startjob);
+  WM_jobs_kill_type(&wm, &snode, WM_JOB_TYPE_RENDER_PREVIEW);
   snode.runtime->tree_previews_per_context.clear_and_shrink();
 }
 

source/blender/editors/space_sequencer/sequencer_thumbnails.cc

@@ -311,7 +311,7 @@ static void sequencer_thumbnail_start_job_if_necessary(
   if (v2d->cur.xmax != sseq->runtime->last_thumbnail_area.xmax ||
       v2d->cur.ymax != sseq->runtime->last_thumbnail_area.ymax)
   {
-    WM_jobs_stop(CTX_wm_manager(C), nullptr, thumbnail_start_job);
+    WM_jobs_stop_type(CTX_wm_manager(C), nullptr, WM_JOB_TYPE_SEQ_DRAW_THUMBNAIL);
   }
 
   sequencer_thumbnail_init_job(C, v2d, ed, thumb_height);

source/blender/editors/space_view3d/space_view3d.cc

@@ -167,7 +167,7 @@ void ED_view3d_stop_render_preview(wmWindowManager *wm, ARegion *region)
     BPy_BEGIN_ALLOW_THREADS;
 #endif
 
-    WM_jobs_kill_type(wm, region, WM_JOB_TYPE_RENDER_PREVIEW);
+    WM_jobs_kill_type(wm, nullptr, WM_JOB_TYPE_RENDER_PREVIEW);
 
 #ifdef WITH_PYTHON
     BPy_END_ALLOW_THREADS;

source/blender/windowmanager/WM_api.hh

@@ -1671,13 +1671,19 @@ void WM_jobs_callbacks_ex(wmJob *wm_job,
  */
 void WM_jobs_start(wmWindowManager *wm, wmJob *wm_job);
 /**
- * Signal job(s) from this owner or callback to stop, timer is required to get handled.
+ * Signal all jobs of this type and owner (if non-null) to stop, timer is required to get
+ * handled.
+ *
+ * Don't pass #WM_JOB_TYPE_ANY as \a job_type. Use #WM_jobs_stop_all_from_owner() instead.
  */
-void WM_jobs_stop(wmWindowManager *wm, const void *owner, wm_jobs_start_callback startjob);
+void WM_jobs_stop_type(wmWindowManager *wm, const void *owner, eWM_JobType job_type);
 /**
- * Actually terminate thread and job timer.
+ * Signal all jobs from this owner to stop, timer is required to get handled.
+ *
+ * Beware of the impact of calling this. For example passing the scene will stop **all** jobs
+ * having the scene as owner, even otherwise unrelated jobs.
  */
-void WM_jobs_kill(wmWindowManager *wm, void *owner, wm_jobs_start_callback startjob);
+void WM_jobs_stop_all_from_owner(wmWindowManager *wm, const void *owner) ATTR_NONNULL();
 /**
  * Wait until every job ended.
  */
@@ -1686,7 +1692,19 @@ void WM_jobs_kill_all(wmWindowManager *wm);
  * Wait until every job ended, except for one owner (used in undo to keep screen job alive).
  */
 void WM_jobs_kill_all_except(wmWindowManager *wm, const void *owner);
+/**
+ * Terminate thread and timer of all jobs of this type and owner (if non-null).
+ *
+ * Don't pass #WM_JOB_TYPE_ANY as \a job_type. Use #WM_jobs_kill_all_from_owner() instead.
+ */
 void WM_jobs_kill_type(wmWindowManager *wm, const void *owner, int job_type);
+/**
+ * Terminate thread and timer of all jobs from this owner.
+ *
+ * Beware of the impact of calling this. For example passing the scene will kill **all** jobs
+ * having the scene as owner, even otherwise unrelated jobs.
+ */
+void WM_jobs_kill_all_from_owner(wmWindowManager *wm, const void *owner) ATTR_NONNULL();
 
 bool WM_jobs_has_running(const wmWindowManager *wm);
 bool WM_jobs_has_running_type(const wmWindowManager *wm, int job_type);

source/blender/windowmanager/intern/wm_jobs.cc

@@ -596,21 +596,37 @@ void WM_jobs_kill_all_except(wmWindowManager *wm, const void *owner)
 
 void WM_jobs_kill_type(wmWindowManager *wm, const void *owner, int job_type)
 {
+  BLI_assert(job_type != WM_JOB_TYPE_ANY);
+
   LISTBASE_FOREACH_MUTABLE (wmJob *, wm_job, &wm->jobs) {
     if (owner && wm_job->owner != owner) {
       continue;
     }
 
-    if (ELEM(job_type, WM_JOB_TYPE_ANY, wm_job->job_type)) {
+    if (wm_job->job_type == job_type) {
       wm_jobs_kill_job(wm, wm_job);
     }
   }
 }
 
-void WM_jobs_stop(wmWindowManager *wm, const void *owner, wm_jobs_start_callback startjob)
+void WM_jobs_kill_all_from_owner(wmWindowManager *wm, const void *owner)
 {
+  LISTBASE_FOREACH_MUTABLE (wmJob *, wm_job, &wm->jobs) {
+    if (wm_job->owner == owner) {
+      wm_jobs_kill_job(wm, wm_job);
+    }
+  }
+}
+
+void WM_jobs_stop_type(wmWindowManager *wm, const void *owner, eWM_JobType job_type)
+{
+  BLI_assert(job_type != WM_JOB_TYPE_ANY);
+
   LISTBASE_FOREACH (wmJob *, wm_job, &wm->jobs) {
-    if (wm_job->owner == owner || wm_job->startjob == startjob) {
+    if (owner && wm_job->owner != owner) {
+      continue;
+    }
+    if (wm_job->job_type == job_type) {
       if (wm_job->running) {
         wm_job->worker_status.stop = true;
       }
@@ -618,11 +634,13 @@ void WM_jobs_stop(wmWindowManager *wm, const void *owner, wm_jobs_start_callback
   }
 }
 
-void WM_jobs_kill(wmWindowManager *wm, void *owner, wm_jobs_start_callback startjob)
+void WM_jobs_stop_all_from_owner(wmWindowManager *wm, const void *owner)
 {
-  LISTBASE_FOREACH_MUTABLE (wmJob *, wm_job, &wm->jobs) {
-    if (wm_job->owner == owner || wm_job->startjob == startjob) {
-      wm_jobs_kill_job(wm, wm_job);
+  LISTBASE_FOREACH (wmJob *, wm_job, &wm->jobs) {
+    if (wm_job->owner == owner) {
+      if (wm_job->running) {
+        wm_job->worker_status.stop = true;
+      }
     }
   }
 }