Repository navigation
perf(rendering): cheaper blur sampling and overlay capture #3584
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
00102ad
02c1dae
a9a4198
9f68ede
2abcf37
bab1fb3
4397279
9549056
7fbeb8c
0c28d46
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| use naga::{ | ||
| front::wgsl, | ||
| valid::{Capabilities, ValidationFlags, Validator}, | ||
| }; | ||
| use std::path::PathBuf; | ||
|
|
||
| fn shader_paths() -> Vec<PathBuf> { | ||
| let dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("src/shaders"); | ||
| let mut paths: Vec<PathBuf> = std::fs::read_dir(&dir) | ||
| .unwrap_or_else(|error| panic!("read {}: {error}", dir.display())) | ||
| .map(|entry| entry.unwrap().path()) | ||
| .filter(|path| { | ||
| path.extension() | ||
| .is_some_and(|extension| extension == "wgsl") | ||
| }) | ||
| .collect(); | ||
| paths.sort(); | ||
| paths | ||
| } | ||
|
|
||
| fn validate(source: &str) -> Result<(), String> { | ||
| let module = wgsl::parse_str(source).map_err(|error| error.emit_to_string(source))?; | ||
| Validator::new(ValidationFlags::all(), Capabilities::all()) | ||
| .validate(&module) | ||
| .map(|_| ()) | ||
| .map_err(|error| error.emit_to_string(source)) | ||
| } | ||
|
|
||
| #[test] | ||
| fn every_wgsl_shader_parses_and_validates() { | ||
| let paths = shader_paths(); | ||
| assert!(!paths.is_empty(), "no shaders found"); | ||
|
|
||
| let failures: Vec<String> = paths | ||
| .iter() | ||
| .filter_map(|path| { | ||
| let source = std::fs::read_to_string(path).unwrap(); | ||
| validate(&source) | ||
| .err() | ||
| .map(|error| format!("{}:\n{error}", path.file_name().unwrap().to_string_lossy())) | ||
| }) | ||
| .collect(); | ||
|
|
||
| assert!(failures.is_empty(), "{}", failures.join("\n\n")); | ||
| } | ||
|
|
||
| #[test] | ||
| fn validation_rejects_an_undefined_identifier() { | ||
| let source = "@fragment fn main() -> @location(0) vec4<f32> { return vec4(missing); }"; | ||
| assert!(validate(source).is_err()); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -622,15 +622,15 @@ impl Capturer { | |
| let crop_data = settings | ||
| .crop | ||
| .map(|crop| { | ||
| let texture = create_copy_texture( | ||
| &d3d_device, | ||
| crop.right - crop.left, | ||
| crop.bottom - crop.top, | ||
| settings.pixel_format, | ||
| ) | ||
| .map_err(NewCapturerError::CropTexture)?; | ||
|
|
||
| Ok::<_, NewCapturerError>((texture, crop)) | ||
| let width = crop.right - crop.left; | ||
| let height = crop.bottom - crop.top; | ||
| let texture = | ||
| create_copy_texture(&d3d_device, width, height, settings.pixel_format) | ||
| .map_err(NewCapturerError::CropTexture)?; | ||
| let crop_pool = Arc::new(TrimPool::default()); | ||
| crop_pool.give(texture, width, height); | ||
|
|
||
| Ok::<_, NewCapturerError>((crop_pool, crop, width, height)) | ||
| }) | ||
| .transpose()?; | ||
|
|
||
|
|
@@ -737,7 +737,9 @@ impl Capturer { | |
| return Ok(()); | ||
| } | ||
|
|
||
| let frame = if let Some((cropped_texture, crop)) = crop_data.clone() { | ||
| let frame = if let Some((crop_pool, crop, crop_width, crop_height)) = | ||
| crop_data.clone() | ||
| { | ||
| // A shrunk item leaves the crop box hanging off the | ||
| // edge of the source texture, which copies nothing | ||
| // and would vend a stale surface as if it were live. | ||
|
|
@@ -750,6 +752,13 @@ impl Capturer { | |
| return Ok(()); | ||
| } | ||
|
|
||
| let cropped_texture = crop_pool.take( | ||
| &d3d_device, | ||
| crop_width, | ||
| crop_height, | ||
| pixel_format, | ||
| )?; | ||
|
|
||
| unsafe { | ||
| d3d_context.CopySubresourceRegion( | ||
| &cropped_texture, | ||
|
|
@@ -764,15 +773,15 @@ impl Capturer { | |
| } | ||
|
|
||
| Frame { | ||
| width: crop.right - crop.left, | ||
| height: crop.bottom - crop.top, | ||
| width: crop_width, | ||
| height: crop_height, | ||
| pixel_format: settings.pixel_format, | ||
| inner: frame, | ||
| texture: cropped_texture, | ||
| d3d_context: d3d_context.clone(), | ||
| d3d_device: d3d_device.clone(), | ||
| staging_pool: staging_pool.clone(), | ||
| recycle: None, | ||
| recycle: Some(crop_pool), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Returning cropped textures to Prompt To Fix With AIThis is a comment left during a code review.
Path: crates/scap-direct3d/src/lib.rs
Line: 778
Comment:
**Crop textures return too early**
Returning cropped textures to `TrimPool` when `Frame` drops does not fully fix the in-flight overwrite. The Windows encoder callbacks return only `frame.texture().clone()`, dropping `Frame` before the encoder calls `process_texture`. The next capture can then reuse and overwrite the same surface before the encoder copies its pixels, producing overwritten frames in cropped recordings. Keep the frame's pool ownership alive until the encoder has copied the texture; a cloned texture handle does not keep `Frame` alive.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly. |
||
| } | ||
| } else { | ||
| // The frame is the content bounded by its surface, | ||
|
|
@@ -981,6 +990,10 @@ impl Frame { | |
| &self.texture | ||
| } | ||
|
|
||
| pub fn is_pooled(&self) -> bool { | ||
| self.recycle.is_some() | ||
| } | ||
|
|
||
| pub fn d3d_device(&self) -> &ID3D11Device { | ||
| &self.d3d_device | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moving
safe_minandsafe_maxintocropped_sample_uvleaves them undefined insample_texture, which still uses both names for neighboring samples. WGSL rejects the whole shader whenCompositeVideoFramePipeline::newcreates it, so video rendering fails even with motion blur disabled. Keep the bounds available insidesample_texture, or return them from the helper.Prompt To Fix With AI