Fix T92815: Incorrect handling of evaluated meshes from curves
ClosedPublic
Actions

Authored by Hans Goudey (HooglyBoogly) on Nov 4 2021, 9:38 PM.

Details

Reviewers

Brecht Van Lommel (brecht)
Jacques Lucke (JacquesLucke)

Maniphest Tasks

T92815: Geometry Nodes "Set Material" and Cuve to Mesh

Commits

rB3211c80a3110: Fix T92815: Incorrect handling of evaluated meshes from curves
rB1bc655c5aa64: Fix T92815: Incorrect handling of evaluated meshes from curves

Summary

Evaluated meshes from curves are presented to render engines as
separate instance objects now, just like evaluated meshes from other
object types like point clouds and volumes. For that reason, cycles
should not consider curve objects as geometry (previously it did,
meaning it retrieved a second mesh from the curve object as well
as the temporary evaluated mesh geometry).

Further, avoid adding a curve object's evaluated mesh as data_eval,
since that is special behavior for meshes that is arbitrary. Adding an
evaluated mesh there but not an evalauted pointcloud is arbitrary,
for example. Retrieve the evaluated mesh in from the geometry set
in BKE_object_get_evaluated_mesh now, to support that change.

This gets us closer to a place where all of an object's evaluated data
is stored in geometry_set_eval, and we just have helper functions
to access specific geometry components.

Diff Detail

Repository: rB Blender

Event Timeline

Hans Goudey (HooglyBoogly) requested review of this revision.Nov 4 2021, 9:38 PM

Hans Goudey (HooglyBoogly) created this revision.

Harbormaster completed remote builds in B18487: Diff 44388.Nov 4 2021, 9:38 PM

Hans Goudey (HooglyBoogly) added a task: T92815: Geometry Nodes "Set Material" and Cuve to Mesh.Nov 4 2021, 9:38 PM

Hans Goudey (HooglyBoogly) edited the summary of this revision. (Show Details)

Hans Goudey (HooglyBoogly) added reviewers: Brecht Van Lommel (brecht), Jacques Lucke (JacquesLucke).

Hans Goudey (HooglyBoogly) added projects: Geometry Nodes, Render & Cycles.

Hans Goudey (HooglyBoogly) edited the summary of this revision. (Show Details)

Hans Goudey (HooglyBoogly) mentioned this in D13112: Fix T92815: Wrong curve rendering in cycles (WIP)..Nov 4 2021, 9:41 PM

Tests are running here: https://builder.blender.org/admin/#/builders/18/builds/158

I think this is the more correct fix. The current state of the patch breaks the modifier test. This can be fixed with the following patch, which seems reasonable:

diff --git a/source/blender/blenkernel/intern/DerivedMesh.cc b/source/blender/blenkernel/intern/DerivedMesh.cc
index 4b1332d5b84..96b1ea2323c 100644
--- a/source/blender/blenkernel/intern/DerivedMesh.cc
+++ b/source/blender/blenkernel/intern/DerivedMesh.cc
@@ -1896,9 +1896,9 @@ static void mesh_build_data(struct Depsgraph *depsgraph,
   const bool is_mesh_eval_owned = (mesh_eval != mesh->runtime.mesh_eval);
   BKE_object_eval_assign_data(ob, &mesh_eval->id, is_mesh_eval_owned);
 
-  /* Add the final mesh as read-only non-owning component to the geometry set. */
+  /* Add the final mesh as non-owning component to the geometry set. */
   MeshComponent &mesh_component = geometry_set_eval->get_component_for_write<MeshComponent>();
-  mesh_component.replace(mesh_eval, GeometryOwnershipType::ReadOnly);
+  mesh_component.replace(mesh_eval, GeometryOwnershipType::Editable);
   ob->runtime.geometry_set_eval = geometry_set_eval;
 
   ob->runtime.mesh_deform_eval = mesh_deform_eval;

This patch also works well together with D12738 even though long term we may want to get rid of data_eval because it does not have much meaning when objects can evaluate to multiple geometries.

Jacques Lucke (JacquesLucke) requested changes to this revision.Nov 5 2021, 12:05 PM

This revision now requires changes to proceed.Nov 5 2021, 12:05 PM

Yes, using editable for the evaluated mesh makes sense. I'm guessing it was being copied when something asked for write access, leading to invalid pointers somewhere.