Subdivision Surface modifier causes visual artifacts in Cycles rendered viewport - CPU and OptiX #83915

New Issue

Alaska · 2020-12-18T02:32:55+01:00

Alaska commented

2020-12-18 02:32:55 +01:00

System Information 1:
Operating system: Windows-10-10.0.19041-SP0 64 Bits
Central Processing Unit: Ryzen 9 3900X
Graphics card: RTX 2060 SUPER 460.79

System Information 2:
Operating system: Linux-5.8.0-3-amd64-x86_64-with-debian-bullseye-sid 64 Bits
Central Processing Unit: Ryzen 9 3900X
Graphics card: RTX 2060 SUPER 450.80.02

Blender Version:
Broken version: 2.92.0 Alpha, branch: master, commit date: 2020-12-17 20:43, hash: 23233fcf05
Worked: Prior to commit bfb6fce659

Short description of error:
When giving an object a Subdivision Surface modifier while in Cycles the rendered viewport, visual glitches will appear. I have been able to reproduce this with CPU and OptiX rendering. CUDA does not seem to be affected and I have not tested OpenCL. Sometimes Blender was crash while doing this. Here's an image of an example of the issue:


Expected results	Actual results


Expected results	Actual results

Exact steps for others to reproduce the error:

With the the default startup file, change the render engine to Cycles, set the render device to CPU or GPU Compute if you're using OptiX.
Add an extra object to the scene like a cube or a plane so you have two objects. The default cube and something else.
Enter rendered viewport mode, select one of the objects, and give it a Subdivision Surface modifier.
You should see a visual artifact like the images linked above. If you don't, maybe try increasing the Subdivision Surface modifier quality, changing the camera view, or giving a different object a Subdivision Surface modifier.

**System Information 1:** Operating system: Windows-10-10.0.19041-SP0 64 Bits Central Processing Unit: Ryzen 9 3900X Graphics card: RTX 2060 SUPER 460.79 **System Information 2:** Operating system: Linux-5.8.0-3-amd64-x86_64-with-debian-bullseye-sid 64 Bits Central Processing Unit: Ryzen 9 3900X Graphics card: RTX 2060 SUPER 450.80.02 **Blender Version:** Broken version: 2.92.0 Alpha, branch: master, commit date: 2020-12-17 20:43, hash: `23233fcf05` Worked: Prior to commit `bfb6fce659` **Short description of error:** When giving an object a `Subdivision Surface` modifier while in Cycles the rendered viewport, visual glitches will appear. I have been able to reproduce this with CPU and OptiX rendering. CUDA does not seem to be affected and I have not tested OpenCL. Sometimes Blender was crash while doing this. Here's an image of an example of the issue: |![Expected.jpg](https://archive.blender.org/developer/F9515943/Expected.jpg)|![Actual Results.jpg](https://archive.blender.org/developer/F9515945/Actual_Results.jpg)| | -- | -- | |Expected results|Actual results| |![Expected.jpg](https://archive.blender.org/developer/F9515939/Expected.jpg)|![Actual Results.jpg](https://archive.blender.org/developer/F9515937/Actual_Results.jpg)| | -- | -- | |Expected results|Actual results| **Exact steps for others to reproduce the error:** 1. With the the default startup file, change the render engine to `Cycles`, set the render device to `CPU` or `GPU Compute` if you're using `OptiX`. 2. Add an extra object to the scene like a cube or a plane so you have two objects. The default cube and something else. 3. Enter rendered viewport mode, select one of the objects, and give it a `Subdivision Surface` modifier. 4. You should see a visual artifact like the images linked above. If you don't, maybe try increasing the `Subdivision Surface` modifier quality, changing the camera view, or giving a different object a `Subdivision Surface` modifier.

Alaska commented

2020-12-18 02:32:55 +01:00

Added subscriber: @Alaska

Alaska commented

2020-12-18 03:21:54 +01:00

Added subscriber: @pmoursnv

Alaska commented

2020-12-18 03:21:54 +01:00

Bisecting points to bfb6fce659 as the first bad commit

CC @pmoursnv

Bisecting points to bfb6fce659 as the first bad commit CC @pmoursnv

Robert Guetzkow commented

2020-12-18 10:40:56 +01:00

Changed status from 'Needs Triage' to: 'Confirmed'

blender-admin commented

2021-01-05 14:39:29 +01:00

This issue was referenced by 166c0db3f9

This issue was referenced by 166c0db3f9412925b501b7172875cb8ee2eb6958

Patrick Mours commented

2021-01-05 15:01:28 +01:00

Added subscriber: @kevindietrich

Patrick Mours commented

2021-01-05 15:01:28 +01:00

This is the same issue @kevindietrich mentioned in D9555 #247586 where Geometry::optix_prim_offset changes are not propagated to the Embree/OptiX BVH.

Got a quick fix for this, but would prefer to have @kevindietrich 's opinion on it before committing:

diff --git a/intern/cycles/bvh/bvh_embree.cpp b/intern/cycles/bvh/bvh_embree.cpp
index b874bda718..c082478e5b 100644
--- a/intern/cycles/bvh/bvh_embree.cpp
+++ b/intern/cycles/bvh/bvh_embree.cpp
@@ -682,6 +682,7 @@ void BVHEmbree::refit(Progress &progress)
         if (mesh->num_triangles() > 0) {
           RTCGeometry geom = rtcGetGeometry(scene, geom_id);
           set_tri_vertex_buffer(geom, mesh, true);
+          rtcSetGeometryUserData(geom, (void *)mesh->optix_prim_offset);
           rtcCommitGeometry(geom);
         }
       }
@@ -690,6 +691,7 @@ void BVHEmbree::refit(Progress &progress)
         if (hair->num_curves() > 0) {
           RTCGeometry geom = rtcGetGeometry(scene, geom_id + 1);
           set_curve_vertex_buffer(geom, hair, true);
+          rtcSetGeometryUserData(geom, (void *)hair->optix_prim_offset);
           rtcCommitGeometry(geom);
         }
       }
diff --git a/intern/cycles/render/geometry.cpp b/intern/cycles/render/geometry.cpp
index 64b98a9185..f2c8e3f16d 100644
--- a/intern/cycles/render/geometry.cpp
+++ b/intern/cycles/render/geometry.cpp
@@ -915,7 +915,7 @@ void GeometryManager::device_update_attributes(Device *device,
   scene->object_manager->device_update_mesh_offsets(device, dscene, scene);
 }
 
-void GeometryManager::mesh_calc_offset(Scene *scene)
+void GeometryManager::mesh_calc_offset(Scene *scene, BVHLayout bvh_layout)
 {
   size_t vert_size = 0;
   size_t tri_size = 0;
@@ -930,6 +930,14 @@ void GeometryManager::mesh_calc_offset(Scene *scene)
   size_t optix_prim_size = 0;
 
   foreach (Geometry *geom, scene->geometry) {
+    if (geom->optix_prim_offset != optix_prim_size) {
+      /* Need to rebuild BVH in OptiX, since refit only allows modified mesh data there */
+      const bool has_optix_bvh = bvh_layout == BVH_LAYOUT_OPTIX ||
+                                 bvh_layout == BVH_LAYOUT_MULTI_OPTIX ||
+                                 bvh_layout == BVH_LAYOUT_MULTI_OPTIX_EMBREE;
+      geom->tag_update(scene, has_optix_bvh); /* TODO: This also marks the ObjectManager for update again ... */
+    }
+
     if (geom->geometry_type == Geometry::MESH || geom->geometry_type == Geometry::VOLUME) {
       Mesh *mesh = static_cast<Mesh *>(geom);
 
@@ -1526,7 +1534,9 @@ void GeometryManager::device_update(Device *device,
   /* Device update. */
   device_free(device, dscene);
 
-  mesh_calc_offset(scene);
+  const BVHLayout bvh_layout = BVHParams::best_bvh_layout(scene->params.bvh_layout,
+                                                          device->get_bvh_layout_mask());
+  mesh_calc_offset(scene, bvh_layout);
   if (true_displacement_used) {
     scoped_callback_timer timer([scene](double time) {
       if (scene->update_stats) {
@@ -1553,8 +1563,6 @@ void GeometryManager::device_update(Device *device,
   }
 
   /* Update displacement. */
-  BVHLayout bvh_layout = BVHParams::best_bvh_layout(scene->params.bvh_layout,
-                                                    device->get_bvh_layout_mask());
   bool displacement_done = false;
   size_t num_bvh = 0;
 
diff --git a/intern/cycles/render/geometry.h b/intern/cycles/render/geometry.h
index d3daf0cc80..5883e5cb7b 100644
--- a/intern/cycles/render/geometry.h
+++ b/intern/cycles/render/geometry.h
@@ -198,7 +198,7 @@ class GeometryManager {
                              vector<AttributeRequestSet> &object_attributes);
 
   /* Compute verts/triangles/curves offsets in global arrays. */
-  void mesh_calc_offset(Scene *scene);
+  void mesh_calc_offset(Scene *scene, BVHLayout bvh_layout);
 
   void device_update_object(Device *device, DeviceScene *dscene, Scene *scene, Progress &progress);

This is the same issue @kevindietrich mentioned in [D9555](https://archive.blender.org/developer/D9555)#247586 where `Geometry::optix_prim_offset` changes are not propagated to the Embree/OptiX BVH. Got a quick fix for this, but would prefer to have @kevindietrich 's opinion on it before committing: ``` diff --git a/intern/cycles/bvh/bvh_embree.cpp b/intern/cycles/bvh/bvh_embree.cpp index b874bda718..c082478e5b 100644 --- a/intern/cycles/bvh/bvh_embree.cpp +++ b/intern/cycles/bvh/bvh_embree.cpp @@ -682,6 +682,7 @@ void BVHEmbree::refit(Progress &progress) if (mesh->num_triangles() > 0) { RTCGeometry geom = rtcGetGeometry(scene, geom_id); set_tri_vertex_buffer(geom, mesh, true); + rtcSetGeometryUserData(geom, (void *)mesh->optix_prim_offset); rtcCommitGeometry(geom); } } @@ -690,6 +691,7 @@ void BVHEmbree::refit(Progress &progress) if (hair->num_curves() > 0) { RTCGeometry geom = rtcGetGeometry(scene, geom_id + 1); set_curve_vertex_buffer(geom, hair, true); + rtcSetGeometryUserData(geom, (void *)hair->optix_prim_offset); rtcCommitGeometry(geom); } } diff --git a/intern/cycles/render/geometry.cpp b/intern/cycles/render/geometry.cpp index 64b98a9185..f2c8e3f16d 100644 --- a/intern/cycles/render/geometry.cpp +++ b/intern/cycles/render/geometry.cpp @@ -915,7 +915,7 @@ void GeometryManager::device_update_attributes(Device *device, scene->object_manager->device_update_mesh_offsets(device, dscene, scene); } -void GeometryManager::mesh_calc_offset(Scene *scene) +void GeometryManager::mesh_calc_offset(Scene *scene, BVHLayout bvh_layout) { size_t vert_size = 0; size_t tri_size = 0; @@ -930,6 +930,14 @@ void GeometryManager::mesh_calc_offset(Scene *scene) size_t optix_prim_size = 0; foreach (Geometry *geom, scene->geometry) { + if (geom->optix_prim_offset != optix_prim_size) { + /* Need to rebuild BVH in OptiX, since refit only allows modified mesh data there */ + const bool has_optix_bvh = bvh_layout == BVH_LAYOUT_OPTIX || + bvh_layout == BVH_LAYOUT_MULTI_OPTIX || + bvh_layout == BVH_LAYOUT_MULTI_OPTIX_EMBREE; + geom->tag_update(scene, has_optix_bvh); /* TODO: This also marks the ObjectManager for update again ... */ + } + if (geom->geometry_type == Geometry::MESH || geom->geometry_type == Geometry::VOLUME) { Mesh *mesh = static_cast<Mesh *>(geom); @@ -1526,7 +1534,9 @@ void GeometryManager::device_update(Device *device, /* Device update. */ device_free(device, dscene); - mesh_calc_offset(scene); + const BVHLayout bvh_layout = BVHParams::best_bvh_layout(scene->params.bvh_layout, + device->get_bvh_layout_mask()); + mesh_calc_offset(scene, bvh_layout); if (true_displacement_used) { scoped_callback_timer timer([scene](double time) { if (scene->update_stats) { @@ -1553,8 +1563,6 @@ void GeometryManager::device_update(Device *device, } /* Update displacement. */ - BVHLayout bvh_layout = BVHParams::best_bvh_layout(scene->params.bvh_layout, - device->get_bvh_layout_mask()); bool displacement_done = false; size_t num_bvh = 0; diff --git a/intern/cycles/render/geometry.h b/intern/cycles/render/geometry.h index d3daf0cc80..5883e5cb7b 100644 --- a/intern/cycles/render/geometry.h +++ b/intern/cycles/render/geometry.h @@ -198,7 +198,7 @@ class GeometryManager { vector<AttributeRequestSet> &object_attributes); /* Compute verts/triangles/curves offsets in global arrays. */ - void mesh_calc_offset(Scene *scene); + void mesh_calc_offset(Scene *scene, BVHLayout bvh_layout); void device_update_object(Device *device, DeviceScene *dscene, Scene *scene, Progress &progress); ```

Kévin Dietrich commented

2021-01-05 17:09:10 +01:00

@pmoursnv, this seems reasonable. As for the TODO about retagging the ObjectManager for an update, I think this can be solved somehow with the update flags I added in D9555 so I can perhaps tackle this there (Just thinking, maybe the bool need_rebuild parameter to Geometry::tag_update could also be a flag instead of a boolean so we have better knowledge of what the tagging is for.) Another solution could be to have some private Geometry::tag_bvh_update method where we just tag the geometry as modified (Geometry::tag_modified) and also set need_update_rebuild if some OptiX BVH is used. Then we won't need to worry about tagging managers for extra updates.

@pmoursnv, this seems reasonable. As for the TODO about retagging the ObjectManager for an update, I think this can be solved somehow with the update flags I added in [D9555](https://archive.blender.org/developer/D9555) so I can perhaps tackle this there (Just thinking, maybe the `bool need_rebuild` parameter to `Geometry::tag_update` could also be a flag instead of a boolean so we have better knowledge of what the tagging is for.) Another solution could be to have some private `Geometry::tag_bvh_update` method where we just tag the geometry as modified (`Geometry::tag_modified`) and also set `need_update_rebuild` if some OptiX BVH is used. Then we won't need to worry about tagging managers for extra updates.

Patrick Mours commented

2021-01-05 17:36:55 +01:00

Sounds good. I think I'll just go with a Geometry::tag_bvh_update routine for now and we can then later see if it can be simplified with the update flags when that gets merged.

Sounds good. I think I'll just go with a `Geometry::tag_bvh_update` routine for now and we can then later see if it can be simplified with the update flags when that gets merged.

Patrick Mours commented

2021-01-05 18:01:44 +01:00

Changed status from 'Confirmed' to: 'Resolved'

Patrick Mours closed this issue

2021-01-05 18:01:44 +01:00

Patrick Mours self-assigned this 2021-01-05 18:01:44 +01:00

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Subdivision Surface modifier causes visual artifacts in Cycles rendered viewport - CPU and OptiX #83915