Fix #128887: LineArt: Prevent iterating over not evaluated objects

Line art uses `DEG_OBJECT_ITER_BEGIN` to load all objects that has a
geometry in the scene, which is kind of a hack since the beginning. This
left potential problem where the iterator could go through some objects
that line art didn't have a dependency on (e.g. other grease pencil
objects):

- Since those "other" objects often evaluates fast enough, so
line art always end up having valid data from everywhere after lengthy
mesh evaluation prior to itself, so this problem was not prominent.
- However, in rare cases, there are other objects that takes a lot of
time to evaluate, this causes line art to potentially iterate to objects
that are still invalid.

This fix will build a `Vector<Object *>` during `update_depsgraph`, and
use such list inside `DEGObjectIterSettings` to filter out objects that
the modifier isn't dependent on, thus remove the possibility of reading
objects that hasn't been evaluated.

Since Line art will not add grease pencil objects as potential geometry
inputs, all mesh/curve types of geometries generated by geometry nodes
modifier directly inside other grease pencil objects won't be loaded.
This can be mitigated by having a third mesh object that reads the
result from the grease pencil object that generates geometries, then
directly output them for line art to read.

This also fixes #128888.

Pull Request: https://projects.blender.org/blender/blender/pulls/128890

source/blender/depsgraph/DEG_depsgraph_query.hh

@@ -12,6 +12,7 @@
 
 #include "BLI_function_ref.hh"
 #include "BLI_iterator.h"
+#include "BLI_set.hh"
 #include "BLI_utildefines.h"
 
 #include "DEG_depsgraph.hh"
@@ -199,6 +200,12 @@ struct DEGObjectIterSettings {
    * geometry for the viewer path included in the iterator.
    */
   const ViewerPath *viewer_path;
+
+  /**
+   * If not empty, the iterator should only return objects that are in this list (or their
+   * instances are in it). Pointers in this span should be the original data-block.
+   */
+  blender::Set<const Object *> *included_objects;
 };
 
 /**

source/blender/depsgraph/intern/depsgraph_query_iter.cc

@@ -145,6 +145,14 @@ bool deg_iterator_duplis_step(DEGObjectIterData *data)
       continue;
     }
 
+    DEGObjectIterSettings *settings = data->settings;
+    if (settings->included_objects) {
+      Object *object_orig = DEG_get_original_object(obd);
+      if (!settings->included_objects->contains(object_orig)) {
+        continue;
+      }
+    }
+
     free_owned_memory(data);
 
     data->dupli_object_current = dob;
@@ -240,6 +248,13 @@ bool deg_iterator_objects_step(DEGObjectIterData *data)
     Object *object = (Object *)id_node->id_cow;
     Object *object_orig = DEG_get_original_object(object);
 
+    DEGObjectIterSettings *settings = data->settings;
+    if (settings->included_objects) {
+      if (!settings->included_objects->contains(object_orig)) {
+        continue;
+      }
+    }
+
     /* NOTE: The object might be invisible after the latest depsgraph evaluation, in which case
      * going into its evaluated state might not be safe. For example, its evaluated mesh state
      * might point to a freed data-block if the mesh is animated.

source/blender/editors/grease_pencil/intern/grease_pencil_lineart.cc

@@ -40,7 +40,7 @@
 #include "DEG_depsgraph.hh"
 #include "DEG_depsgraph_query.hh"
 
-#include "MOD_lineart.h"
+#include "MOD_lineart.hh"
 
 namespace blender::ed::greasepencil {
 

source/blender/makesdna/DNA_modifier_types.h

@@ -3219,6 +3219,9 @@ typedef struct GreasePencilLineartModifierData {
 
   /* Keep a pointer to the render buffer so we can call destroy from #ModifierData. */
   struct LineartData *la_data_ptr;
+
+  /* Points to a `LineartModifierRuntime`, which includes the object dependency list. */
+  void *runtime;
 } GreasePencilLineartModifierData;
 
 typedef struct GreasePencilArmatureModifierData {

source/blender/modifiers/CMakeLists.txt

@@ -135,8 +135,8 @@ set(SRC
   intern/MOD_ui_common.hh
   intern/MOD_util.hh
   intern/MOD_weightvg_util.hh
-  intern/lineart/MOD_lineart.h
-  intern/lineart/lineart_intern.h
+  intern/lineart/MOD_lineart.hh
+  intern/lineart/lineart_intern.hh
 )
 
 set(LIB

source/blender/modifiers/intern/MOD_lineart.cc

@@ -27,7 +27,7 @@
 #include "UI_resources.hh"
 
 #include "MOD_grease_pencil_util.hh"
-#include "MOD_lineart.h"
+#include "MOD_lineart.hh"
 #include "MOD_modifiertypes.hh"
 #include "MOD_ui_common.hh"
 
@@ -82,6 +82,32 @@ static void init_data(ModifierData *md)
 static void copy_data(const ModifierData *md, ModifierData *target, const int flag)
 {
   BKE_modifier_copydata_generic(md, target, flag);
+
+  const GreasePencilLineartModifierData *source_lmd =
+      reinterpret_cast<const GreasePencilLineartModifierData *>(md);
+  const LineartModifierRuntime *source_runtime = reinterpret_cast<const LineartModifierRuntime *>(
+      source_lmd->runtime);
+
+  GreasePencilLineartModifierData *target_lmd =
+      reinterpret_cast<GreasePencilLineartModifierData *>(target);
+
+  target_lmd->runtime = MEM_new<LineartModifierRuntime>(__func__);
+  LineartModifierRuntime *target_runtime = reinterpret_cast<LineartModifierRuntime *>(
+      target_lmd->runtime);
+
+  blender::Set<const Object *> *object_dependencies = source_runtime->object_dependencies.get();
+  target_runtime->object_dependencies.reset(
+      new blender::Set<const Object *>(*object_dependencies));
+}
+
+static void free_data(ModifierData *md)
+{
+  GreasePencilLineartModifierData *lmd = reinterpret_cast<GreasePencilLineartModifierData *>(md);
+  if (lmd->runtime) {
+    LineartModifierRuntime *runtime = reinterpret_cast<LineartModifierRuntime *>(lmd->runtime);
+    MEM_delete(runtime);
+    lmd->runtime = nullptr;
+  }
 }
 
 static bool is_disabled(const Scene * /*scene*/, ModifierData *md, bool /*use_render_params*/)
@@ -107,7 +133,8 @@ static bool is_disabled(const Scene * /*scene*/, ModifierData *md, bool /*use_re
 
 static void add_this_collection(Collection &collection,
                                 const ModifierUpdateDepsgraphContext *ctx,
-                                const int mode)
+                                const int mode,
+                                Set<const Object *> &object_dependencies)
 {
   bool default_add = true;
   /* Do not do nested collection usage check, this is consistent with lineart calculation, because
@@ -124,13 +151,15 @@ static void add_this_collection(Collection &collection,
       {
         DEG_add_object_relation(ctx->node, ob, DEG_OB_COMP_GEOMETRY, "Line Art Modifier");
         DEG_add_object_relation(ctx->node, ob, DEG_OB_COMP_TRANSFORM, "Line Art Modifier");
+        object_dependencies.add(ob);
       }
     }
     if (ob->type == OB_EMPTY && (ob->transflag & OB_DUPLICOLLECTION)) {
       if (!ob->instance_collection) {
         continue;
       }
-      add_this_collection(*ob->instance_collection, ctx, mode);
+      add_this_collection(*ob->instance_collection, ctx, mode, object_dependencies);
+      object_dependencies.add(ob);
     }
   }
   FOREACH_COLLECTION_VISIBLE_OBJECT_RECURSIVE_END;
@@ -147,8 +176,24 @@ static void update_depsgraph(ModifierData *md, const ModifierUpdateDepsgraphCont
 
   /* Do we need to distinguish DAG_EVAL_VIEWPORT or DAG_EVAL_RENDER here? */
 
-  add_this_collection(*ctx->scene->master_collection, ctx, DAG_EVAL_VIEWPORT);
+  LineartModifierRuntime *runtime = reinterpret_cast<LineartModifierRuntime *>(lmd->runtime);
+  if (!runtime) {
+    runtime = MEM_new<LineartModifierRuntime>(__func__);
+    lmd->runtime = runtime;
+    runtime->object_dependencies = nullptr;
+  }
+  Set<const Object *> *object_dependencies = runtime->object_dependencies.get();
+  if (!object_dependencies) {
+    runtime->object_dependencies = std::make_unique<Set<const Object *>>();
+    object_dependencies = runtime->object_dependencies.get();
+  }
 
+  object_dependencies->clear();
+  add_this_collection(
+      *ctx->scene->master_collection, ctx, DAG_EVAL_VIEWPORT, *object_dependencies);
+
+  /* No need to add any non-geometry objects into `lmd->object_dependencies` because we won't be
+   * loading */
   if (lmd->calculation_flags & MOD_LINEART_USE_CUSTOM_CAMERA && lmd->source_camera) {
     DEG_add_object_relation(
         ctx->node, lmd->source_camera, DEG_OB_COMP_TRANSFORM, "Line Art Modifier");
@@ -852,7 +897,7 @@ ModifierTypeInfo modifierType_GreasePencilLineart = {
 
     /*init_data*/ blender::init_data,
     /*required_data_mask*/ nullptr,
-    /*free_data*/ nullptr,
+    /*free_data*/ blender::free_data,
     /*is_disabled*/ blender::is_disabled,
     /*update_depsgraph*/ blender::update_depsgraph,
     /*depends_on_time*/ nullptr,

source/blender/modifiers/intern/lineart/MOD_lineart.hh

@@ -12,6 +12,7 @@
 #include "BLI_listbase.h"
 #include "BLI_math_matrix_types.hh"
 #include "BLI_math_vector.h"
+#include "BLI_set.hh"
 #include "BLI_threads.h"
 
 #include "ED_grease_pencil.hh"
@@ -23,6 +24,13 @@
 extern "C" {
 #endif
 
+typedef struct LineartModifierRuntime {
+  /* This list is constructed during `update_depsgraph()` call, and stays valid until the next
+   * update. This way line art can load objects from this list instead of iterating over all
+   * obejcts that may or may not have finished evaluating. */
+  std::unique_ptr<blender::Set<const Object *>> object_dependencies;
+} LineartModifierRuntime;
+
 typedef struct LineartStaticMemPoolNode {
   Link item;
   size_t size;

source/blender/modifiers/intern/lineart/lineart_chain.cc

@@ -9,9 +9,9 @@
 #include "BLI_listbase.h"
 #include "BLI_math_geom.h"
 
-#include "MOD_lineart.h"
+#include "MOD_lineart.hh"
 
-#include "lineart_intern.h"
+#include "lineart_intern.hh"
 
 #include <algorithm> /* For `min/max`. */
 #include <cmath>

source/blender/modifiers/intern/lineart/lineart_cpu.cc

@@ -8,7 +8,7 @@
 
 #include <algorithm>
 
-#include "MOD_lineart.h"
+#include "MOD_lineart.hh"
 
 #include "BLI_listbase.h"
 #include "BLI_math_base.hh"
@@ -62,7 +62,7 @@
 
 #include "GEO_join_geometries.hh"
 
-#include "lineart_intern.h"
+#include "lineart_intern.hh"
 
 #include <algorithm> /* For `min/max`. */
 
@@ -2551,7 +2551,8 @@ void lineart_main_load_geometries(Depsgraph *depsgraph,
                                   LineartData *ld,
                                   bool allow_duplicates,
                                   bool do_shadow_casting,
-                                  ListBase *shadow_elns)
+                                  ListBase *shadow_elns,
+                                  blender::Set<const Object *> *included_objects)
 {
   double proj[4][4], view[4][4], result[4][4];
   float inv[4][4];
@@ -2615,9 +2616,8 @@ void lineart_main_load_geometries(Depsgraph *depsgraph,
   DEGObjectIterSettings deg_iter_settings = {nullptr};
   deg_iter_settings.depsgraph = depsgraph;
   deg_iter_settings.flags = flags;
+  deg_iter_settings.included_objects = included_objects;
 
-  /* XXX(@Yiming): Temporary solution, this iterator is technically unsafe to use *during*
-   * depsgraph evaluation, see D14997 for detailed explanations. */
   DEG_OBJECT_ITER_BEGIN (&deg_iter_settings, ob) {
 
     obindex++;
@@ -5093,13 +5093,18 @@ bool MOD_lineart_compute_feature_lines_v3(Depsgraph *depsgraph,
   /* Get view vector before loading geometries, because we detect feature lines there. */
   lineart_main_get_view_vector(ld);
 
+  LineartModifierRuntime *runtime = reinterpret_cast<LineartModifierRuntime *>(lmd.runtime);
+  blender::Set<const Object *> *included_objects = runtime ? runtime->object_dependencies.get() :
+                                                             nullptr;
+
   lineart_main_load_geometries(depsgraph,
                                scene,
                                lineart_camera,
                                ld,
                                lmd.calculation_flags & MOD_LINEART_ALLOW_DUPLI_OBJECTS,
                                false,
-                               shadow_elns);
+                               shadow_elns,
+                               included_objects);
 
   if (shadow_generated) {
     lineart_main_transform_and_add_shadow(ld, shadow_veln, shadow_eeln);

source/blender/modifiers/intern/lineart/lineart_intern.hh

@@ -131,7 +131,8 @@ void lineart_main_load_geometries(struct Depsgraph *depsgraph,
                                   struct LineartData *ld,
                                   bool allow_duplicates,
                                   bool do_shadow_casting,
-                                  struct ListBase *shadow_elns);
+                                  struct ListBase *shadow_elns,
+                                  blender::Set<const Object *> *included_objects);
 /**
  * The calculated view vector will point towards the far-plane from the camera position.
  */

source/blender/modifiers/intern/lineart/lineart_shadow.cc

@@ -6,9 +6,9 @@
  * \ingroup modifiers
  */
 
-#include "MOD_lineart.h"
+#include "MOD_lineart.hh"
 
-#include "lineart_intern.h"
+#include "lineart_intern.hh"
 
 #include "BKE_global.hh"
 #include "BKE_grease_pencil.hh"
@@ -1251,8 +1251,18 @@ bool lineart_main_try_generate_shadow_v3(Depsgraph *depsgraph,
 
   lineart_main_get_view_vector(ld);
 
-  lineart_main_load_geometries(
-      depsgraph, scene, nullptr, ld, lmd->flags & MOD_LINEART_ALLOW_DUPLI_OBJECTS, true, nullptr);
+  LineartModifierRuntime *runtime = reinterpret_cast<LineartModifierRuntime *>(lmd->runtime);
+  blender::Set<const Object *> *included_objects = runtime ? runtime->object_dependencies.get() :
+                                                             nullptr;
+
+  lineart_main_load_geometries(depsgraph,
+                               scene,
+                               nullptr,
+                               ld,
+                               lmd->flags & MOD_LINEART_ALLOW_DUPLI_OBJECTS,
+                               true,
+                               nullptr,
+                               included_objects);
 
   if (!ld->geom.vertex_buffer_pointers.first) {
     /* No geometry loaded, return early. */

source/blender/modifiers/intern/lineart/lineart_util.cc

@@ -16,9 +16,9 @@
 
 #include "BLI_math_matrix.h"
 
-#include "MOD_lineart.h"
+#include "MOD_lineart.hh"
 
-#include "lineart_intern.h"
+#include "lineart_intern.hh"
 
 /* Line art memory and list helper */