Code cleanup and move common functions and consts to separate files #2
Conversation
|
Appreciate that! This project is under a time sensitive develop status, some refactor job always delayed... |
gameknife
left a comment
gameknife
left a comment
There was a problem hiding this comment.
a small bug when refactor the visibility buffer fetch:
I offset the instance_index in order to leave the default zero value to sky, but the get_material_index function has a slightly different, check the 2 comments.
thanks
| Vertex get_material_data(ivec2 pixel, uvec2 vBuffer, vec3 ray_origin, vec3 ray_direction) | ||
| { | ||
| // fetch primitive_index high 14bit as instance index and low 18bit as triangle index | ||
| uint instance_index = vBuffer.x - 1; |
There was a problem hiding this comment.
do not -1 here, this leads the hybrid renderer bugs with instance offset
remove it here, and vbuffer.x - 1 outside
There was a problem hiding this comment.
assets/shaders/common/VertexFunc.glsl
line 6 must be
uint instance_index = vBuffer.x;
?
assets/shaders/ModernDeferredShading.comp
line 48-50 is
uvec2 vBuffer = imageLoad(MiniGBuffer, ipos).rg;
vec2 uv = vec2(ipos) / vec2(size) * 2.0 - 1.0;
vec4 origin = Camera.ModelViewInverse * vec4(0, 0, 0, 1);
please read code of my last commit. I fixed some code
There was a problem hiding this comment.
if you start with --renderer=4, select hybrid renderer, the indirect shadow missing
the two get_material_data function before this change had a different:
the one in HybridShading is uint instance_index = vBuffer.x;
the one in ModernShading is uint instance_index = vBuffer.x - 1;
the fix I mentioned above will fix it
There was a problem hiding this comment.
vBuffer.x = vBuffer.x - 1; after line 48
|
I will check and fix. Please wait |
|
done. please check. I use only RayTracingRenderer |
|
PR accepted, 🤝 |
2 participants
I dont understand why you use hardcoded consts like 0.5f / 180.f * 3.14159f, 45.f / 180.f * 3.14159f and vec3(0.0072f, 1.0f, 0.0034f).
Maybe better to use it as consts?