Design to support instancing (GeometrySet) for mesh objects #83357

New Issue

Dalai Felinto · 2020-12-03T12:33:50+01:00

Dalai Felinto commented

2020-12-03 12:33:50 +01:00

Problem: Any geometry node that outputs non-mesh data doesn't work if the modified object is of Mesh type.

The goal of this task is to create a few implementation tasks that make supporting this doable in time for 2.92 release.

For example, it is helps to get this solved sooner, it is acceptable for instances to only be supported if the modifier is the last modifier of the stack. They can be converted to real data otherwise.

Technical Explanation:

Before the geometry nodes, the modifier only took a mesh, volume, ... and would output the same type of data. For geometry node we now have a new datastruct called GeometrySet that can handle:

Point Cloud
Instance
Geometry

To support instancing and points for mesh objects we need to change data_eval and use geometry_set_eval everywhere. Those are properties of the Object_Runtime struct:

Object_Runtime
-> geometry_set_eval (can handle advanced data)
-> data_eval (assumes a single data type)

Note: The Point Distribute node can be changed to output regular mesh points. So the real challenge here is only to support instances.

Note 2: This also allows Point Cloud objects to be hidden under experimental once again.

**Problem**: Any geometry node that outputs non-mesh data doesn't work if the modified object is of Mesh type. **Test File**: [mesh_support.blend](https://archive.blender.org/developer/F9432633/mesh_support.blend) The goal of this task is to create a few implementation tasks that make supporting this doable in time for 2.92 release. For example, it is helps to get this solved sooner, it is acceptable for instances to only be supported if the modifier is the last modifier of the stack. They can be converted to real data otherwise. **Technical Explanation**: Before the geometry nodes, the modifier only took a mesh, volume, ... and would output the same type of data. For geometry node we now have a new datastruct called GeometrySet that can handle: * Point Cloud * Instance * Geometry To support instancing and points for mesh objects we need to change data_eval and use geometry_set_eval everywhere. Those are properties of the Object_Runtime struct: ``` Object_Runtime -> geometry_set_eval (can handle advanced data) -> data_eval (assumes a single data type) ``` Note: The Point Distribute node can be changed to output regular mesh points. So the real challenge here is only to support instances. Note 2: This also allows Point Cloud objects to be hidden under experimental once again.

Dalai Felinto commented

2020-12-03 12:33:50 +01:00

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

Dalai Felinto commented

2020-12-03 12:33:50 +01:00

Added subscriber: @dfelinto

Daniel Salazar commented

2020-12-03 14:27:25 +01:00

Added subscriber: @zanqdo

Jacques Lucke commented

2020-12-08 16:35:38 +01:00

Added subscriber: @JacquesLucke

Jacques Lucke commented

2020-12-08 16:35:38 +01:00

The following design seems to work fairly well and checks the following boxes:

The amount of changes required for our use case is quite small (can easily be reviewed in a single patch).
object->runtime.data_eval does not have to be removed (we might want to remove it eventually, but it's not necessary).

The main idea is to change the "definition" of object->runtime.data_eval as follows:
Old: Data created during object evaluation with all modifiers applied.
New: The part of the data created during object evaluation, that existing Blender code expects (e.g. for a mesh object data_eval will be a mesh; for a volume object it will be a volume; for a curve object it will be a mesh as well).

Note, this change in definition does not have any impact on existing objects "by definition". Therefore, "the part of the data ..." mentioned in the new definition, is actually "all of of the data ..." for those objects.
Furthermore, there is a new object->runtime.geometry_set_eval field. This field contains all the data that has been created during object evaluation, including the data that is also referenced by data_eval.

Implementing this is fairly straight forward. There are only two main places that have to be changed slightly.

Modifier evaluation on mesh objects:

Currently, the loop that iterates over the modifiers updates a `Mesh *mesh_final` variable for every modifier (roughly speaking). In addition to that, it will also maintain a new `geometry_set_final` variable. For almost all modifiers, the new `geometry_set_final` variable is not touched. Only when there is a modifier that supports the `modifyGeometrySet`callback, the following happens:

The current state of mesh_final is added to the geometry set (other geometry components might be there already).
modifyGeometrySet is called with the geometry set.
The mesh component is released from the geometry set and stored in mesh_final again (other components remain in geometry_set_final).


After all modifiers are evaluated, `mesh_final` is assigned to `object->runtime.data_eval` (as it is already). Furthermore, `mesh_final` is added as read-only mesh component to the `geometry_set` (it's not owned by the geometry set). Then the resulting geometry set is assigned to `object->runtime.geometry_set_eval`. 
For modifier evaluation in edit mode (in `editbmesh_build_data`) almost the same happens, except that the final mesh is assigned to `object->data->edit_mesh->mesh_eval_final` instead of `object->runtime.data_eval` for some reason that I don't know yet (this is in existing code).

Depsgraph object iterator: This can almost stay as it is in master now. The main change is that not only point cloud object supports geometry_set_eval now.

A initial version of these changes can be found in the temp-geometry-nodes-mesh-modifier branch (I used this branch just for testing different idea, I intend to rewrite it from scratch in a few days).

These are some more smaller non-functional changes that should be done as well:

Move DerivedMesh.c to C++, so that the GeometrySet data structure becomes more comfortable to use.
Rename ModifierTypeInfo->modifyPointCloud to ModifierTypeInfo->modifyGeometrySet.

There is one issue that is not easily solved with this. It is not required to be solved for our use case, but would be good to solve eventually. Currently, a the type of a modifier has to be defined at compile time (e.g. whether it only deforms, is constructive other supports mapping). In the context of the geometry nodes modifier, these things are not known at compile time (and might not even always be known at run-time). In the long term, we will probably have to rethink how modifiers specify what they can and cannot do.

Here is a nice short video, that shows these changes in action. Notice how the geometry nodes modifier is added to a mesh object and the instancing still works (only worked on pointcloud objects before).
It should be fairly simple to extend this solution to work on other object types (e.g. curves and volumes). Modifiers on those object types could also change the geometry type of the output dynamically (e.g. a volume object becoming a mesh without a separate mesh object).

2020-12-08 13-16-07.mp4

The following design seems to work fairly well and checks the following boxes: * The amount of changes required for our use case is quite small (can easily be reviewed in a single patch). * `object->runtime.data_eval` does not have to be removed (we might want to remove it eventually, but it's not necessary). ------- The main idea is to change the "definition" of `object->runtime.data_eval` as follows: Old: Data created during object evaluation with all modifiers applied. New: The part of the data created during object evaluation, that existing Blender code expects (e.g. for a mesh object `data_eval` will be a mesh; for a volume object it will be a volume; for a curve object it will be a mesh as well). Note, this change in definition does not have any impact on existing objects "by definition". Therefore, "the part of the data ..." mentioned in the new definition, is actually "all of of the data ..." for those objects. Furthermore, there is a new `object->runtime.geometry_set_eval` field. This field contains all the data that has been created during object evaluation, including the data that is also referenced by `data_eval`. ------- Implementing this is fairly straight forward. There are only two main places that have to be changed slightly. 1. Modifier evaluation on mesh objects: ``` Currently, the loop that iterates over the modifiers updates a `Mesh *mesh_final` variable for every modifier (roughly speaking). In addition to that, it will also maintain a new `geometry_set_final` variable. For almost all modifiers, the new `geometry_set_final` variable is not touched. Only when there is a modifier that supports the `modifyGeometrySet`callback, the following happens: ``` 1. The current state of `mesh_final` is added to the geometry set (other geometry components might be there already). 2. `modifyGeometrySet` is called with the geometry set. 3. The mesh component is released from the geometry set and stored in `mesh_final` again (other components remain in `geometry_set_final`). ``` After all modifiers are evaluated, `mesh_final` is assigned to `object->runtime.data_eval` (as it is already). Furthermore, `mesh_final` is added as read-only mesh component to the `geometry_set` (it's not owned by the geometry set). Then the resulting geometry set is assigned to `object->runtime.geometry_set_eval`. For modifier evaluation in edit mode (in `editbmesh_build_data`) almost the same happens, except that the final mesh is assigned to `object->data->edit_mesh->mesh_eval_final` instead of `object->runtime.data_eval` for some reason that I don't know yet (this is in existing code). ``` 2. Depsgraph object iterator: This can almost stay as it is in master now. The main change is that not only point cloud object supports `geometry_set_eval` now. A initial version of these changes can be found in the `temp-geometry-nodes-mesh-modifier` branch (I used this branch just for testing different idea, I intend to rewrite it from scratch in a few days). --- These are some more smaller non-functional changes that should be done as well: * Move `DerivedMesh.c` to C++, so that the `GeometrySet` data structure becomes more comfortable to use. * Rename `ModifierTypeInfo->modifyPointCloud` to `ModifierTypeInfo->modifyGeometrySet`. There is one issue that is not easily solved with this. It is not required to be solved for our use case, but would be good to solve eventually. Currently, a the type of a modifier has to be defined at compile time (e.g. whether it only deforms, is constructive other supports mapping). In the context of the geometry nodes modifier, these things are not known at compile time (and might not even always be known at run-time). In the long term, we will probably have to rethink how modifiers specify what they can and cannot do. ------- Here is a nice short video, that shows these changes in action. Notice how the geometry nodes modifier is added to a mesh object and the instancing still works (only worked on pointcloud objects before). It should be fairly simple to extend this solution to work on other object types (e.g. curves and volumes). Modifiers on those object types could also change the geometry type of the output dynamically (e.g. a volume object becoming a mesh without a separate mesh object). [2020-12-08 13-16-07.mp4](https://archive.blender.org/developer/F9494322/2020-12-08_13-16-07.mp4)

Brecht Van Lommel commented

2020-12-08 19:18:08 +01:00

Added subscriber: @brecht

Brecht Van Lommel commented

2020-12-08 19:18:08 +01:00

This all sounds reasonable, it's in line with what we discussed before.

In general, what is the expected behavior of modifiers/nodes on a geometry set? I guess it's applying the specified operation on all compatible geometries? So ideally we should run mesh modifier on all meshes in the geometry set. I'm not sure how practical it is though, exceptions would likely be needed for e.g. subsurf/multires, physics modifiers, mesh deform, ... that do some kind of caching or storage for specific meshes. These could eventually be supported as well, but it's not simple. Best left for a future release I guess.
Applying a modifier that generates a geometry set will either be (initially) unsupported, or would show a warning about losing some geometry set objects, or (ideally?) create new objects rather than just modifying the existing one.
For cage editing in edit mode, it would be nice if there was some heuristic so that it still works with geometry nodes. Maybe just picking the first mesh from the geometry set as data_eval is good enough in practice?

This all sounds reasonable, it's in line with what we discussed before. * In general, what is the expected behavior of modifiers/nodes on a geometry set? I guess it's applying the specified operation on all compatible geometries? So ideally we should run mesh modifier on all meshes in the geometry set. I'm not sure how practical it is though, exceptions would likely be needed for e.g. subsurf/multires, physics modifiers, mesh deform, ... that do some kind of caching or storage for specific meshes. These could eventually be supported as well, but it's not simple. Best left for a future release I guess. * Applying a modifier that generates a geometry set will either be (initially) unsupported, or would show a warning about losing some geometry set objects, or (ideally?) create new objects rather than just modifying the existing one. * For cage editing in edit mode, it would be nice if there was some heuristic so that it still works with geometry nodes. Maybe just picking the first mesh from the geometry set as `data_eval` is good enough in practice?

Jacques Lucke commented

2020-12-08 19:42:40 +01:00

In general, what is the expected behavior of modifiers/nodes on a geometry set? I guess it's applying the specified operation on all compatible geometries? So ideally we should run mesh modifier on all meshes in the geometry set.

Since a geometry set only contains a single mesh currently, this is already working I believe. Whether we will always only have a single mesh component in a geometry set (from the user perspective) is still a bit up to debate, there are pros and cons. Internally we could do whatever we want to avoid unnecessary computations (e.g. avoid joining meshes until necessary). This topic is also related to how we want to handle instances. Lately, the opinion seems to become that instancing should be an implementation detail in geometry nodes (that is what I got from Dalai's summary of his meeting with Ton).

Applying a modifier that generates a geometry set will either be (initially) unsupported, or would show a warning about losing some geometry set objects, or (ideally?) create new objects rather than just modifying the existing one.

Right, the apply operator can be updated to use geometry_set_eval instead of data_eval as well. Until then, we should show warning. I also think that it should create new objects of different types, but we haven't made an official decision on that yet.

For cage editing in edit mode, it would be nice if there was some heuristic so that it still works with geometry nodes. Maybe just picking the first mesh from the geometry set as data_eval is good enough in practice?

Again, since currently there is only a single mesh in a geometry set, this is not really a question. Using data_eval is good enough in all cases. If we get multiple meshes in a geometry set eventually, such a heuristic is necessary indeed.

> In general, what is the expected behavior of modifiers/nodes on a geometry set? I guess it's applying the specified operation on all compatible geometries? So ideally we should run mesh modifier on all meshes in the geometry set. Since a geometry set only contains a single mesh currently, this is already working I believe. Whether we will always only have a single mesh component in a geometry set (from the user perspective) is still a bit up to debate, there are pros and cons. Internally we could do whatever we want to avoid unnecessary computations (e.g. avoid joining meshes until necessary). This topic is also related to how we want to handle instances. Lately, the opinion seems to become that instancing should be an implementation detail in geometry nodes (that is what I got from Dalai's summary of his meeting with Ton). > Applying a modifier that generates a geometry set will either be (initially) unsupported, or would show a warning about losing some geometry set objects, or (ideally?) create new objects rather than just modifying the existing one. Right, the apply operator can be updated to use `geometry_set_eval` instead of `data_eval` as well. Until then, we should show warning. I also think that it should create new objects of different types, but we haven't made an official decision on that yet. > For cage editing in edit mode, it would be nice if there was some heuristic so that it still works with geometry nodes. Maybe just picking the first mesh from the geometry set as data_eval is good enough in practice? Again, since currently there is only a single mesh in a geometry set, this is not really a question. Using `data_eval` is good enough in all cases. If we get multiple meshes in a geometry set eventually, such a heuristic is necessary indeed.

Brecht Van Lommel commented

2020-12-09 18:30:12 +01:00

In #83357#1071304, @JacquesLucke wrote:
Since a geometry set only contains a single mesh currently, this is already working I believe. Whether we will always only have a single mesh component in a geometry set (from the user perspective) is still a bit up to debate, there are pros and cons. Internally we could do whatever we want to avoid unnecessary computations (e.g. avoid joining meshes until necessary).

That indeed makes things simpler simple. I guess the question is still how it might handle e.g. hair, nurbs, volumes for modifiers that can apply to any geometry. But we can ignore those for now I think.

This topic is also related to how we want to handle instances. Lately, the opinion seems to become that instancing should be an implementation detail in geometry nodes (that is what I got from Dalai's summary of his meeting with Ton).

Ok, my guess would be to not make it an implementation detail, but either way can work.

Not sure about the policy in general. If we have mesh, point clouds, curves, nurbs, hair, volumes, and instances, ... it's not so obvious which automatic conversions make sense.

> In #83357#1071304, @JacquesLucke wrote: > Since a geometry set only contains a single mesh currently, this is already working I believe. Whether we will always only have a single mesh component in a geometry set (from the user perspective) is still a bit up to debate, there are pros and cons. Internally we could do whatever we want to avoid unnecessary computations (e.g. avoid joining meshes until necessary). That indeed makes things simpler simple. I guess the question is still how it might handle e.g. hair, nurbs, volumes for modifiers that can apply to any geometry. But we can ignore those for now I think. > This topic is also related to how we want to handle instances. Lately, the opinion seems to become that instancing should be an implementation detail in geometry nodes (that is what I got from Dalai's summary of his meeting with Ton). Ok, my guess would be to not make it an implementation detail, but either way can work. Not sure about the policy in general. If we have mesh, point clouds, curves, nurbs, hair, volumes, and instances, ... it's not so obvious which automatic conversions make sense.

blender-admin commented

2020-12-10 14:35:02 +01:00

This issue was referenced by efb741b280

This issue was referenced by efb741b280f20cb189e23f2b1335358a95ab609c

blender-admin commented

2020-12-14 13:00:28 +01:00

This issue was referenced by 732d0b458b

This issue was referenced by 732d0b458b6f9024b285747a643cacb128888b8c

blender-admin commented

2020-12-14 15:28:24 +01:00

This issue was referenced by 9ee7270e0a

This issue was referenced by 9ee7270e0acc6d60c8eac9e25cca00aa384c0879