Desktop: Use dirty rects for UI texture uploads #4305
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| use cef::Rect; | ||
| use std::sync::{Arc, Mutex}; | ||
|
|
||
| use crate::wrapper::WgpuContext; | ||
|
|
||
| #[derive(Clone)] | ||
| pub(crate) struct View { | ||
| context: WgpuContext, | ||
| texture: Arc<Mutex<Option<wgpu::Texture>>>, | ||
| } | ||
|
|
||
| impl View { | ||
| pub(crate) fn new(context: WgpuContext) -> Self { | ||
| Self { | ||
| context, | ||
| texture: Arc::new(Mutex::new(None)), | ||
| } | ||
| } | ||
|
|
||
| pub(crate) fn texture(&self) -> Option<wgpu::Texture> { | ||
| let Ok(texture) = self.texture.lock() else { | ||
| tracing::error!("Failed to lock view texture"); | ||
| return None; | ||
| }; | ||
| texture.clone() | ||
| } | ||
|
|
||
| pub(super) fn upload_frame_buffer(&self, buffer: &[u8], width: u32, height: u32, dirty_rects: &[Rect]) { | ||
| debug_assert_eq!(buffer.len(), width as usize * height as usize * 4); | ||
|
|
||
| let Ok(mut slot) = self.texture.lock() else { | ||
| tracing::error!("Failed to lock view texture"); | ||
| return; | ||
| }; | ||
|
|
||
| let needs_new_texture = slot.as_ref().is_none_or(|texture| texture.width() != width || texture.height() != height); | ||
| if needs_new_texture { | ||
| *slot = Some(self.context.device.create_texture(&wgpu::TextureDescriptor { | ||
| label: Some("CEF Texture"), | ||
| size: wgpu::Extent3d { | ||
| width, | ||
| height, | ||
| depth_or_array_layers: 1, | ||
| }, | ||
| mip_level_count: 1, | ||
| sample_count: 1, | ||
| dimension: wgpu::TextureDimension::D2, | ||
| format: wgpu::TextureFormat::Bgra8Unorm, | ||
| usage: wgpu::TextureUsages::TEXTURE_BINDING | wgpu::TextureUsages::COPY_DST, | ||
| view_formats: &[], | ||
| })); | ||
| } | ||
| let texture = slot.as_ref().expect("Texture was just created"); | ||
|
|
||
| let full_frame = [Rect { | ||
| x: 0, | ||
| y: 0, | ||
| width: width as i32, | ||
| height: height as i32, | ||
| }]; | ||
| let rects = if needs_new_texture || dirty_rects.is_empty() { &full_frame } else { dirty_rects }; | ||
|
|
||
| for rect in rects { | ||
| let x = (rect.x.max(0) as u32).min(width); | ||
| let y = (rect.y.max(0) as u32).min(height); | ||
| let rect_width = (rect.width.max(0) as u32).min(width - x); | ||
| let rect_height = (rect.height.max(0) as u32).min(height - y); | ||
| if rect_width == 0 || rect_height == 0 { | ||
| continue; | ||
| } | ||
|
Comment on lines
+64
to
+70
Contributor
There was a problem hiding this comment. The current clamping logic does not correctly handle cases where the dirty rectangle starts off-screen (e.g., Using proper rectangle intersection ensures that only the visible portion of the dirty rectangle is uploaded, avoiding unnecessary GPU writes. let x1 = rect.x.max(0).min(width as i32);
let y1 = rect.y.max(0).min(height as i32);
let x2 = (rect.x + rect.width).max(0).min(width as i32);
let y2 = (rect.y + rect.height).max(0).min(height as i32);
let rect_width = (x2 - x1).max(0) as u32;
let rect_height = (y2 - y1).max(0) as u32;
if rect_width == 0 || rect_height == 0 {
continue;
}
let x = x1 as u32;
let y = y1 as u32; |
||
| self.context.queue.write_texture( | ||
| wgpu::TexelCopyTextureInfo { | ||
| texture, | ||
| mip_level: 0, | ||
| origin: wgpu::Origin3d { x, y, z: 0 }, | ||
| aspect: wgpu::TextureAspect::All, | ||
| }, | ||
| buffer, | ||
| wgpu::TexelCopyBufferLayout { | ||
| offset: 4 * (y as u64 * width as u64 + x as u64), | ||
| bytes_per_row: Some(4 * width), | ||
| rows_per_image: None, | ||
| }, | ||
| wgpu::Extent3d { | ||
| width: rect_width, | ||
| height: rect_height, | ||
| depth_or_array_layers: 1, | ||
| }, | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| #[cfg(feature = "accelerated_paint")] | ||
| pub(super) fn import_shared_texture(&self, info: &cef::AcceleratedPaintInfo) { | ||
| let texture = match cef::osr_texture_import::SharedTextureHandle::new(info).import_texture(&self.context.device) { | ||
| Ok(texture) => texture, | ||
| Err(e) => { | ||
| tracing::error!("Failed to import shared texture: {e}"); | ||
| return; | ||
| } | ||
| }; | ||
| let Ok(mut slot) = self.texture.lock() else { | ||
| tracing::error!("Failed to lock view texture"); | ||
| return; | ||
| }; | ||
| *slot = Some(texture); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,2 @@ | ||
| mod state; | ||
| pub(crate) use state::{RenderError, RenderState}; |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
P1: Accelerated paint can crash the app when no texture info is provided, because
info.unwrap()panics in a callback path. HandlingNonelocally (log + return) keeps rendering resilient.(Based on your team's feedback about avoiding panicking Option/Result handling in application code.)
View Feedback: 1 2
Prompt for AI agents