Skip to content
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

Validate task and mesh shader memory limits #6668

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ziga-lunarg
Copy link
Contributor

Closes #6656

@ziga-lunarg ziga-lunarg requested a review from a team as a code owner October 10, 2023 20:24
@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 60658.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 13974 running.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 13974 failed.

@@ -64,10 +64,14 @@ void DecorationBase::Add(uint32_t decoration, uint32_t value) {
case spv::DecorationPerPrimitiveEXT: // VK_EXT_mesh_shader
flags |= per_primitive_ext;
break;
case spv::DecorationPerViewNV:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are only found in the SPV_NV_mesh_shader extension, not the EXT version

Would it be better to check the Entrypoint::execution_model if ExecutionModelMeshNV or ExecutionModelMeshEXT and then have two different versions of CalculateMeshOutputMemory t(or at least a bool is_ext argument) that are called as it seems they are slightly different

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't there be a an enum for the EXT as well? Or do ext mesh shaders not have per view attributes?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so while the vulkan enum for shader stage is the same, the SPIR-V version of MeshNV and MeshEXT are slightly different (which is why there is no enum for EXT in spirv-headers)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 64140.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 14051 running.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 14051 failed.


for (const auto& variable : entrypoint.stage_interface_variables) {
for (const auto& slot : variable.interface_slots) {
uint32_t size = slot.bit_width / 8u;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spec talks about the number of attributes (components?) but doesn't say anything about their size. So I think this should just be 1. Maybe 2 if the size is 64 bits, but that depends on whether interface_slots has one or two entries per 64-bit value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing Runtime Mesh Shading limits
4 participants