Skip to content

Make downsample.wesl valid WESL - #25744

Merged
alice-i-cecile merged 5 commits into
bevyengine:mainfrom
stuartparmenter:mip-gen-alias-shader-def
Sep 15, 2026
Merged

alice-i-cecile merged 5 commits into
bevyengine:mainfrom
stuartparmenter:mip-gen-alias-shader-def

Conversation

@stuartparmenter

@stuartparmenter stuartparmenter commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Objective

Fixes #25733. We were doing a find-and-replace on ##TEXTURE_FORMAT## in downsample.wesl at runtime, which meant the file was never valid WESL.

Solution

  • Specialize the shader with shader defs instead. It declares one @if(TEXTURE_FORMAT_*) block per supported format, and ARRAY_TEXTURE picks the 2D or array variant.
  • DownsampleShaders::general is now a single Handle<Shader>, specialized lazily by the pipeline cache instead of 40 shader assets created at startup.
  • The shader is specialized through a new DownsamplePipeline resource and SpecializedComputePipelines<DownsamplePipeline>, like the other compute pipelines. The light probe generator uses it instead of its own copy of the layouts and defs.
  • Renamed the FIRST_PASS and SECOND_PASS shader defs to SPLIT_BIND_GROUP_FIRST and SPLIT_BIND_GROUP_SECOND, since exactly one of those and COMBINE_BIND_GROUP is set.

I don't love the boilerplate in this approach, but don't see a better solution. I did prototype a working constants:: version too, but rejected it since it seems like we're generally trying to move away from them.

Testing

dynamic_mip_generation and reflection_probes render correctly.


Used Claude Code & Fable 5.1 to help explore various ways to do this, build tests to see what naga supports and what wesl supports (and what they disagree on) to try various approaches, then to build out multiple approaches, which I then reviewed, settled on this one, validated questions I had, cleaned up PR a bit, then wrote migration guide and here we are.

@stuartparmenter stuartparmenter added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Sep 10, 2026
@github-project-automation github-project-automation Bot moved this to Needs SME Triage in Rendering Sep 10, 2026

@CodingDaniel1 CodingDaniel1 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.

I have also found discrepencies related to pipeline creation (light_probe) against other areas in bevy (clustering, binning). But thats outside the scope of this PR. Which could be follow up work.

Overall this pr is good imo. Except the weird naming pre-existed.

Comment thread crates/bevy_core_pipeline/src/mip_generation/downsample.wesl Outdated
Comment thread crates/bevy_core_pipeline/src/mip_generation/mod.rs Outdated
Comment thread _release-content/migration-guides/downsample_shader_specialization.md Outdated
stuartparmenter and others added 2 commits September 12, 2026 10:42
…tion.md

Co-authored-by: Benjamin Brienen <benjamin.brienen@outlook.com>
Comment thread crates/bevy_core_pipeline/src/mip_generation/mod.rs Outdated

@CodingDaniel1 CodingDaniel1 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.

Looks good to me

@JMS55 JMS55 added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Sep 13, 2026
@stuartparmenter stuartparmenter added this to the 0.20 milestone Sep 15, 2026
@alice-i-cecile
alice-i-cecile added this pull request to the merge queue Sep 15, 2026
Merged via the queue into bevyengine:main with commit ff252f5 Sep 15, 2026
40 checks passed
@github-project-automation github-project-automation Bot moved this from Needs SME Triage to Done in Rendering Sep 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

downsample.wesl has invalid WESL syntax

5 participants