When the bounding boxes of a path and its clip are disjoint (i.e. they
do not intersect) the result of their intersection is a negative
rectangle.
When calculating the intersection of the bboxes, the binning stage
ensures that the bbox is non-negative. It then normalizes the
coordinates to bin dimensions and rounds the top-left corner down
to the neareast integer and the bottom-right corner up.
However this rounding causes zero-area bounding boxes to have a non-zero
area and sends the bottom-right corner to the placed in the next bin.
This causes fully clipped out draw objects to be included in binning,
with an incorrect clip bounding box that causes them to be erroneously
drawn with partial clipping.
`binning` now takes care around this logic to make sure that a zero-area
intersected-bbox gets skipped and clipped out. `tile_alloc`, also takes
care in its logic.
Fixes#286 and #333
Fixed several other shader validation errors caught when running
vello_shaders natively on Metal.
These were primarily caused by reading an invalid drawtag while
accessing the scene buffer. Scene buffer access in the offending
pipelines now initialize the draw tag to DRAWTAG_NOP if an
invocation ID would land beyond the valid index range of encoded draw
objects.
The number of global invocations for draw_leaf can exceed the size of
the draw_monoids buffer which gets conservatively set to the number of
draw objects. Added an explicit bounds check to prevent the invalid
write.
This is not an issue when targeting wgpu as the WGSL compiler emits
implicit bounds checking. When targeting Metal, we disable implicit
bounds checks as that requires an extra buffer binding containing buffer
sizes.
This was caught by Xcode's Metal shader validation and resulted in
visual artifacts in native rendering.
* replace one_minus_focal_x and abs_one_minus_focal_x variables with the actual expressions
* replace division by r^2-1 with multiplication by reciprocal
* revert chain selects to branchy code for clarity. Branching is dynamically uniform so shouldn't affect performance
* add suggested comment describing gradient kind/flags constants
This exchanges the per-pixel branching with additional ALU + selects. My expectation is that this will be faster, but that may be hardware/driver dependent and likely requires profiling and examination of generated code.
The original code is kept in a comment with notes to explain the more obfuscated select version.
Adds full support for COLRv1 radial gradients based on the two-point conical gradient algorithm at https://skia.org/docs/dev/design/conical/
Also adds robustness to degenerate cases in gradient encoding:
* Radial where p0 == p1 && r0 == r1 renders transparent solid
* Empty stops render as transparent solid
* Single stop renders as solid
This patch implements the pad, repeat and reflect extend modes for gradient brushes. Adds a new example demonstrating the functionality.
Also fixes a few bugs:
* Clamps alpha in blend.wgsl for the `blend_compose` function. The `Plus` mode was generating `alpha > 1.0` leading to incorrect rendering.
* Small change to radial gradients in fine.wgsl to reject pixels outside the cone when the circles don't nest. This requires further work to properly extend the cone when one of the radii is not 0.
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 fixes an incorrect application of the inverse transform for radial gradients in fine.
Also fixes an edge case in `SceneBuilder` where a brush transform is identical to the path transform leading to a corrupt encoding.
Just load the atomic bump counter directly instead of piping it through a shared variable, when workgroupUniformLoad is not available. The value is in fact dynamically uniform, but that depends on the stage not setting its own failure flag, a fairly subtle invariant.
I think there was a write-after-read hazard for the reuse of sh_part_count[0]. However, doing the experiment of just changing that doesn't fix the problem on mac. It's possible there's a shader compilation problem (possibly the same one as provoking the storageBarrier workaround in tile_alloc), or also possibly a logic error I'm not understanding.
In any case, this change does appear to fix the hangs on mac.
Fixes#267
A number of things were wrong:
* The args were missing to `run`
* The robust memory changes introduced uniformity errors
* `clear_buffer` is a todo for wgpu on wasm
* Some more time calls crept in
* Initializing both env_logger and console_logger fails
In addition, we conditionally opt the shaders into
`workgroupUniformLoad`, as that's available on wasm but not yet native.
Some of the things (args, uniformity errors) are important fixes. Other
things (clear_buffer, wUL being optional) are workarounds for wgpu
limitations and have TODO items to be removed when wgpu catches up.
* Add counts to offsets when comparing against buffer size limits
* Remove multiplication by 4 in blend buffer allocation (we use units of u32)
* Move buffer sizes from BumpAllocators to Config
* Add comments about early exit
This helps performance but not all performance issues have been resolved. Nontrivial CPU goes into write_buffer, and it's also possible that there isn't enough overlapping between CPU and GPU work.
The area bug was found and fixed by @dfrg, and is adapted from #239. I wanted to move it to a separate PR so that one would be more focused on API.
The other bug is currently silent because the two quantities swapped are both 4, but it is triggered when experimenting with tuning for performance.
Previously there was a limit of 256k pathtags in a scene, due to the need for multi-dispatch prefix sum for the pathtag monoid. This patch increases the limit to 64M, which ought to be enough for most applications.
It works by having 4 dispatches for the pathtag prefix sum: 2 to reduce, then 2 to scan.
* Move the vello crate to the root of the crate
* Add warning that README is work in progress
* Add newline in warning
* Move the unlicense into the shader folder
* Fixup wgsl-analyzer include paths