Skip to content

Revert Screen Space Transmission Gate for Mesh Bind Groups#24089

Merged
alice-i-cecile merged 3 commits intobevyengine:mainfrom
kfc35:24084_remove_transmission_gating
May 4, 2026
Merged

Revert Screen Space Transmission Gate for Mesh Bind Groups#24089
alice-i-cecile merged 3 commits intobevyengine:mainfrom
kfc35:24084_remove_transmission_gating

Conversation

@kfc35
Copy link
Copy Markdown
Contributor

@kfc35 kfc35 commented May 2, 2026

Objective

Solution

So, originally the SCREEN_SPACE_TRANSMISSION was enabled with key.intersects(MeshPipelineKey::SCREEN_SPACE_SPECULAR_TRANSMISSION_RESERVED_BITS). However, a low quality transmission would make this false, since low’s MeshPipelineKey is configured like this: const SCREEN_SPACE_SPECULAR_TRANSMISSION_LOW = 0 << Self::SCREEN_SPACE_SPECULAR_TRANSMISSION_SHIFT_BITS;. So, a ScreenSpaceTransmission with Low Quality would break rendering (since another if-block merely checks that the ScreenSpaceTransmission component exists)

Making it so that the low transmission truly does not include the view_transmission_textures makes the transmission render not properly - the spheres disappear!

So, I think the proper fix here is to remove the gating around transmission textures.
Edit: Another potential fix is to change the condition of the intersects but I’m not sure how to encode what we want unless we want to add another bit for ScreenSpaceTransmission component exists essentially? Happy to close this PR if that is an acceptable direction.

Testing

cargo run --example transmission works for all quality levels.

@kfc35 kfc35 added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 2, 2026
@github-project-automation github-project-automation Bot moved this to Needs SME Triage in Rendering May 2, 2026
@kfc35 kfc35 requested a review from beicause May 2, 2026 21:34
@kfc35 kfc35 added this to the 0.19 milestone May 2, 2026
@kfc35 kfc35 force-pushed the 24084_remove_transmission_gating branch from 09df482 to c8734aa Compare May 2, 2026 21:38
@kfc35 kfc35 force-pushed the 24084_remove_transmission_gating branch from c8734aa to 5b1581d Compare May 2, 2026 21:46
@kfc35 kfc35 changed the title Remove Screen Space Transmission Gate for Mesh Bind Groups Revert Screen Space Transmission Gate for Mesh Bind Groups May 2, 2026
@kfc35
Copy link
Copy Markdown
Contributor Author

kfc35 commented May 3, 2026

This screen space transmission issue is causing problems (and strobing!) with a couple other examples at least:
fog_volumes and meshlet

Copy link
Copy Markdown
Member

@beicause beicause left a comment

Choose a reason for hiding this comment

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

I can accept this since ScreenSpaceTransmission is a required component and enabled by default. We can add a way to disable it in a separate PR.

if key.contains(MeshPipelineKey::ATMOSPHERE) {
shader_defs.push("ATMOSPHERE".into());
}
if key.intersects(MeshPipelineKey::SCREEN_SPACE_SPECULAR_TRANSMISSION_RESERVED_BITS) {
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.

Why not check all the bits for the different quality levels?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If I understand your question and the code correctly, the quality level is encoded in 2 bits (for low, medium, high, and ultra), a low quality level is encoded as a 0, and we want SCREEN_SPACE_TRANSMISSION to be enabled in that scenario anyway (or else low quality just doesn’t render — I tested with the transmission example). Checking the bits for each individual quality would be redundant in that case since any combo of bits should result in screen space transmission being enabled, so I’m removing the useless gating.

I’m interpreting that you mean to check every individual quality level as already done so here:

let blur_quality =
key.intersection(MeshPipelineKey::SCREEN_SPACE_SPECULAR_TRANSMISSION_RESERVED_BITS);
shader_defs.push(ShaderDefVal::Int(
"SCREEN_SPACE_SPECULAR_TRANSMISSION_BLUR_TAPS".into(),
match blur_quality {
MeshPipelineKey::SCREEN_SPACE_SPECULAR_TRANSMISSION_LOW => 4,
MeshPipelineKey::SCREEN_SPACE_SPECULAR_TRANSMISSION_MEDIUM => 8,
MeshPipelineKey::SCREEN_SPACE_SPECULAR_TRANSMISSION_HIGH => 16,
MeshPipelineKey::SCREEN_SPACE_SPECULAR_TRANSMISSION_ULTRA => 32,
_ => unreachable!(), // Not possible, since the mask is 2 bits, and we've covered all 4 cases
},
));

@alice-i-cecile alice-i-cecile requested a review from JMS55 May 4, 2026 02:43
@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 May 4, 2026
@alice-i-cecile alice-i-cecile added this pull request to the merge queue May 4, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch May 4, 2026
@alice-i-cecile alice-i-cecile enabled auto-merge May 4, 2026 04:47
@alice-i-cecile alice-i-cecile added this pull request to the merge queue May 4, 2026
Merged via the queue into bevyengine:main with commit 1a21cbe May 4, 2026
38 checks passed
@github-project-automation github-project-automation Bot moved this from Needs SME Triage to Done in Rendering May 4, 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 D-Straightforward Simple bug fixes and API improvements, docs, test and examples 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.

4 participants