vello_hybrid: Use vec3 instead of arrays for BlurParams#1601
Merged
Conversation
Collaborator
Author
|
I guess Windows doesn't like that change. 🙃 |
Contributor
Looks related to: gfx-rs/wgpu#4460 Maybe you can do something like: diff --git a/sparse_strips/vello_sparse_shaders/shaders/filters.wgsl b/sparse_strips/vello_sparse_shaders/shaders/filters.wgsl
index 5413ce3d..2f08f9cc 100644
--- a/sparse_strips/vello_sparse_shaders/shaders/filters.wgsl
+++ b/sparse_strips/vello_sparse_shaders/shaders/filters.wgsl
@@ -87,13 +87,16 @@ fn unpack_flood_filter(data: GpuFilterData) -> FloodFilter {
fn unpack_blur_params(data: GpuFilterData) -> BlurParams {
let n_linear_taps = unpack_header_n_linear_taps(data.data[0]);
let center_weight = bitcast<f32>(data.data[1]);
- var weights = vec3<f32>(0.0);
- var offsets = vec3<f32>(0.0);
-
- for (var i = 0u; i < MAX_TAPS_PER_SIDE; i++) {
- weights[i] = bitcast<f32>(data.data[2u + i]);
- offsets[i] = bitcast<f32>(data.data[2u + MAX_TAPS_PER_SIDE + i]);
- }
+ let weights = vec3<f32>(
+ bitcast<f32>(data.data[2u]),
+ bitcast<f32>(data.data[3u]),
+ bitcast<f32>(data.data[4u]),
+ );
+ let offsets = vec3<f32>(
+ bitcast<f32>(data.data[2u + MAX_TAPS_PER_SIDE]),
+ bitcast<f32>(data.data[3u + MAX_TAPS_PER_SIDE]),
+ bitcast<f32>(data.data[4u + MAX_TAPS_PER_SIDE]),
+ );
return BlurParams(n_linear_taps, center_weight, weights, offsets);
}
@@ -283,10 +286,13 @@ fn convolve(
// First compute the color contribution of the center pixel.
var color = textureSampleLevel(in_tex, linear_sampler, (src_texel + 0.5) / tex_size, 0.0) * center_weight;
+ let weights_arr = array<f32, 3>(weights.x, weights.y, weights.z);
+ let offsets_arr = array<f32, 3>(offsets.x, offsets.y, offsets.z);
+
// Then, compute and sum the contributions of the adjacent pixels.
for (var i = 0u; i < n_linear_taps; i++) {
- let w = weights[i];
- let d = dir * offsets[i];
+ let w = weights_arr[i];
+ let d = dir * offsets_arr[i];
color += textureSampleLevel(in_tex, linear_sampler, (src_texel + d + 0.5) / tex_size, 0.0) * w;
color += textureSampleLevel(in_tex, linear_sampler, (src_texel - d + 0.5) / tex_size, 0.0) * w;
} |
taj-p
approved these changes
Apr 26, 2026
Contributor
taj-p
left a comment
There was a problem hiding this comment.
LGTM after Windows CI passes
Collaborator
Author
LaurenzV
added a commit
that referenced
this pull request
Apr 28, 2026
)" This reverts commit 78cd63a.
pull Bot
pushed a commit
to Mu-L/vello
that referenced
this pull request
Apr 30, 2026
This is a follow-up to linebender#1601. As was mentioned there, we seemingly cannot use indices to index the vector, otherwise it fails on Windows. Therefore, we changed it so the vector is first converted into an array. However, this now leads to issues on certain Mali GPUs (in this case tested on a Realme 8, but I saw other phones affected by this as well). A shader as the following: ``` #version 300 es precision highp float; precision highp int; out vec4 outColor; void main() { float weight = 0.25; float weights_arr[1] = float[1](weight); outColor = vec4(weights_arr[0], 0.0, 0.0, 1.0); } ``` unfortunately fails to compile for many phones: <img width="330" height="85" alt="image" src="https://github.com/user-attachments/assets/b14e134b-cef4-490e-a966-fb160f8106f1" /> First zero-initializing and then assigning each field seems to work fine: ``` float weights_arr[1] = float[1](0.0); weights_arr[0] = weight; ``` Therefore, we need to use this approach as well to make it work on those phones.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

This was tested on a Vivo y21. For some reason, this phone seems to have problems accepting structs that contain more than one array. Something like this:
If I replace this with a single float array of larger size or try other stuff, it seems to work fine, it really seems to just stop working if there are at least two array fields. I don't think this is something actionable to report to naga.
However, luckily we can work around this from our side easily by instead using
vec3, which the phone does support fine. While Vello Hybrid still doesn't work on that device, at least it fails with a different error now. 😆