[shaders] Explicitly guard writes to clip_bboxes

The very last statement of the `clip_leaf` shader is the assignment to
the `clip_bboxes` buffer. The buffer write is indexed on the global
invocation ID. It is possible for this index to be larger than the total
number of clips in at least one workgroup since the clip count isn't
strictly a multiple of workgroup size.

Currently the size of the clip_bboxes buffer matches the number of
clips. This means the buffer index is likely to run past the buffer.
This is not an issue when running on wgpu as it internally enables
bounds checking when compiling WGSL (so all buffer accesses are
implicitly conditional). When compiling the shaders to native backends
the vello_shaders crate currently does not enable implicit bounds
checking, so a buffer overrun is possible.

There are a few potential solutions:

1. Have an explicit bounds check in the shader. This is straightforward
   and consistent with the existing code that reads from clip_inp. The
   downside is that with bounds checking enabled, this extra check is
   redundant in the generated code. This is the solution included in
   this PR.

2. Make sure that the clip_bboxes buffer has a size that is a multiple
   of clip_leaf's workgroup size. This was the approach taken by
   piet-gpu on its native HALs. This effectively wastes up to 4080 bytes
   (255 * 16) to store unused bbox values.

3. Enable Naga's implicit bounds checks when compiling to native. This
   would make the behavior consistent with the wgpu backend, however it
   comes at the cost of increased renderer complexity as the native
   implementation must supply the sizes of each buffer in an implicitly
   generated buffer binding to every shader stage.
This commit is contained in:
Arman Uguray 2023-04-21 18:43:51 -07:00
parent 3d83bd7fa0
commit ceeb0b33b6

View file

@ -200,5 +200,7 @@ fn main(
bbox = vec4(-1e9, -1e9, 1e9, 1e9);
}
}
clip_bboxes[global_id.x] = bbox;
if global_id.x < config.n_clip {
clip_bboxes[global_id.x] = bbox;
}
}