Fix T87929: Cycles, missing update when visibility is modified

This issue originates from a missing BVH packing for visibility data
when it is modified.

To fix this, this adds update flags to the managers to carry the
modified visibility information from the Objects' modified flag to the
GeometryManager.

Another set of flags is added to determine which data need to be packed:
geometry, vertices, or visibility. Those flags are then used when
packing the primivites.

Reviewed By: brecht

Maniphest Tasks: T87929

Differential Revision: https://developer.blender.org/D11219

intern/cycles/render/geometry.cpp

@@ -46,6 +46,12 @@ CCL_NAMESPACE_BEGIN
 
 /* Geometry */
 
+PackFlags operator |=(PackFlags &pack_flags, uint32_t value)
+{
+  pack_flags = (PackFlags)((uint32_t)pack_flags | value);
+  return pack_flags;
+}
+
 NODE_ABSTRACT_DEFINE(Geometry)
 {
   NodeType *type = NodeType::add("geometry_base", NULL);
@@ -1235,7 +1241,16 @@ void GeometryManager::device_update_bvh(Device *device,
 
   const bool can_refit = scene->bvh != nullptr &&
                          (bparams.bvh_layout == BVHLayout::BVH_LAYOUT_OPTIX);
-  const bool pack_all = scene->bvh == nullptr;
+
+  PackFlags pack_flags = PackFlags::PACK_NONE;
+
+  if (scene->bvh == nullptr) {
+    pack_flags |= PackFlags::PACK_ALL;
+  }
+
+  if (dscene->prim_visibility.is_modified()) {
+    pack_flags |= PackFlags::PACK_VISIBILITY;
+  }
 
   BVH *bvh = scene->bvh;
   if (!scene->bvh) {
@@ -1273,10 +1288,14 @@ void GeometryManager::device_update_bvh(Device *device,
 
     pack.root_index = -1;
 
-    if (!pack_all) {
+    if (pack_flags != PackFlags::PACK_ALL) {
       /* if we do not need to recreate the BVH, then only the vertices are updated, so we can
        * safely retake the memory */
       dscene->prim_tri_verts.give_data(pack.prim_tri_verts);
+
+      if ((pack_flags & PackFlags::PACK_VISIBILITY) != 0) {
+        dscene->prim_visibility.give_data(pack.prim_visibility);
+      }
     }
     else {
       /* It is not strictly necessary to skip those resizes we if do not have to repack, as the OS
@@ -1305,13 +1324,20 @@ void GeometryManager::device_update_bvh(Device *device,
     // Iterate over scene mesh list instead of objects, since 'optix_prim_offset' was calculated
     // based on that list, which may be ordered differently from the object list.
     foreach (Geometry *geom, scene->geometry) {
-      if (!pack_all && !geom->is_modified()) {
+      /* Make a copy of the pack_flags so the current geometry's flags do not pollute the others'. */
+      PackFlags geom_pack_flags = pack_flags;
+
+      if (geom->is_modified()) {
+        geom_pack_flags |= PackFlags::PACK_VERTICES;
+      }
+
+      if (geom_pack_flags == PACK_NONE) {
         continue;
       }
 
       const pair<int, uint> &info = geometry_to_object_info[geom];
       pool.push(function_bind(
-          &Geometry::pack_primitives, geom, &pack, info.first, info.second, pack_all));
+          &Geometry::pack_primitives, geom, &pack, info.first, info.second, geom_pack_flags));
     }
     pool.wait_work();
   }
@@ -1346,7 +1372,7 @@ void GeometryManager::device_update_bvh(Device *device,
     dscene->prim_type.steal_data(pack.prim_type);
     dscene->prim_type.copy_to_device();
   }
-  if (pack.prim_visibility.size() && (dscene->prim_visibility.need_realloc() || has_bvh2_layout)) {
+  if (pack.prim_visibility.size() && (dscene->prim_visibility.is_modified() || has_bvh2_layout)) {
     dscene->prim_visibility.steal_data(pack.prim_visibility);
     dscene->prim_visibility.copy_to_device();
   }
@@ -1594,6 +1620,10 @@ void GeometryManager::device_update_preprocess(Device *device, Scene *scene, Pro
     }
   }
 
+  if ((update_flags & VISIBILITY_MODIFIED) != 0) {
+    dscene->prim_visibility.tag_modified();
+  }
+
   if (device_update_flags & ATTR_FLOAT_NEEDS_REALLOC) {
     dscene->attributes_map.tag_realloc();
     dscene->attributes_float.tag_realloc();
@@ -1920,7 +1950,7 @@ void GeometryManager::device_update(Device *device,
    * Also update the BVH if the transformations change, we cannot rely on tagging the Geometry
    * as modified in this case, as we may accumulate displacement if the vertices do not also
    * change. */
-  bool need_update_scene_bvh = (scene->bvh == nullptr || (update_flags & TRANSFORM_MODIFIED) != 0);
+  bool need_update_scene_bvh = (scene->bvh == nullptr || (update_flags & (TRANSFORM_MODIFIED | VISIBILITY_MODIFIED)) != 0);
   {
     scoped_callback_timer timer([scene](double time) {
       if (scene->update_stats) {

intern/cycles/render/geometry.h

@@ -43,6 +43,24 @@ class Shader;
 class Volume;
 struct PackedBVH;
 
+/* Flags used to determine which geometry data need to be packed. */
+enum PackFlags : uint32_t {
+  PACK_NONE = 0u,
+
+  /* Pack the geometry information (e.g. triangle or curve keys indices). */
+  PACK_GEOMETRY = (1u << 0),
+
+  /* Pack the vertice, for Meshes and Volumes' bouding meshes. */
+  PACK_VERTICES = (1u << 1),
+
+  /* Pack the visibility flags for each triangle or curve. */
+  PACK_VISIBILITY = (1u << 2),
+
+  PACK_ALL = (PACK_GEOMETRY | PACK_VERTICES | PACK_VISIBILITY),
+};
+
+PackFlags operator |= (PackFlags &pack_flags, uint32_t value);
+
 /* Geometry
  *
  * Base class for geometric types like Mesh and Hair. */
@@ -126,7 +144,7 @@ class Geometry : public Node {
                    int n,
                    int total);
 
-  virtual void pack_primitives(PackedBVH *pack, int object, uint visibility, bool pack_all) = 0;
+  virtual void pack_primitives(PackedBVH *pack, int object, uint visibility, PackFlags pack_flags) = 0;
 
   /* Check whether the geometry should have own BVH built separately. Briefly,
    * own BVH is needed for geometry, if:
@@ -191,6 +209,8 @@ class GeometryManager {
 
     TRANSFORM_MODIFIED = (1 << 10),
 
+    VISIBILITY_MODIFIED = (1 << 11),
+
     /* tag everything in the manager for an update */
     UPDATE_ALL = ~0u,
 

intern/cycles/render/hair.cpp

@@ -494,38 +494,46 @@ void Hair::pack_curves(Scene *scene,
   }
 }
 
-void Hair::pack_primitives(PackedBVH *pack, int object, uint visibility, bool pack_all)
+void Hair::pack_primitives(PackedBVH *pack, int object, uint visibility, PackFlags pack_flags)
 {
   if (curve_first_key.empty())
     return;
 
-  /* If the BVH does not have to be recreated, we can bail out. */
-  if (!pack_all) {
-    return;
+  /* Separate loop as other arrays are not initialized if their packing is not required. */
+  if ((pack_flags & PACK_VISIBILITY) != 0) {
+    unsigned int *prim_visibility = &pack->prim_visibility[optix_prim_offset];
+
+    size_t index = 0;
+    for (size_t j = 0; j < num_curves(); ++j) {
+      Curve curve = get_curve(j);
+      for (size_t k = 0; k < curve.num_segments(); ++k, ++index) {
+        prim_visibility[index] = visibility;
+      }
+    }
   }
 
-  unsigned int *prim_tri_index = &pack->prim_tri_index[optix_prim_offset];
-  int *prim_type = &pack->prim_type[optix_prim_offset];
-  unsigned int *prim_visibility = &pack->prim_visibility[optix_prim_offset];
-  int *prim_index = &pack->prim_index[optix_prim_offset];
-  int *prim_object = &pack->prim_object[optix_prim_offset];
-  // 'pack->prim_time' is unused by Embree and OptiX
+  if ((pack_flags & PACK_GEOMETRY) != 0) {
+    unsigned int *prim_tri_index = &pack->prim_tri_index[optix_prim_offset];
+    int *prim_type = &pack->prim_type[optix_prim_offset];
+    int *prim_index = &pack->prim_index[optix_prim_offset];
+    int *prim_object = &pack->prim_object[optix_prim_offset];
+    // 'pack->prim_time' is unused by Embree and OptiX
 
-  uint type = has_motion_blur() ?
-                  ((curve_shape == CURVE_RIBBON) ? PRIMITIVE_MOTION_CURVE_RIBBON :
-                                                   PRIMITIVE_MOTION_CURVE_THICK) :
-                  ((curve_shape == CURVE_RIBBON) ? PRIMITIVE_CURVE_RIBBON : PRIMITIVE_CURVE_THICK);
+    uint type = has_motion_blur() ?
+                    ((curve_shape == CURVE_RIBBON) ? PRIMITIVE_MOTION_CURVE_RIBBON :
+                                                     PRIMITIVE_MOTION_CURVE_THICK) :
+                    ((curve_shape == CURVE_RIBBON) ? PRIMITIVE_CURVE_RIBBON : PRIMITIVE_CURVE_THICK);
 
-  size_t index = 0;
-  for (size_t j = 0; j < num_curves(); ++j) {
-    Curve curve = get_curve(j);
-    for (size_t k = 0; k < curve.num_segments(); ++k, ++index) {
-      prim_tri_index[index] = -1;
-      prim_type[index] = PRIMITIVE_PACK_SEGMENT(type, k);
-      prim_visibility[index] = visibility;
-      // Each curve segment points back to its curve index
-      prim_index[index] = j + prim_offset;
-      prim_object[index] = object;
+    size_t index = 0;
+    for (size_t j = 0; j < num_curves(); ++j) {
+      Curve curve = get_curve(j);
+      for (size_t k = 0; k < curve.num_segments(); ++k, ++index) {
+        prim_tri_index[index] = -1;
+        prim_type[index] = PRIMITIVE_PACK_SEGMENT(type, k);
+        // Each curve segment points back to its curve index
+        prim_index[index] = j + prim_offset;
+        prim_object[index] = object;
+      }
     }
   }
 }

intern/cycles/render/hair.h

@@ -146,7 +146,7 @@ class Hair : public Geometry {
   /* BVH */
   void pack_curves(Scene *scene, float4 *curve_key_co, float4 *curve_data, size_t curvekey_offset);
 
-  void pack_primitives(PackedBVH *pack, int object, uint visibility, bool pack_all) override;
+  void pack_primitives(PackedBVH *pack, int object, uint visibility, PackFlags pack_flags) override;
 };
 
 CCL_NAMESPACE_END

intern/cycles/render/mesh.cpp

@@ -805,7 +805,7 @@ void Mesh::pack_patches(uint *patch_data, uint vert_offset, uint face_offset, ui
   }
 }
 
-void Mesh::pack_primitives(ccl::PackedBVH *pack, int object, uint visibility, bool pack_all)
+void Mesh::pack_primitives(ccl::PackedBVH *pack, int object, uint visibility, PackFlags pack_flags)
 {
   if (triangles.empty())
     return;
@@ -819,28 +819,38 @@ void Mesh::pack_primitives(ccl::PackedBVH *pack, int object, uint visibility, bo
 
   uint type = has_motion_blur() ? PRIMITIVE_MOTION_TRIANGLE : PRIMITIVE_TRIANGLE;
 
-  if (pack_all) {
-    /* Use optix_prim_offset for indexing as those arrays also contain data for Hair geometries. */
-    unsigned int *prim_tri_index = &pack->prim_tri_index[optix_prim_offset];
-    int *prim_type = &pack->prim_type[optix_prim_offset];
+  /* Separate loop as other arrays are not initialized if their packing is not required. */
+  if ((pack_flags & PackFlags::PACK_VISIBILITY) != 0) {
     unsigned int *prim_visibility = &pack->prim_visibility[optix_prim_offset];
-    int *prim_index = &pack->prim_index[optix_prim_offset];
-    int *prim_object = &pack->prim_object[optix_prim_offset];
-
     for (size_t k = 0; k < num_prims; ++k) {
-      prim_tri_index[k] = (prim_offset + k) * 3;
-      prim_type[k] = type;
-      prim_index[k] = prim_offset + k;
-      prim_object[k] = object;
       prim_visibility[k] = visibility;
     }
   }
 
-  for (size_t k = 0; k < num_prims; ++k) {
-    const Mesh::Triangle t = get_triangle(k);
-    prim_tri_verts[k * 3] = float3_to_float4(verts[t.v[0]]);
-    prim_tri_verts[k * 3 + 1] = float3_to_float4(verts[t.v[1]]);
-    prim_tri_verts[k * 3 + 2] = float3_to_float4(verts[t.v[2]]);
+  if ((pack_flags & PackFlags::PACK_GEOMETRY) != 0) {
+    /* Use optix_prim_offset for indexing as those arrays also contain data for Hair geometries. */
+    unsigned int *prim_tri_index = &pack->prim_tri_index[optix_prim_offset];
+    int *prim_type = &pack->prim_type[optix_prim_offset];
+    int *prim_index = &pack->prim_index[optix_prim_offset];
+    int *prim_object = &pack->prim_object[optix_prim_offset];
+
+    for (size_t k = 0; k < num_prims; ++k) {
+      if ((pack_flags & PackFlags::PACK_GEOMETRY) != 0) {
+        prim_tri_index[k] = (prim_offset + k) * 3;
+        prim_type[k] = type;
+        prim_index[k] = prim_offset + k;
+        prim_object[k] = object;
+      }
+    }
+  }
+
+  if ((pack_flags & PackFlags::PACK_VERTICES) != 0) {
+    for (size_t k = 0; k < num_prims; ++k) {
+      const Mesh::Triangle t = get_triangle(k);
+      prim_tri_verts[k * 3] = float3_to_float4(verts[t.v[0]]);
+      prim_tri_verts[k * 3 + 1] = float3_to_float4(verts[t.v[1]]);
+      prim_tri_verts[k * 3 + 2] = float3_to_float4(verts[t.v[2]]);
+    }
   }
 }
 

intern/cycles/render/mesh.h

@@ -232,7 +232,7 @@ class Mesh : public Geometry {
                   size_t tri_offset);
   void pack_patches(uint *patch_data, uint vert_offset, uint face_offset, uint corner_offset);
 
-  void pack_primitives(PackedBVH *pack, int object, uint visibility, bool pack_all) override;
+  void pack_primitives(PackedBVH *pack, int object, uint visibility, PackFlags pack_flags) override;
 
   void tessellate(DiagSplit *split);
 

intern/cycles/render/object.cpp

@@ -220,6 +220,10 @@ void Object::tag_update(Scene *scene)
       flag |= ObjectManager::TRANSFORM_MODIFIED;
     }
 
+    if (visibility_is_modified()) {
+      flag |= ObjectManager::VISIBILITY_MODIFIED;
+    }
+
     foreach (Node *node, geometry->get_used_shaders()) {
       Shader *shader = static_cast<Shader *>(node);
       if (shader->get_use_mis() && shader->has_surface_emission)
@@ -914,6 +918,10 @@ void ObjectManager::tag_update(Scene *scene, uint32_t flag)
       geometry_flag |= GeometryManager::TRANSFORM_MODIFIED;
     }
 
+    if ((flag & VISIBILITY_MODIFIED) != 0) {
+      geometry_flag |= GeometryManager::VISIBILITY_MODIFIED;
+    }
+
     scene->geometry_manager->tag_update(scene, geometry_flag);
   }
 

intern/cycles/render/object.h

@@ -134,6 +134,7 @@ class ObjectManager {
     OBJECT_MODIFIED = (1 << 5),
     HOLDOUT_MODIFIED = (1 << 6),
     TRANSFORM_MODIFIED = (1 << 7),
+    VISIBILITY_MODIFIED = (1 << 8),
 
     /* tag everything in the manager for an update */
     UPDATE_ALL = ~0u,