New mesh shader limits #8507
New mesh shader limits #8507
Conversation
| InterpolationMismatch(Option<naga::Interpolation>), | ||
| #[error("Input sampling doesn't match provided {0:?}")] | ||
| SamplingMismatch(Option<naga::Sampling>), | ||
| #[error("Pipeline input has perprimitive: {expected} but shader declares perprimitive: {}", !expected)] |
There was a problem hiding this comment.
I expect this will be hard to understand as a user - what is a "perprimitive". I would be a bit more verbose here.
| #[error("Mesh shaders are limited to {limit} output vertices, but the shader has a maximum number of {value}")] | ||
| TooManyMeshVertices { limit: u32, value: u32 }, | ||
| #[error("Mesh shaders are limited to {limit} output primitives, but the shader has a maximum number of {value}")] | ||
| TooManyMeshPrimitives { limit: u32, value: u32 }, | ||
| #[error("Mesh or task shaders are limited to {limit} bytes of task payload, but the shader has a task payload of size {value}")] | ||
| TaskPayloadTooLarge { limit: u32, value: u32 }, |
There was a problem hiding this comment.
I would explicitly name the limit that this is based on so they can see exactly which value they need to increment.
| #[error("Primitive index can only be used in a fragment shader if the preceding shader was a vertex shader or a mesh shader that writes to primitive index. | ||
| If a mesh shader writes to primitive index, it must be read by the fragment shader.")] |
There was a problem hiding this comment.
| #[error("Primitive index can only be used in a fragment shader if the preceding shader was a vertex shader or a mesh shader that writes to primitive index. | |
| If a mesh shader writes to primitive index, it must be read by the fragment shader.")] | |
| #[error("Primitive index can only be used in a fragment shader if the preceding shader was a vertex shader or a mesh shader that writes to primitive index. \ | |
| If a mesh shader writes to primitive index, it must be read by the fragment shader.")] |
Make sure you escape newlines in string literals.
This really needs to be two different errors. this is two separate situations: Fragment uses it, but geometry doesn't provide it; Mesh writes it, fragment doesn't consume it.
| let ( | ||
| max_task_workgroup_total_count, | ||
| max_task_workgroups_per_dimension, | ||
| max_mesh_multiview_view_count, | ||
| max_task_mesh_workgroup_total_count, | ||
| max_task_mesh_workgroups_per_dimension, | ||
| max_task_invocations_per_workgroup, | ||
| max_task_invocations_per_dimension, | ||
| max_mesh_invocations_per_workgroup, | ||
| max_mesh_invocations_per_dimension, | ||
| max_task_payload_size, | ||
| max_mesh_output_vertices, | ||
| max_mesh_output_primitives, | ||
| max_mesh_output_layers, | ||
| max_mesh_multiview_view_count, | ||
| ) = match self.mesh_shader { | ||
| Some(m) => ( | ||
| m.max_task_work_group_total_count, | ||
| m.max_task_work_group_count.into_iter().min().unwrap(), | ||
| m.max_mesh_multiview_view_count, | ||
| m.max_task_work_group_total_count | ||
| .min(m.max_mesh_work_group_total_count), | ||
| m.max_task_work_group_count | ||
| .into_iter() | ||
| .chain(m.max_mesh_work_group_count) | ||
| .min() | ||
| .unwrap(), | ||
| m.max_task_work_group_invocations, | ||
| m.max_task_work_group_size.into_iter().min().unwrap(), | ||
| m.max_mesh_work_group_invocations, | ||
| m.max_mesh_work_group_size.into_iter().min().unwrap(), | ||
| m.max_task_payload_size, | ||
| m.max_mesh_output_vertices, | ||
| m.max_mesh_output_primitives, | ||
| m.max_mesh_output_layers, | ||
| m.max_mesh_multiview_view_count, | ||
| ), | ||
| None => (0, 0, 0, 0), | ||
| None => (0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0), | ||
| }; |
There was a problem hiding this comment.
Okay this seems like a recipe for limit mixup disaster.
I would declare each variable at the outer scope, with let mut name = 0; then mutate it in the has-mesh-shader case, that way all the assignments have both the destination and source name right next to each other. This relies on two lists being in the same order.
There was a problem hiding this comment.
Lmao good thinking
|
|
||
| /// The maximum total value of x*y*z for a given `draw_mesh_tasks` command | ||
| pub max_task_workgroup_total_count: u32, | ||
| /// The maximum total value for a `RenderPass::draw_mesh_tasks(x, y, z)` operation. |
There was a problem hiding this comment.
Lets use the exact same wording as the compute shader limit.
| /// The maximum total value of x*y*z for a given `draw_mesh_tasks` command | ||
| pub max_task_workgroup_total_count: u32, | ||
| /// The maximum total value for a `RenderPass::draw_mesh_tasks(x, y, z)` operation. | ||
| /// Also for task shader outputs. Defaults to 65535. Higher is "better". |
There was a problem hiding this comment.
Maximum number of mesh shader workgroups, either those dispatched by a draw_mesh_tasks or by a task workgroup.
There was a problem hiding this comment.
/// The maximum total value for a `RenderPass::draw_mesh_tasks(x, y, z)` operation or the `@builtin(mesh_task_size)` returned from a task shader. Defaults to 65535. Higher is "better".
Something like this
| // These are fundamentally different. It is very common for limits on mesh shaders to be much lower, | ||
| // so as to properly use the hardware, where task shaders are usually just emulated with compute | ||
| // shaders. Therefore, we should have different limits for mesh vs task shaders. |
There was a problem hiding this comment.
I'm not sure this is actually true. I think just explaining here that task shaders need different limits are fine.
| max_task_workgroups_per_dimension: 256, | ||
| // llvmpipe reports 0 multiview count, which just means no multiview is allowed | ||
| max_mesh_multiview_view_count: 0, | ||
| max_task_mesh_workgroup_total_count: 65536, |
There was a problem hiding this comment.
If there's no limit on d3d12 - we should query that vulkan devices support so we can get a more concrete answer for the maximum.
Co-authored-by: Connor Fitzgerald <connorwadefitzgerald@gmail.com>
Co-authored-by: Connor Fitzgerald <connorwadefitzgerald@gmail.com>
3 participants
Connections
Depends on #8370
Works towards #7197
Closes #8003
Description
Add new limits and validation for mesh shaders
Testing
Same old, same old
Squash or Rebase?
Squash
Checklist
cargo fmt.taplo format.cargo clippy --tests. If applicable, add:--target wasm32-unknown-unknowncargo xtask testto run tests.CHANGELOG.mdentry.