Geometry Nodes: Support rendering generated attributes. #85075

Closed
opened 2021-01-26 12:21:19 +01:00 by Jacques Lucke · 29 comments
Member

This can be used as a parent task for e.g. tasks specific to Cycles and Eevee.
I started looking into this a bit.

The Cycles implementation seems to be relative straight forward, I'll try that.

The Eevee implementation seems more tricky, because DRW_MeshCDMask is not general enough.

This can be used as a parent task for e.g. tasks specific to Cycles and Eevee. I started looking into this a bit. The Cycles implementation seems to be relative straight forward, I'll try that. The Eevee implementation seems more tricky, because `DRW_MeshCDMask` is not general enough.
Author
Member

Added subscriber: @JacquesLucke

Added subscriber: @JacquesLucke
Author
Member

Added subscribers: @brecht, @fclem

Added subscribers: @brecht, @fclem

Added subscriber: @victorlouis

Added subscriber: @victorlouis

Added subscriber: @MiroHorvath

Added subscriber: @MiroHorvath

Added subscriber: @randum

Added subscriber: @randum
Member

Added subscriber: @SimonThommes

Added subscriber: @SimonThommes

This issue was referenced by blender/cycles@a484a197ea

This issue was referenced by blender/cycles@a484a197eadf8373382bef1a7b80ceef83ba0dc8

This issue was referenced by 3a6d6299d7

This issue was referenced by 3a6d6299d7a0f2a6a156bce6731a0b29804d911d

Added subscriber: @hyyou

Added subscriber: @hyyou

Added subscriber: @EricM

Added subscriber: @EricM

Added subscriber: @VDC

Added subscriber: @VDC

Added subscriber: @dfelinto

Added subscriber: @dfelinto
Kévin Dietrich self-assigned this 2021-10-13 13:09:52 +02:00

After looking at the issue I can say that DRW_MeshCDMask will not be enough. To me we have to treat the generic attribs as a special case. For this to work, we need a list of index (uint? ushorts? uchar?) or a bitmap that identifies the needed generic attributes inside the mesh. The issue is that modifying this needs to be threadsafe. Having a per Mesh mutex might be ok for now.

Note that the threadsafety is needed for future per object multithreading but it is not actually done at the moment. So you will not be able to test it.

mesh_cd_calc_used_gpu_layers is the function that matches the needed attribute name requested by the shader to a generic attribute.

After looking at the issue I can say that `DRW_MeshCDMask` will not be enough. To me we have to treat the generic attribs as a special case. For this to work, we need a list of index (uint? ushorts? uchar?) or a bitmap that identifies the needed generic attributes inside the mesh. The issue is that modifying this needs to be threadsafe. Having a per Mesh mutex might be ok for now. Note that the threadsafety is needed for future per object multithreading but it is not actually done at the moment. So you will not be able to test it. `mesh_cd_calc_used_gpu_layers` is the function that matches the needed attribute name requested by the shader to a generic attribute.

Actually I already started implementing this (I added a few extra members to DRW_MeshCDMask as a start, but that will change). Attributes can be on different domains (or in terms of CustomData: cd_vdata, cd_ldata, cd_pdata, etc.). So we need to keep track of the index, but also the domain. So that would mean a list or bitmap per domain. I'll try to figure something out.

Also I am currently using a different VBO for each attribute type which could allow to optimize memory a bit in the future, although that might be premature. All attributes are by default vec4 in the shaders (I think it is due to the color output of the attribute node, and to simplify things?), so that's what I have to use to build VBOs, which is a waste for float and float2.

Actually I already started implementing this (I added a few extra members to `DRW_MeshCDMask` as a start, but that will change). Attributes can be on different domains (or in terms of `CustomData`: `cd_vdata`, `cd_ldata`, `cd_pdata`, etc.). So we need to keep track of the index, but also the domain. So that would mean a list or bitmap per domain. I'll try to figure something out. Also I am currently using a different VBO for each attribute type which could allow to optimize memory a bit in the future, although that might be premature. All attributes are by default `vec4` in the shaders (I think it is due to the color output of the attribute node, and to simplify things?), so that's what I have to use to build VBOs, which is a waste for `float` and `float2`.

Attributes can be declared vec4 in the shader but only have float or vec2 input from the VBO. OpenGL will convert by adding the extra components using the vector (0,0,0,1).

There is also a limit on the number of VBO a GPU batch can contain. I don't remember the implication of increasing this maximum (GPU_BATCH_VBO_MAX_LEN), but it would be nice if we could just have one VBO per attrib and start less worrying about interlaced vertex layout or packing attribs together.

If you profile what I propose and if the overhead is reasonnable, I would be happy to set GPU_BATCH_VBO_MAX_LEN to 16.

Attributes can be declared `vec4` in the shader but only have `float` or `vec2` input from the VBO. OpenGL will convert by adding the extra components using the vector `(0,0,0,1)`. There is also a limit on the number of VBO a GPU batch can contain. I don't remember the implication of increasing this maximum (`GPU_BATCH_VBO_MAX_LEN`), but it would be nice if we could just have one VBO per attrib and start less worrying about interlaced vertex layout or packing attribs together. If you profile what I propose and if the overhead is reasonnable, I would be happy to set `GPU_BATCH_VBO_MAX_LEN` to 16.

How do handle color attributes though? They are using CD_PROP_COLOR which is also used by sculpt vertex colors, which is behind an experimental feature flag, but also merged with regular vertex colors in the vcol buffer. Although this is not a supported feature yet, Alembic (via D11591) and USD in the future may import data as CD_PROP_COLOR.

How do handle color attributes though? They are using `CD_PROP_COLOR` which is also used by sculpt vertex colors, which is behind an experimental feature flag, but also merged with regular vertex colors in the `vcol` buffer. Although this is not a supported feature yet, Alembic (via [D11591](https://archive.blender.org/developer/D11591)) and USD in the future may import data as `CD_PROP_COLOR`.

For sculpt vertex colors, the reason they are behind a feature flag in the code is because the current implementation of that only works with either old or new vertex colors. D12587 has work towards making both work.

For the purpose of this I think they could be handled like any other attribute? And then later there could be some way to unify handling of vertex colors and general attributes, to make vertex colors just a special case of attributes.

For sculpt vertex colors, the reason they are behind a feature flag in the code is because the current implementation of that only works with either old or new vertex colors. [D12587](https://archive.blender.org/developer/D12587) has work towards making both work. For the purpose of this I think they could be handled like any other attribute? And then later there could be some way to unify handling of vertex colors and general attributes, to make vertex colors just a special case of attributes.
Author
Member

Is the work happening on this in some branch?

Is the work happening on this in some branch?

Part of it was added in the tmp-abc-features branch that I pushed to make some experimental builds, rest is sill local, but will be either pushed there for new test builds, or sent to code review, or both.

Part of it was added in the `tmp-abc-features` branch that I pushed to make some experimental builds, rest is sill local, but will be either pushed there for new test builds, or sent to code review, or both.

Added subscriber: @RC12

Added subscriber: @RC12
Member

Added subscriber: @HooglyBoogly

Added subscriber: @HooglyBoogly

Added subscriber: @zNight

Added subscriber: @zNight

I removed the sculpt vertex colors form DRW_MeshCDMask so they are handled with the other attributes.


I wrote a script to generate random attributes with random types on random domains to stress test the code. So far it is pretty stable, all domains and types seem to work.

However, the limit of 15 attributes (from GPU_MAX_ATTR) does not take into account other attributes like normals. So with the default cube with a Principled BSDF, I can only add 14 attributes, then shader compilation fails and it just renders purple. Apart from a message in the console, there is nothing much to help debugging for users. Can we improve this a bit, like adding a proper UI message? I don't know if shader compilation can easily take all attributes into consideration. That's orthogonal to the patch, so not important.

I ran two versions of the code in a profile build using the aforementioned script to generate 100 objects. One version of the code is for one VBO per attribute, the other is one packed VBO per data type (like uvs or vcols currently). The objects are randomly selected from the list of default mesh objects (cube, UV sphere, monkey, etc.), as the idea was to measure the overhead of having one VBO per attribute vs one packed VBO for each type, so the size of the data does not really matter. The shaders are also randomized a bit to avoid having a single shader/ShadingGroup. Essentially, the shaders are made of Attribute nodes mixed together using MixRGB nodes where the blend type is randomized (then the result is plugged in the Principled BSDF node diffuse input).

Having one VBO per attribute has some overhead indeed, here are some screenshots from HotSpot, with 100 objects, we can see that GLBatch::draw is getting chunky:

Packed VBOs on first run (with shader compilation, not shown):
typed_vbos.png

Packed VBOs on second run (after going out of and back in rendered mode):
typed_vbos_after_compilation.png

Separate VBOs on first run (with shader compilation, not shown):
per_attrib_vbos.png

Separate VBOs on second run (after going out of and back in rendered mode):
per_attrib_vbos_after_compilation.png

Here is the .blend file: random_geo_nodes_attributes.blend

I did not time it in a release build, so I do not have numbers, but by the looks of things, it seems that only initialization is much costlier. As it stands, I am using a compile time definition to switch between the two versions of the code, so depending on what we choose, I can simply remove the other.

I removed the sculpt vertex colors form `DRW_MeshCDMask` so they are handled with the other attributes. ------- I wrote a script to generate random attributes with random types on random domains to stress test the code. So far it is pretty stable, all domains and types seem to work. However, the limit of 15 attributes (from `GPU_MAX_ATTR`) does not take into account other attributes like normals. So with the default cube with a Principled BSDF, I can only add 14 attributes, then shader compilation fails and it just renders purple. Apart from a message in the console, there is nothing much to help debugging for users. Can we improve this a bit, like adding a proper UI message? I don't know if shader compilation can easily take all attributes into consideration. That's orthogonal to the patch, so not important. I ran two versions of the code in a profile build using the aforementioned script to generate 100 objects. One version of the code is for one VBO per attribute, the other is one packed VBO per data type (like uvs or vcols currently). The objects are randomly selected from the list of default mesh objects (cube, UV sphere, monkey, etc.), as the idea was to measure the overhead of having one VBO per attribute vs one packed VBO for each type, so the size of the data does not really matter. The shaders are also randomized a bit to avoid having a single shader/ShadingGroup. Essentially, the shaders are made of Attribute nodes mixed together using MixRGB nodes where the blend type is randomized (then the result is plugged in the Principled BSDF node diffuse input). Having one VBO per attribute has some overhead indeed, here are some screenshots from HotSpot, with 100 objects, we can see that `GLBatch::draw` is getting chunky: Packed VBOs on first run (with shader compilation, not shown): ![typed_vbos.png](https://archive.blender.org/developer/F11371407/typed_vbos.png) Packed VBOs on second run (after going out of and back in rendered mode): ![typed_vbos_after_compilation.png](https://archive.blender.org/developer/F11371419/typed_vbos_after_compilation.png) Separate VBOs on first run (with shader compilation, not shown): ![per_attrib_vbos.png](https://archive.blender.org/developer/F11371426/per_attrib_vbos.png) Separate VBOs on second run (after going out of and back in rendered mode): ![per_attrib_vbos_after_compilation.png](https://archive.blender.org/developer/F11371480/per_attrib_vbos_after_compilation.png) Here is the .blend file: [random_geo_nodes_attributes.blend](https://archive.blender.org/developer/F11371652/random_geo_nodes_attributes.blend) I did not time it in a release build, so I do not have numbers, but by the looks of things, it seems that only initialization is much costlier. As it stands, I am using a compile time definition to switch between the two versions of the code, so depending on what we choose, I can simply remove the other.

@kevindietrich hi, if the alembic changes are not aimed at 3.0, can you separate the code early on and have in its own branch so we can try it? (or even if they are aimed at 3.0, it would be helpful to look at this issue/feature isolated from alembic changes).

@kevindietrich hi, if the alembic changes are not aimed at 3.0, can you separate the code early on and have in its own branch so we can try it? (or even if they are aimed at 3.0, it would be helpful to look at this issue/feature isolated from alembic changes).

Added subscriber: @Bradley_G

Added subscriber: @Bradley_G

This issue was referenced by 03013d19d1

This issue was referenced by 03013d19d16704672f9db93bc62547651b6a5cb8

This issue was referenced by 3e3ff1a464

This issue was referenced by 3e3ff1a464b93c39cf31f30ef11b87e5b5786735
Author
Member

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

Changed status from 'Needs Triage' to: 'Resolved'
Author
Member

Cycles and Eevee now support attribute rendering.

Cycles and Eevee now support attribute rendering.
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
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
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
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
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
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
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
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 Info 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
17 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#85075
No description provided.