Conversation
|
Welcome, new contributor! Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨ |
|
Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke! If it's expected, please add the M-Deliberate-Rendering-Change label. If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it. |
| camera_fov_y, | ||
| camera_near, | ||
| camera_far, | ||
| frame_time_delta: time.delta_secs() * 1000.0, |
| @@ -310,7 +338,7 @@ fn update_ui( | |||
| #[cfg(all(feature = "dlss", not(feature = "force_disable_dlss")))] | |||
| let (projection, fxaa, smaa, taa, cas, msaa, dlss) = *camera; | |||
There was a problem hiding this comment.
probably want fsr in both code paths, otherwise dlss would kind of be a negative feature?
| MipBias, | ||
| DepthPrepass, | ||
| MotionVectorPrepass, | ||
| Fsr3, |
There was a problem hiding this comment.
Nit: Move Fsr3 to the front of the list
| MotionBlur, | ||
| Taa, | ||
| Fsr3, | ||
| DlssSuperResolution, |
There was a problem hiding this comment.
Idk if we need a generic TSR label or something long term, but for now this is fine.
| //! AMD FidelityFX Super Resolution 3 (FSR3). | ||
| //! | ||
| //! FSR3 is a temporal upscaling and anti-aliasing technique that uses | ||
| //! machine learning-based upscaling to render at a lower resolution |
There was a problem hiding this comment.
Oops trusted copilot's suggestions too much
| /// Pros: | ||
| /// * Much better performance by rendering at lower resolution | ||
| /// * High quality temporal anti-aliasing | ||
| /// * Works on AMD, NVIDIA, and Intel GPUs |
There was a problem hiding this comment.
Yes but future, I'll revise this
There was a problem hiding this comment.
Also what about macOS / android / iOS?
| /// * Much better performance by rendering at lower resolution | ||
| /// * High quality temporal anti-aliasing | ||
| /// * Works on AMD, NVIDIA, and Intel GPUs | ||
| /// * Includes optional sharpening pass |
There was a problem hiding this comment.
Does the optional sharpening pass make sense to include? We already have RCAS built into Bevy.
I'd rather people use that, as it's compatible with TAA/DLSS, and will play better once we make a generic TSR abstraction.
There was a problem hiding this comment.
I haven't actually wired it up yet in FSR, but it reuses an existing pass, so I would expect it to be faster.
| MainPassResolutionOverride(render_resolution), | ||
| )); | ||
| } else { | ||
| // Update resolution override in case it changed |
There was a problem hiding this comment.
I haven't sat down to think about it, but why is this structured differently than the DLSS prepare code?
There was a problem hiding this comment.
I was modeling after TAA more than DLSS, I'll have a better look how this can be made more consistent
There was a problem hiding this comment.
Yeah, I would look at the DLSS code and copy that, it's much more 1:1 with how FSR should work.
| .add_systems( | ||
| Render, | ||
| ( | ||
| prepare_fsr3_jitter_and_context.in_set(RenderSystems::ManageViews), |
There was a problem hiding this comment.
Needs .before(prepare_view_targets), since you modify the main camera usages.
| mut commands: Commands, | ||
| mut texture_cache: ResMut<TextureCache>, | ||
| render_device: Res<RenderDevice>, | ||
| views: Query<(Entity, &ExtractedCamera, &ExtractedView, &Fsr3), With<Fsr3>>, |
There was a problem hiding this comment.
You don't need With if you already have &Fsr3 in your query.
| render_device: Res<RenderDevice>, | ||
| views: Query<(Entity, &ExtractedCamera, &ExtractedView, &Fsr3), With<Fsr3>>, | ||
| ) { | ||
| for (entity, camera, view, fsr3) in &views { |
There was a problem hiding this comment.
Please follow the newer pattern for texture-prepare systems as used in https://github.com/bevyengine/bevy/blob/main/crates/bevy_solari/src/realtime/prepare.rs (you can just store TextureView now as textures can be retrieved from views, I should update the solari code)
| /// | ||
| /// See [`Fsr3`] for more details. | ||
| #[derive(Default)] | ||
| pub struct Fsr3Plugin; |
There was a problem hiding this comment.
Please split this file up to match the DLSS file structure (mode, extract, prepare, node).
|
It looks like your PR has been selected for a highlight in the next release blog post, but you didn't provide a release note. Please review the instructions for writing release notes, then expand or revise the content in the release notes directory to showcase your changes. |
|
Hey! I have zero time to actually work on this PR, so am turning it over to any interested contributors. |
Objective
Solution
Testing
Showcase
While a showcase should aim to be brief and digestible, you can use a toggleable section to save space on longer showcases:
Click to view showcase