Goal is to add the length attribute to the Hair Info node, for better control over color gradients or similar along the hair.
Eevee Render
Cycles Render (CPU)
Cycles Render (GPU)
Test-File:
Details
Details
Diff Detail
Diff Detail
- Repository
- rB Blender
- Branch
- cycles_hair_info_length (branched from master)
- Build Status
Buildable 13029 Build 13029: arc lint + arc unit
Event Timeline
There are a very large number of changes, so older changes are hidden. Show Older Changes
Comment Actions
It is my understanding that Cycles handles the data as vertex attributes, which is created during the generation of the hair particle meshes for rendering. I also think exposing this data to geometry nodes shouldn't be a problem, since adding the attribute to the object would automatically populate it during the next mesh generation. But dont quote me on this as i never looked at geometry nodes.
I think handling as a vertex attribute is what @Clément Foucault (fclem) has in mind for this for eevee too, hence the gpu_attribute() but i cant quite figure out how the eevee attribute system is working and where to put my checks and how. The system doesnt allow me to use attributes the same simple way cycles does, but instead ties in very tightly with the shader codegen, which makes this more or less impossible for me to understand, as there are to many aspects of the eevee rendering code that i would need to understand first. And since i never really touched rendering code on this level before (my first time working with blender source, and my first time C as well, since i only really worked with C++) i'm not sure what exactly im doin here :D
| source/blender/nodes/shader/nodes/node_shader_hair_info.c | ||
|---|---|---|
| 42 | The variadic parameters do not math the node_hair_info parameters. return GPU_stack_link(
mat, node, "node_hair_info", in, out, NULL, NULL, length_link, NULL, NULL, NULL);NOTE: unsure if NULL works as a dummy.
But still comment above seems still valid. for CD_ORCO a different code path is taken, similar changes seems to be needed. To continue you need to add exceptions to the GPU_codegen.c if you need help with that we can have a chat online. | |
Comment Actions
- Implemented codegen exceptions for CD_HAIRLENGTH and got it generating correctly thus far except for error where hair_len_get() isnt found.
Comment Actions
I am currently experiencing this bug caused by the codegen. Fixing this should in case my buffer allocation of C side of things is correct fix this issue for now.
I am currently experiencing this bug caused by the codegen. Fixing this should in case my buffer allocation of C side of things is correct fix this issue for now.
Comment Actions
- Fix: Codegen for hairlength now includes common defines. This fixes the codegen.
- Currently the data isnt passed to the shader
Comment Actions
- Added CD_MAKS_HAIRLENGTH and made sure buffer texture gets created for old and new hair.
- Assignment to uniform buffer currently doesnt work but could be due to internal handling of attrbiutes as texture isnt found in interface.
Comment Actions
- Cleanup: Removed old includes, lines that have to purpose anymore and restructured code to fit in better with other code style
| source/blender/gpu/shaders/material/gpu_shader_material_hair_info.glsl | ||
|---|---|---|
| 23 | Not sure how to access the attrubute data here. | |
| source/blender/draw/intern/draw_hair.c | ||
|---|---|---|
| 312 ↗ | (On Diff #41271) | @Jeroen Bakker (jbakker) This assignment is currently not working. I think this is because i dont access the length buffer in the shader correctly causing that part of the code to be optimized away. I am not sure if settings the buffer values directly from C is the right approach here as we already use the GPU_attribute system but it is unclear to me how to access it in this way. |
Comment Actions
Current patch doesn't compile for me.
/home/jeroen/blender-git/blender/source/blender/draw/intern/draw_cache_impl_hair.c: In function ‘hair_batch_cache_ensure_procedural_pos’:
/home/jeroen/blender-git/blender/source/blender/draw/intern/draw_cache_impl_hair.c:198:3: error: too many arguments to function ‘GPU_vertbuf_data_alloc’
198 | GPU_vertbuf_data_alloc(cache->proc_length_buf, pos_id, &point_step);
| ^~~~~~~~~~~~~~~~~~~~~~
In file included from /home/jeroen/blender-git/blender/source/blender/gpu/GPU_batch.h:33,
from /home/jeroen/blender-git/blender/source/blender/draw/intern/draw_cache_impl_hair.c:39:
/home/jeroen/blender-git/blender/source/blender/gpu/GPU_vertex_buffer.h:96:6: note: declared here
96 | void GPU_vertbuf_data_alloc(GPUVertBuf *, uint v_len);Comment Actions
- Merge remote-tracking branch 'origin/master' into arcpatch-D10481
- Fix: Wrong invocation of GPU_vertbuf_data_alloc()
Comment Actions
It seems like the buffer containing the hairLengths isn't attached to the program. On my system it fallsback and reads the first element of the hairPointBuffer.
The var0 column should be the hairLen. Next image shows the content of the hairPointBuffer.
Do you know the application renderdoc? it can help finding these issues. When using you can start blender inside renderdoc with the --debug-gpu option
There seems also be a memory leak:
GLVertBuf len: 504 0x7fe17c2944b8 GPUNodeLink len: 24 0x7fe179bfdd78 GPUNodeLink len: 24 0x7fe17a0eaa18
Think the issue might be that the texture isn't created.
| source/blender/draw/intern/draw_cache_impl_particles.c | ||
|---|---|---|
| 638 ↗ | (On Diff #41337) | Should also be done when total_len == 0. If not the hair length buffer will not match the hair id. |
| source/blender/draw/intern/draw_hair.c | ||
| 312 ↗ | (On Diff #41271) | It cannot find the binding. The binding index of hairLen = -1. reason for this is that the GLSL compiler has optimized this buffer out as it wasn't read at all. In openGL this can also happen when you assign it to a variable you pass to the fragment shader and it ain't used there. You can see this as the loc attribute in DRW_shgroup_uniform_texture_ex is -1. In the relevant shader the next interface is defined. Uniforms :
| fd8df7d0 : 3090 : hairRadTip
| ec1498e0 : 3091 : hairRadShape
| e6ad2fa5 : 3087 : hairStrandsRes
| d71413ca : 3088 : hairThicknessRes
| a96da4db : 5628 : outputSssId
| a96d9f82 : 5627 : outputSsrId
| a58365bb : 3089 : hairRadRoot
| a3e676b7 : 3093 : hairDupliMatrix
| 239e9af3 : 3092 : hairCloseTip
| 0194cb27 : 3097 : hairStrandOffset
Uniform Buffer Objects :
| fb58bc97 : 4 : common_block
| ea4c9ac4 : 6 : shadow_block
| df88bc1c : 0 : modelBlock
| d50570da : 9 : probe_block
| 9a719e18 : 10 : planar_block
| 949ce768 : 3 : grid_block
| 70a0f3ff : 1 : infoBlock
| 57548381 : 7 : renderpass_block
| 4de6622a : 8 : nodeTree
| 0cf29056 : 5 : light_block
| 06086e0a : 2 : viewBlock
Samplers :
| d03be8bb : 1 : utilTex
| a3a76dac : 0 : hairPointBufferThe node_hair_info reads from ShaderHairInterface::hairLength. but the vertex shader stored the hairLength in ShaderStageInterface::var0. Best solution is to store it the hair length in shader hair interface at the same time the other attributes are stored there (check for hair_get_pos_tan_binor_time) |
| source/blender/draw/intern/draw_cache_impl_hair.c | ||
|---|---|---|
| 167 ↗ | (On Diff #41337) | Should be done even if total_len = 0.0 otherwise indexes will not match. |
Comment Actions
- Total length will be added to verbuf also when total_len == 0
- Working version of hair length eevee shader.
| source/blender/draw/engines/eevee/shaders/surface_vert.glsl | ||
|---|---|---|
| 65 ↗ | (On Diff #41535) | Is there a better way to do this than with defines through GPU_codegen.c? |
| source/blender/draw/intern/draw_cache_impl_hair.c | ||
| 210 ↗ | (On Diff #41535) | The proc_length_buf and texture will always be alllocated on the GPU defeating the purpose of using GPU_attribute. Same at draw_cache_impl_particles.c line 1134. @Jeroen Bakker (jbakker) @Clément Foucault (fclem) Are there examples of how to do this? |
Comment Actions
Detected a memory leak
Error: Not freed memory blocks: 2, total unfreed memory 0.000046 MB GPUNodeLink len: 24 0x7f9116615e38 GPUNodeLink len: 24 0x7f910d1b8f58
| source/blender/draw/intern/draw_cache_impl_hair.c | ||
|---|---|---|
| 210 ↗ | (On Diff #41535) | Not sure if it uploaded to the GPU. the VBO is known by the driver, but when not used and optimized away it will not be uploaded to the GPU. Yes there is an overhead. But think it is ok for now. Another solution is to pack the hair length with the hair time. (eg compress it as 2 F16s inside a single F32). To reduce the overhead. but that could be an optimization afterwards. |
| source/blender/nodes/shader/nodes/node_shader_hair_info.c | ||
|---|---|---|
| 41 | Memory leak happens here. Direct leak of 32 byte(s) in 1 object(s) allocated from:
#0 0x7f07c7b7fe17 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
#1 0x244669c0 in MEM_lockfree_callocN /home/jeroen/blender-git/blender/intern/guardedalloc/intern/mallocn_lockfree_impl.c:236
#2 0x2267d64a in gpu_node_link_create /home/jeroen/blender-git/blender/source/blender/gpu/intern/gpu_node_graph.c:47
#3 0x226829f7 in GPU_attribute /home/jeroen/blender-git/blender/source/blender/gpu/intern/gpu_node_graph.c:500
#4 0x774c4a0 in node_shader_gpu_hair_info /home/jeroen/blender-git/blender/source/blender/nodes/shader/nodes/node_shader_hair_info.c:41
#5 0x7947a69 in ntreeExecGPUNodes /home/jeroen/blender-git/blender/source/blender/nodes/shader/node_shader_util.c:274
#6 0x79433fe in ntreeGPUMaterialNodes /home/jeroen/blender-git/blender/source/blender/nodes/shader/node_shader_tree.c:936
#7 0x2266b69f in GPU_material_from_nodetree /home/jeroen/blender-git/blender/source/blender/gpu/intern/gpu_material.c:711
#8 0x65cbd62 in DRW_shader_create_from_material /home/jeroen/blender-git/blender/source/blender/draw/intern/draw_manager_shader.c:523
#9 0x63c29c3 in eevee_material_get_ex /home/jeroen/blender-git/blender/source/blender/draw/engines/eevee/eevee_shaders.c:1479
#10 0x63c33fe in EEVEE_material_get /home/jeroen/blender-git/blender/source/blender/draw/engines/eevee/eevee_shaders.c:1517
#11 0x638c383 in material_opaque /home/jeroen/blender-git/blender/source/blender/draw/engines/eevee/eevee_materials.c:598
#12 0x638fd54 in eevee_material_cache_get /home/jeroen/blender-git/blender/source/blender/draw/engines/eevee/eevee_materials.c:755
#13 0x638fd54 in EEVEE_materials_cache_populate /home/jeroen/blender-git/blender/source/blender/draw/engines/eevee/eevee_materials.c:827
#14 0x63394e3 in EEVEE_cache_populate /home/jeroen/blender-git/blender/source/blender/draw/engines/eevee/eevee_engine.c:126
#15 0x62cc25c in drw_engines_cache_populate /home/jeroen/blender-git/blender/source/blender/draw/intern/draw_manager.c:1015
#16 0x62d104a in DRW_draw_render_loop_ex /home/jeroen/blender-git/blender/source/blender/draw/intern/draw_manager.c:1536
#17 0x62d00c6 in DRW_draw_view /home/jeroen/blender-git/blender/source/blender/draw/intern/draw_manager.c:1443
#18 0xab0a6ec in view3d_draw_view /home/jeroen/blender-git/blender/source/blender/editors/space_view3d/view3d_draw.c:1565
#19 0xab0a875 in view3d_main_region_draw /home/jeroen/blender-git/blender/source/blender/editors/space_view3d/view3d_draw.c:1587
#20 0x8735dfb in ED_region_do_draw /home/jeroen/blender-git/blender/source/blender/editors/screen/area.c:564
#21 0x56b9984 in wm_draw_window_offscreen /home/jeroen/blender-git/blender/source/blender/windowmanager/intern/wm_draw.c:728
#22 0x56babaa in wm_draw_window /home/jeroen/blender-git/blender/source/blender/windowmanager/intern/wm_draw.c:877
#23 0x56bc019 in wm_draw_update /home/jeroen/blender-git/blender/source/blender/windowmanager/intern/wm_draw.c:1078
#24 0x56ac0a2 in WM_main /home/jeroen/blender-git/blender/source/blender/windowmanager/intern/wm.c:654
#25 0x38e2a69 in main /home/jeroen/blender-git/blender/source/creator/creator.c:558
#26 0x7f07c7595564 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x28564)
Direct leak of 32 byte(s) in 1 object(s) allocated from:
#0 0x7f07c7b7fe17 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
#1 0x244669c0 in MEM_lockfree_callocN /home/jeroen/blender-git/blender/intern/guardedalloc/intern/mallocn_lockfree_impl.c:236
#2 0x2267d64a in gpu_node_link_create /home/jeroen/blender-git/blender/source/blender/gpu/intern/gpu_node_graph.c:47
#3 0x226829f7 in GPU_attribute /home/jeroen/blender-git/blender/source/blender/gpu/intern/gpu_node_graph.c:500
#4 0x774c4a0 in node_shader_gpu_hair_info /home/jeroen/blender-git/blender/source/blender/nodes/shader/nodes/node_shader_hair_info.c:41
#5 0x7947a69 in ntreeExecGPUNodes /home/jeroen/blender-git/blender/source/blender/nodes/shader/node_shader_util.c:274
#6 0x79433fe in ntreeGPUMaterialNodes /home/jeroen/blender-git/blender/source/blender/nodes/shader/node_shader_tree.c:936
#7 0x2266b69f in GPU_material_from_nodetree /home/jeroen/blender-git/blender/source/blender/gpu/intern/gpu_material.c:711
#8 0x65cbd62 in DRW_shader_create_from_material /home/jeroen/blender-git/blender/source/blender/draw/intern/draw_manager_shader.c:523
#9 0x63c29c3 in eevee_material_get_ex /home/jeroen/blender-git/blender/source/blender/draw/engines/eevee/eevee_shaders.c:1479
#10 0x63c33fe in EEVEE_material_get /home/jeroen/blender-git/blender/source/blender/draw/engines/eevee/eevee_shaders.c:1517
#11 0x638c383 in material_opaque /home/jeroen/blender-git/blender/source/blender/draw/engines/eevee/eevee_materials.c:598
#12 0x638ed1e in eevee_material_cache_get /home/jeroen/blender-git/blender/source/blender/draw/engines/eevee/eevee_materials.c:755
#13 0x638ed1e in eevee_hair_cache_populate /home/jeroen/blender-git/blender/source/blender/draw/engines/eevee/eevee_materials.c:769
#14 0x6391fb9 in EEVEE_particle_hair_cache_populate /home/jeroen/blender-git/blender/source/blender/draw/engines/eevee/eevee_materials.c:919
#15 0x633928e in EEVEE_cache_populate /home/jeroen/blender-git/blender/source/blender/draw/engines/eevee/eevee_engine.c:121
#16 0x62cc25c in drw_engines_cache_populate /home/jeroen/blender-git/blender/source/blender/draw/intern/draw_manager.c:1015
#17 0x62d104a in DRW_draw_render_loop_ex /home/jeroen/blender-git/blender/source/blender/draw/intern/draw_manager.c:1536
#18 0x62d00c6 in DRW_draw_view /home/jeroen/blender-git/blender/source/blender/draw/intern/draw_manager.c:1443
#19 0xab0a6ec in view3d_draw_view /home/jeroen/blender-git/blender/source/blender/editors/space_view3d/view3d_draw.c:1565
#20 0xab0a875 in view3d_main_region_draw /home/jeroen/blender-git/blender/source/blender/editors/space_view3d/view3d_draw.c:1587
#21 0x8735dfb in ED_region_do_draw /home/jeroen/blender-git/blender/source/blender/editors/screen/area.c:564
#22 0x56b9984 in wm_draw_window_offscreen /home/jeroen/blender-git/blender/source/blender/windowmanager/intern/wm_draw.c:728
#23 0x56babaa in wm_draw_window /home/jeroen/blender-git/blender/source/blender/windowmanager/intern/wm_draw.c:877
#24 0x56bc019 in wm_draw_update /home/jeroen/blender-git/blender/source/blender/windowmanager/intern/wm_draw.c:1078
#25 0x56ac0a2 in WM_main /home/jeroen/blender-git/blender/source/blender/windowmanager/intern/wm.c:654
#26 0x38e2a69 in main /home/jeroen/blender-git/blender/source/creator/creator.c:558
#27 0x7f07c7595564 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x28564) | |
| source/blender/draw/intern/draw_cache_impl_hair.c | ||
|---|---|---|
| 210 ↗ | (On Diff #41535) | The way you would do this is by having access to the GPUMaterial in this function. But doing so would require a bit more code change on EEVEE & workbench to pass it down to there. For workbench just pass NULL as GPUMaterial *. So the internal of the hair_batch_cache should expect NULL material. |
Comment Actions
- Implemented working gpu attribute check for hair rendering length attribute
CPU side data will always be allocated together with the proc_point_buf to simplify things and the texture only created and uploaded once needed.
@Clément Foucault (fclem) Does this fit what you had in mind?
Comment Actions
This is exactly what I had in mind. If there is no more memory leak, I think we are good to go.
Note that I did not test the patch personally yet.
| source/blender/draw/intern/draw_cache_impl_hair.c | ||
|---|---|---|
| 215 ↗ | (On Diff #41696) | You may want to break out of the loop after this line. |
Comment Actions
@Clément Foucault (fclem) The memory leak @Jeroen Bakker (jbakker) found is still not dealt with and i also dont know what is causing it as i am, to my knowledge, not doing anything differently than elsewhere concerning the handling of gpu attributes. Do you have thoughts on this?
Comment Actions
- Breaking after CD_HAIRLENGTH Attribute is found
- Merge remote-tracking branch 'origin/master' into arcpatch-D10481
Comment Actions
Memory leak:
In node_shader_gpu_hair_info the length_link is created and passed to the GPU_stack_link. The GPU_stack_link cannot find any input parameter and doesn't link it to the graph. So the object isn't used in the final GPU shader.
A solution is to add an input parameter to node_hair_info this input will contain a link to the length_link.
void node_hair_info(
float hair_length
out float is_strand,
out float intercept,
out float length,
out float thickness,
out vec3 tangent,
out float random)Internally the hair_length parameter is directed to the length out parameter.
return GPU_stack_link(
mat, node, "node_hair_info", in, out, length_link);This might need some additional tweaks so the generate the correct link in the fragment shader. (Currently hardcoded to hairLength)
Comment Actions
--- a/source/blender/gpu/shaders/material/gpu_shader_material_hair_info.glsl +++ b/source/blender/gpu/shaders/material/gpu_shader_material_hair_info.glsl @@ -10,7 +10,8 @@ float wang_hash_noise(uint s) return fract(float(s) / 4294967296.0); } -void node_hair_info(out float is_strand, +void node_hair_info(float hair_length, + out float is_strand, out float intercept, out float length, out float thickness, @@ -20,7 +21,7 @@ void node_hair_info(out float is_strand, #ifdef HAIR_SHADER is_strand = 1.0; intercept = hairTime; - length = hairLength; + length = hair_length; thickness = hairThickness; tangent = normalize(worldNormal); random = wang_hash_noise( diff --git a/source/blender/nodes/shader/nodes/node_shader_hair_info.c b/source/blender/nodes/shader/nodes/node_shader_hair_info.c index 79ec174a553..776e3e0d2f9 100644 --- a/source/blender/nodes/shader/nodes/node_shader_hair_info.c +++ b/source/blender/nodes/shader/nodes/node_shader_hair_info.c @@ -39,7 +39,7 @@ static int node_shader_gpu_hair_info(GPUMaterial *mat, /* Length: don't request length if not needed. */ GPUNodeLink *length_link = (!out[2].hasoutput) ? GPU_constant(0) : GPU_attribute(mat, CD_HAIRLENGTH, ""); - return GPU_stack_link(mat, node, "node_hair_info", in, out, NULL, NULL, &length_link, NULL, NULL, NULL); + return GPU_stack_link(mat, node, "node_hair_info", in, out, length_link); } /* node type definition */
Seems to remove the memory leak. I haven't checked if the generated code is optimal.
Comment Actions
- Fix: Fixed memory leak and finally using GPU attribute as intended
@Jeroen Bakker (jbakker) @Clément Foucault (fclem) This should be it! Memory leak should be fixed and i removed all the redundant code changes i could spot.
Comment Actions
- FIX: using GPU_constant caused crash cause data pointer was 0 instead of actuall value.
Comment Actions
Just a minor code style.
I tested and LGTM.
@Brecht Van Lommel (brecht) can you approve the cycles part.
| source/blender/nodes/shader/nodes/node_shader_hair_info.c | ||
|---|---|---|
| 40 | ||