Skip to content

Conversation

@JMS55
Copy link
Contributor

@JMS55 JMS55 commented Oct 25, 2025

  • Fix compile error when compiling with DLSS enabled after Add BindGroupLayout caching via descriptors #21205
  • Use permutation sampling for ReSTIR DI temporal reuse to fix artifacts under DLSS-RR
    • For both DI and GI, removed the spatial raytrace, and moved it to the final reservoir before shading.
  • Reduced DI initial samples 32 -> 8 for better performance at the cost of quality
  • Various specular GI improvements and bugfixes (still kinda terrible overall, I need to do some research on how people usually do this kind of thing)
  • Made the world cache adapt faster / be less stable
  • Switched spatial hashing collisions from to linear probing

@JMS55 JMS55 requested review from SparkyPotato and atlv24 October 25, 2025 00:59
@JMS55 JMS55 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 C-Refinement Improves output quality, without fixing a clear bug or adding new functionality. labels Oct 25, 2025
var<push_constant> constants: PushConstants;

const INITIAL_SAMPLES = 32u;
const INITIAL_SAMPLES = 8u;
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the perf difference?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure, couldn't get bistro setup properly to test :(

var radiance: vec3<f32>;
var wi: vec3<f32>;
if surface.material.roughness > 0.04 {
if surface.material.roughness > 0.1 {
Copy link
Contributor

@SparkyPotato SparkyPotato Oct 25, 2025

Choose a reason for hiding this comment

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

Would it be worth subgroupAny-ing this (and flipping the condition)? One branch is a lot more expensive than the other.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe? Would have to test. I feel like for latency reasons, you'd want to trace the minimum amount of rays possible, even if some threads end up idle.

Comment on lines 78 to 79
let diffuse_brdf = ray_hit.material.base_color / PI;
radiance += throughput * diffuse_brdf * query_world_cache(ray_hit.world_position, ray_hit.geometric_world_normal, view.world_position);
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that this is only sampling the glossy path, I'm not sure if it's worth treating the surface as diffuse and lighting it with the world cache, especially since the moment we hit a rough surface we would do the same (in the previous code).

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm idk. But I think we want to sample the world cache at every bounce, and not just the last. I don't have screenshots on me atm, but comparing the screws in the PICA PICA scene before/after this PR, they look a lot closer to the PT reference.

@alice-i-cecile alice-i-cecile added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Oct 27, 2025
@JMS55 JMS55 added S-Needs-Review Needs reviewer attention (from anyone!) to move forward and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Oct 31, 2025
Copy link
Contributor

@atlv24 atlv24 left a comment

Choose a reason for hiding this comment

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

nice work

@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 Nov 2, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Nov 2, 2025
Merged via the queue into bevyengine:main with commit f94997c Nov 2, 2025
38 checks passed
@github-project-automation github-project-automation bot moved this to Done in Rendering Nov 2, 2025
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 C-Refinement Improves output quality, without fixing a clear bug or adding new functionality. 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.

5 participants