-
Notifications
You must be signed in to change notification settings - Fork 781
Raytracing execution reordering #1461
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Raytracing execution reordering #1461
Conversation
…vocation_reorder extension.
…shadow mapping, ambient occlusion, flame particles, and dynamic object support
…reorder to support older Vulkan SDK versions
|
Does not start for me. I get an error about missing glsl shaders: [error] Error Message: Failed to open file for reading at path: shaders/ray_tracing_invocation_reorder/glsl/raygen.rgen.spv There are some .spv files in that folder, but not for all shaders. Can you add the missing ones? Also fails when selecting slang, but with another file in the error: [error] Error Message: Failed to open file for reading at path: shaders/ray_tracing_invocation_reorder/slang/closesthit_normal.rchit.spv |
|
Ahh... that must be a left over bug. One of the comments mentioned that GLSL might not be capable of outputting the requisite spv. So I switched to slang and apparently didn't fix the code path. Hang on a sec and I'll update it. |
|
Oh, nevermind. I'm an idiot. Wrong PR. Yes, there's some missing shaders, committing now. |
asuessenbach
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, I don't have HW supporting VK_RAY_TRACING_INVOCATION_REORDER_MODE_REORDER_NV available.
Besides that, and a couple of mostly minor issues, it looks great.
| // We read/write a storage image without specifying a format in the shader (untyped image) | ||
| // so we must enable these core device features. | ||
| gpu.get_mutable_requested_features().shaderStorageImageReadWithoutFormat = VK_TRUE; | ||
| gpu.get_mutable_requested_features().shaderStorageImageWriteWithoutFormat = VK_TRUE; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should check for support of those features first.
| auto normal = normalize(pt + camera.position); | ||
| if (freeze_z) | ||
| { | ||
| normal = normalize(abs(dot(normal, vec3{0, 1, 0})) > 0.99f ? vec3{0, 0, 1} : vec3{normal.x, 0.f, normal.z}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need to normalize (0,0,1), so maybe
normal = abs(dot(normal, vec3{0, 1, 0})) > 0.99f ? vec3{0, 0, 1} : normalize(vec3{normal.x, 0.f, normal.z});
| assign_buffer(new_buffer); | ||
| raytracing_scene->model_buffers.emplace_back(std::move(new_buffer)); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe instead:
auto bufferIt = std::ranges::find_if( raytracing_scene->model_buffers, [](const ModelBuffer &buffer) { return buffer.object_type == ObjectType::OBJECT_REFRACTION; });
if (bufferIt != raytracing_scene->model_buffers.end())
{
assign_buffer(*bufferIt);
}
else
{
ModelBuffer new_buffer;
assign_buffer(new_buffer);
raytracing_scene->model_buffers.emplace_back(std::move(new_buffer));
}
| RENDER_GLOBAL_XYZ = 4, | ||
| RENDER_SHADOW_MAP = 5, | ||
| RENDER_AO = 6 | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This RenderMode enum is used just once in create_ray_tracing_pipeline, with RENDER_DEFAULT.
Do you intend to extend this sample to support all those RenderModes?
| OBJECT_NORMAL, // has AO and ray traced shadows | ||
| OBJECT_REFRACTION, // pass-through with IOR | ||
| OBJECT_FLAME // emission surface; constant amplitude | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe make this an enum class, like
enum class ObjectType : uint32_t
{
NORMAL, // has AO and ray traced shadows
REFRACTION, // pass-through with IOR
FLAME // emission surface; constant amplitude
};
| uint vertexOffset = data_map.indices[4 * index]; | ||
| uint triangleOffset = data_map.indices[4*index + 1]; | ||
| uint imageOffset = data_map.indices[4 * index + 2]; | ||
| uint objectType = data_map.indices[4 * index + 3]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The data_map seems to hold offsets, not indices. Should it be named accordingly?
And what do you think about an array of structs, instead of a plain array of values?
|
|
||
| vec3 heatmap(float value, float minValue, float maxValue) | ||
| { | ||
| float scaled = (min(max(value, minValue), maxValue) - minValue) / (maxValue - minValue); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
min(max(value, minValue), maxValue) == clamp(value, minValue, maxValue)
Description
Ray Tracing execution reordering is now public. This sample demonstrates the benefits of SER.
General Checklist:
Please ensure the following points are checked:
Note: The Samples CI runs a number of checks including:
If this PR contains framework changes:
batchcommand line argument to make sure all samples still work properlySample Checklist
If your PR contains a new or modified sample, these further checks must be carried out in addition to the General Checklist: