Skip to content

Add @interpolate(flat) to integer varyings in test shaders - #841

Merged
almarklein merged 1 commit into
pygfx:mainfrom
hmaarrfk:wgsl-test-spec-compliance
Oct 2, 2026
Merged

almarklein merged 1 commit into
pygfx:mainfrom
hmaarrfk:wgsl-test-spec-compliance

Conversation

@hmaarrfk

@hmaarrfk hmaarrfk commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

LGTM

Adds a few small things to help make the WGSL valid

claude

[!NOTE]
This PR was written with the help of an AI assistant (Claude).

Some test shaders are accepted by naga but are invalid WGSL: user-defined vertex outputs (and the matching fragment inputs) of integer type must be @interpolate(flat) (spec). Tint (Dawn's and Chrome's WGSL compiler) rejects them when the shader module is created:

integral user-defined vertex outputs must have a '@interpolate(flat)' attribute

This adds @interpolate(flat) to the integer varyings in:

  • tests/test_set_override.py (values: vec4u)
  • tests/test_wgpu_native_query_set.py (index: u32)
  • tests/test_set_immediates.py (index: u32, value: u32)
  • tests/test_wgpu_vertex_instance.py (info: vec2u)

Integer varyings are never interpolated, so this only makes it explicit; it does not change behaviour with wgpu-native. Test results with wgpu-native for these four files are the same before and after this change.

How these were found: all WGSL that the test suite passes to create_shader_module() (with wgpu-native) was recorded and compiled with Dawn. The remaining shaders that Tint rejects are intentionally invalid (test_wgpu_native_errors.py), or use a wgpu-native specific feature (an array in the immediate address space in test_bad_set_immediates), so they are left as is.

Same kind of fix as pygfx/pygfx#1329. Related: #839 (a Dawn backend), which runs these tests against Dawn.

https://claude.ai/code/session_01QMLpZTQYYCu2K7EaWNnkEG

<details><summary>Claude's draft</summary>

The WGSL spec requires user-defined vertex outputs and fragment inputs of
integer type to have @interpolate(flat). Naga accepts them without it, but
Tint (Dawn, Chrome) rejects the shader:

    integral user-defined vertex outputs must have a '@interpolate(flat)' attribute

Add the attribute to the shaders in test_set_override.py,
test_wgpu_native_query_set.py, test_set_immediates.py and
test_wgpu_vertex_instance.py. This does not change behaviour with
wgpu-native: integer varyings are never interpolated anyway.

Found by recording all WGSL that the test suite passes to
create_shader_module() and compiling it with Dawn. The other shaders that
Tint rejects are intentionally invalid (test_wgpu_native_errors.py), or use
a wgpu-native specific feature (arrays in the immediate address space in
test_bad_set_immediates).

Resume this Claude session:
```
cd /home/mark/git/feedstock/staged-recipes
claude --resume 2cab6db9-a6ea-4976-a0e6-f4153fe5a651
```
</details>

Claude-Session: https://claude.ai/code/session_01QMLpZTQYYCu2K7EaWNnkEG

@Vipitis Vipitis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will need this either way for wgpuv30 too.

Yesterday I had pretty much the same commit on my fork: cbbf492

@almarklein

Copy link
Copy Markdown
Member

@hmaarrfk this is still marked as draft, do you plan to add more?

@hmaarrfk

hmaarrfk commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

I haven't really read any of this. But in general. No I think the point was to improve the state of the art toward best syntax.

@hmaarrfk
hmaarrfk marked this pull request as ready for review October 1, 2026 10:14
@hmaarrfk

hmaarrfk commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

LGTM now

@almarklein
almarklein merged commit d21fd6d into pygfx:main Oct 2, 2026
19 checks passed
@hmaarrfk

hmaarrfk commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants