This turns on clippy checking and also fixes all lints in the code.
Many lints are obvious improvements. Only a small number are slightly annoying, so I think overall worth having a vanilla default config.
Fixes the warnings remaining in the code, which should in turn let us check that in CI.
Also checks the optional features, which are easy enough to break.
* Bazel builds seem to fail to open relative paths to parent directories
due to hermetic sandboxing of third-party repositories. This adds a
WORKSPACE_MANIFEST_FILE environment variable that allows the caller to
optionally provide an absolute path to the workspace root manifest
file.
* The existing code processed a shader file only if
`FileType::is_file` returns true for it. This is not the case when
sources are accessed via symbolic links, which is possible in a Bazel
sandbox. The code now filters for the ".wgsl" file extension instead
of the file type which should generally be safe.
* 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.
Previously the generated shader data structures were rooted in
backend-specific top-level mods (`mod wgsl`, `mod msl`, etc). This made
access to per-shader information that is common to all backends (e.g.
workgroup sizes, shader name etc) awkward to access from backend
agnostic code, especially when feature-gated conditional compilation is
used on the client side.
The data structures have been rearranged such that there is a top-level
`ComputeShader` declaration for each stage under a `gen` mod. The
`ComputeShader` struct declares feature-gated fields for backend shader
sources, such that backend specific data is now a leaf node in the
structure rather than the root. This has some additional benefits:
1. Common data doesn't have to be redeclared, saving on code size when
multiple backends are enabled.
2. The backend specific source code was previously encoded as a `[u8]`.
We can now use types that more closely match the expected format, for
example `&str` for WGSL and MSL, `[u32]` for SPIR-V, etc.
3. If we ever need to expose additional backend-specific metadata in the
future, we can bundle them alongside the source code in a
backend-specific data structure at this level of the tree.
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
* rename resolve_simple to resolve_solid_paths_only to better capture the semantics of the function
* move duplicated buffer size computation code to separate function
* change Resolver::resolve to call resolve_solid_paths_only when encoding.patches.is_empty() is true. This is likely to be slightly faster and will ensure that the "simple" code path is actually used.
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.
Fix WASM build
- Rolled wgpu to 0.16.
- Incorprated the instant crate in lieu of std::time::Instant which works on WASM and native builds.
- Fixed the issue with window scaling by setting the canvas size based on winit Window dimensions.
- Fixed a division-by-zero issue in path_coarse_full
This resolves#276
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.
The examples logged the total processing time for an SVG including both
parse and GPU buffer encoding times. Times for these two operations are
now logged separately.
