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

Closed
opened 2 years ago by dfelinto · 14 comments
Owner

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

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.
Poster
Owner

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

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

Added subscriber: @dfelinto

Added subscriber: @dfelinto
zanqdo commented 2 years ago
Collaborator

Added subscriber: @zanqdo

Added subscriber: @zanqdo
Collaborator

Added subscriber: @JacquesLucke

Added subscriber: @JacquesLucke
Collaborator

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).
  1. 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 commented 2 years ago
Owner

Added subscriber: @brecht

Added subscriber: @brecht
brecht commented 2 years ago
Owner

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?
Collaborator

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 commented 2 years ago
Owner

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.
Collaborator

This issue was referenced by efb741b280

This issue was referenced by efb741b280f20cb189e23f2b1335358a95ab609c
Collaborator

This issue was referenced by 732d0b458b

This issue was referenced by 732d0b458b6f9024b285747a643cacb128888b8c
Collaborator

This issue was referenced by 9ee7270e0a

This issue was referenced by 9ee7270e0acc6d60c8eac9e25cca00aa384c0879
Collaborator

This issue was referenced by 0d58eabee6

This issue was referenced by 0d58eabee620cc534fab075764a83f5a328100c1
Poster
Owner

Changed status from 'Confirmed' to: 'Resolved'

Changed status from 'Confirmed' to: 'Resolved'
dfelinto closed this issue 2 years ago
dfelinto self-assigned this 2 years ago
Sign in to join this conversation.
No Label
Interest/Alembic
Interest/Animation & Rigging
Interest/Asset Browser
Interest/Asset Browser Project Overview
Interest/Audio
Interest/Automated Testing
Interest/Blender Asset Bundle
Interest/Collada
Interest/Compositing
Interest/Core
Interest/Cycles
Interest/Dependency Graph
Interest/Development Management
Interest/Eevee & Viewport
Interest/Freestyle
Interest/Geometry Nodes
Interest/Grease Pencil
Interest/ID Management
Interest/Images & Movies
Interest/Import/Export
Interest/Line Art
Interest/Masking
Interest/Modeling
Interest/Modifiers
Interest/Motion Tracking
Interest/Nodes & Physics
Interest/Overrides
Interest/Performance
Interest/Performance
Interest/Physics
Interest/Pipeline, Assets & I/O
Interest/Platforms, Builds, Tests & Devices
Interest/Python API
Interest/Render & Cycles
Interest/Render Pipeline
Interest/Sculpt, Paint & Texture
Interest/Text Editor
Interest/Translations
Interest/Triaging
Interest/Undo
Interest/USD
Interest/User Interface
Interest/UV Editing
Interest/VFX & Video
Interest/Video Sequencer
Interest/Virtual Reality
legacy module/Animation & Rigging
legacy module/Core
legacy module/Eevee & Viewport
legacy module/Grease Pencil
legacy module/Modeling
legacy module/Nodes & Physics
legacy module/Pipeline, Assets & IO
legacy module/Platforms, Builds, Tests & Devices
legacy module/Python API
legacy module/Sculpt, Paint & Texture
legacy module/User Interface
legacy module/VFX & Video
legacy project/BF Blender: 2.8
legacy project/Milestone 1: Basic, Local Asset Browser
legacy project/OpenGL Error
legacy project/Retrospective
Meta/Good First Issue
Meta/Papercut
migration/requires-manual-verification
Module › Animation & Rigging
Module › Core
Module › Development Management
Module › Eevee & Viewport
Module › Grease Pencil
Module › Modeling
Module › Nodes & Physics
Module › Pipeline, Assets & IO
Module › Platforms, Builds, Tests & Devices
Module › Python API
Module › Render & Cycles
Module › Sculpt, Paint & Texture
Module › Triaging
Module › User Interface
Module › VFX & Video
Platform/FreeBSD
Platform/Linux
Platform/macOS
Platform/Windows
Priority › High
Priority › Low
Priority › Normal
Priority › Unbreak Now!
Status › Archived
Status › Confirmed
Status › Duplicate
Status › Needs Information from Developers
Status › Needs Information from User
Status › Needs Triage
Status › Resolved
Type › Bug
Type › Design
Type › Known Issue
Type › Patch
Type › Report
Type › To Do
No Milestone
No project
No Assignees
5 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#83357
Loading…
There is no content yet.