From 50d25893168a048b5d87c5dbe589d1ceb2afcdf5 Mon Sep 17 00:00:00 2001 From: noituri Date: Tue, 23 Jan 2024 17:02:30 +0100 Subject: [PATCH] Remove `NodeId` usage from `ChromiumSenderThread` --- compositor_render/src/scene.rs | 5 + compositor_render/src/scene/validation.rs | 31 +++++ compositor_render/src/state/node.rs | 3 +- compositor_render/src/state/render_graph.rs | 3 +- .../web_renderer/chromium_sender.rs | 25 +--- .../web_renderer/chromium_sender_thread.rs | 120 ++++++++++-------- .../transformations/web_renderer/embedder.rs | 21 +-- .../src/transformations/web_renderer/node.rs | 9 +- .../transformations/web_renderer/renderer.rs | 10 +- .../web_renderer/shared_memory.rs | 29 +---- examples/web_view.html | 4 +- src/bin/process_helper/handler.rs | 2 +- 12 files changed, 130 insertions(+), 132 deletions(-) diff --git a/compositor_render/src/scene.rs b/compositor_render/src/scene.rs index 20e5002e4..cddabff03 100644 --- a/compositor_render/src/scene.rs +++ b/compositor_render/src/scene.rs @@ -201,6 +201,11 @@ pub enum SceneError { #[error("Instance of web renderer \"{0}\" does not exist. You have to register it first before using it in the scene definition.")] WebRendererNotFound(RendererId), + #[error( + "Instance of web renderer \"{0}\" was used more than once. Only one component can use specific web renderer at the time." + )] + WebRendererUsageNotExclusive(RendererId), + #[error("Invalid parameter passed to \"{1}\" shader.")] ShaderNodeParametersValidationError(#[source] ParametersValidationError, RendererId), diff --git a/compositor_render/src/scene/validation.rs b/compositor_render/src/scene/validation.rs index 496d32809..6b24b823b 100644 --- a/compositor_render/src/scene/validation.rs +++ b/compositor_render/src/scene/validation.rs @@ -1,5 +1,7 @@ use std::collections::HashSet; +use crate::RendererId; + use super::{Component, ComponentId, OutputScene, SceneError}; impl Component { @@ -32,6 +34,7 @@ impl Component { pub(super) fn validate_scene_update(outputs: &[OutputScene]) -> Result<(), SceneError> { validate_component_ids_uniqueness(outputs)?; + validate_web_renderer_ids_uniqueness(outputs)?; Ok(()) } @@ -61,3 +64,31 @@ fn validate_component_ids_uniqueness(outputs: &[OutputScene]) -> Result<(), Scen .iter() .try_for_each(|output| visit(&output.root, &mut ids)) } + +fn validate_web_renderer_ids_uniqueness(outputs: &[OutputScene]) -> Result<(), SceneError> { + let mut web_renderer_ids: HashSet<&RendererId> = HashSet::new(); + + fn visit<'a>( + component: &'a Component, + ids: &mut HashSet<&'a RendererId>, + ) -> Result<(), SceneError> { + if let Component::WebView(web_view) = component { + let instance_id = &web_view.instance_id; + if ids.contains(instance_id) { + return Err(SceneError::WebRendererUsageNotExclusive( + instance_id.clone(), + )); + } + ids.insert(instance_id); + } + + component + .children() + .into_iter() + .try_for_each(|c| visit(c, ids)) + } + + outputs + .iter() + .try_for_each(|output| visit(&output.root, &mut web_renderer_ids)) +} diff --git a/compositor_render/src/state/node.rs b/compositor_render/src/state/node.rs index 2b0e03c93..27837b319 100644 --- a/compositor_render/src/state/node.rs +++ b/compositor_render/src/state/node.rs @@ -120,11 +120,10 @@ impl RenderNode { pub(super) fn new_web_renderer_node( ctx: &RenderCtx, inputs: Vec, - node_id: &NodeId, web_renderer: Arc, ) -> Self { let resolution = web_renderer.resolution(); - let node = InnerRenderNode::Web(WebRendererNode::new(node_id, web_renderer)); + let node = InnerRenderNode::Web(WebRendererNode::new(web_renderer)); let mut output = NodeTexture::new(); output.ensure_size(ctx.wgpu_ctx, resolution); diff --git a/compositor_render/src/state/render_graph.rs b/compositor_render/src/state/render_graph.rs index d31874497..7750e3f47 100644 --- a/compositor_render/src/state/render_graph.rs +++ b/compositor_render/src/state/render_graph.rs @@ -117,8 +117,7 @@ impl RenderGraph { new_nodes.insert(node_id, node); } scene::NodeParams::Web(web_renderer) => { - let node = - RenderNode::new_web_renderer_node(ctx, input_pads, &node_id, web_renderer); + let node = RenderNode::new_web_renderer_node(ctx, input_pads, web_renderer); new_nodes.insert(node_id, node); } scene::NodeParams::Image(image) => { diff --git a/compositor_render/src/transformations/web_renderer/chromium_sender.rs b/compositor_render/src/transformations/web_renderer/chromium_sender.rs index e6cd0bcb4..cf198eb79 100644 --- a/compositor_render/src/transformations/web_renderer/chromium_sender.rs +++ b/compositor_render/src/transformations/web_renderer/chromium_sender.rs @@ -9,7 +9,9 @@ use log::error; use crate::wgpu::texture::NodeTexture; -use super::{browser_client::BrowserClient, chromium_sender_thread::ChromiumSenderThread}; +use super::{ + browser_client::BrowserClient, chromium_sender_thread::ChromiumSenderThread, WebRendererSpec, +}; #[derive(Debug)] pub(super) struct ChromiumSender { @@ -27,13 +29,13 @@ impl Drop for ChromiumSender { } impl ChromiumSender { - pub fn new(ctx: &RegisterCtx, url: String, browser_client: BrowserClient) -> Self { + pub fn new(ctx: &RegisterCtx, spec: &WebRendererSpec, browser_client: BrowserClient) -> Self { let (message_sender, message_receiver) = crossbeam_channel::unbounded(); let (unmap_signal_sender, unmap_signal_receiver) = crossbeam_channel::bounded(0); ChromiumSenderThread::new( ctx, - url, + spec, browser_client, message_receiver, unmap_signal_sender, @@ -48,7 +50,6 @@ impl ChromiumSender { pub fn embed_sources( &self, - node_id: NodeId, sources: &[(&NodeId, &NodeTexture)], ) -> Result<(), ChromiumSenderError> { let resolutions = sources @@ -56,16 +57,12 @@ impl ChromiumSender { .map(|(_, texture)| texture.resolution()) .collect(); self.message_sender - .send(ChromiumSenderMessage::EmbedSources { - node_id, - resolutions, - }) + .send(ChromiumSenderMessage::EmbedSources { resolutions }) .map_err(|_| ChromiumSenderError::MessageChannelDisconnected) } pub fn ensure_shared_memory( &self, - node_id: NodeId, sources: &[(&NodeId, &NodeTexture)], ) -> Result<(), ChromiumSenderError> { let resolutions = sources @@ -73,22 +70,17 @@ impl ChromiumSender { .map(|(_, texture)| texture.resolution()) .collect(); self.message_sender - .send(ChromiumSenderMessage::EnsureSharedMemory { - node_id, - resolutions, - }) + .send(ChromiumSenderMessage::EnsureSharedMemory { resolutions }) .map_err(|_| ChromiumSenderError::MessageChannelDisconnected) } pub fn update_shared_memory( &self, - node_id: NodeId, source_idx: usize, buffer: Arc, size: wgpu::Extent3d, ) -> Result<(), ChromiumSenderError> { let info = UpdateSharedMemoryInfo { - node_id, source_idx, buffer, size, @@ -118,11 +110,9 @@ impl ChromiumSender { pub(super) enum ChromiumSenderMessage { EmbedSources { - node_id: NodeId, resolutions: Vec>, }, EnsureSharedMemory { - node_id: NodeId, resolutions: Vec>, }, UpdateSharedMemory(UpdateSharedMemoryInfo), @@ -133,7 +123,6 @@ pub(super) enum ChromiumSenderMessage { } pub(super) struct UpdateSharedMemoryInfo { - pub node_id: NodeId, pub source_idx: usize, pub buffer: Arc, pub size: wgpu::Extent3d, diff --git a/compositor_render/src/transformations/web_renderer/chromium_sender_thread.rs b/compositor_render/src/transformations/web_renderer/chromium_sender_thread.rs index 482bfc459..e34928c2e 100644 --- a/compositor_render/src/transformations/web_renderer/chromium_sender_thread.rs +++ b/compositor_render/src/transformations/web_renderer/chromium_sender_thread.rs @@ -1,6 +1,5 @@ use std::path::PathBuf; use std::{ - collections::HashMap, sync::Arc, thread::{self, JoinHandle}, }; @@ -10,22 +9,22 @@ use crossbeam_channel::{Receiver, Sender}; use log::error; use crate::error::ErrorStack; -use crate::state::render_graph::NodeId; use crate::state::RegisterCtx; use crate::transformations::web_renderer::chromium_sender::{ ChromiumSenderMessage, UpdateSharedMemoryInfo, }; use crate::transformations::web_renderer::shared_memory::{SharedMemory, SharedMemoryError}; -use crate::transformations::web_renderer::WebRenderer; +use crate::transformations::web_renderer::{WebRenderer, UNEMBED_SOURCE_FRAMES_MESSAGE}; use crate::wgpu::texture::utils::pad_to_256; -use crate::Resolution; +use crate::{RendererId, Resolution}; use super::{browser_client::BrowserClient, chromium_context::ChromiumContext}; -use super::{EMBED_SOURCE_FRAMES_MESSAGE, GET_FRAME_POSITIONS_MESSAGE}; +use super::{WebRendererSpec, EMBED_SOURCE_FRAMES_MESSAGE, GET_FRAME_POSITIONS_MESSAGE}; pub(super) struct ChromiumSenderThread { chromium_ctx: Arc, url: String, + web_renderer_id: RendererId, browser_client: BrowserClient, message_receiver: Receiver, @@ -35,14 +34,15 @@ pub(super) struct ChromiumSenderThread { impl ChromiumSenderThread { pub fn new( ctx: &RegisterCtx, - url: String, + spec: &WebRendererSpec, browser_client: BrowserClient, message_receiver: Receiver, unmap_signal_sender: Sender<()>, ) -> Self { Self { chromium_ctx: ctx.chromium.clone(), - url, + url: spec.url.clone(), + web_renderer_id: spec.instance_id.clone(), browser_client, message_receiver, unmap_signal_sender, @@ -62,17 +62,19 @@ impl ChromiumSenderThread { return; }; - let mut state = ThreadState::new(browser, self.chromium_ctx.instance_id()); + let mut state = ThreadState::new( + browser, + self.chromium_ctx.instance_id(), + &self.web_renderer_id, + ); loop { let result = match self.message_receiver.recv().unwrap() { - ChromiumSenderMessage::EmbedSources { - node_id, - resolutions, - } => self.embed_frames(&mut state, node_id, resolutions), - ChromiumSenderMessage::EnsureSharedMemory { - node_id, - resolutions, - } => self.ensure_shared_memory(&mut state, node_id, resolutions), + ChromiumSenderMessage::EmbedSources { resolutions } => { + self.embed_frames(&mut state, resolutions) + } + ChromiumSenderMessage::EnsureSharedMemory { resolutions } => { + self.ensure_shared_memory(&mut state, resolutions) + } ChromiumSenderMessage::UpdateSharedMemory(info) => { self.update_shared_memory(&mut state, info) } @@ -94,12 +96,8 @@ impl ChromiumSenderThread { fn embed_frames( &self, state: &mut ThreadState, - node_id: NodeId, resolutions: Vec>, ) -> Result<(), ChromiumSenderThreadError> { - let Some(shared_memory) = state.shared_memory.get(&node_id) else { - return Err(ChromiumSenderThreadError::SharedMemoryNotAllocated(node_id)); - }; let mut process_message = cef::ProcessMessage::new(EMBED_SOURCE_FRAMES_MESSAGE); let mut index = 0; @@ -107,12 +105,13 @@ impl ChromiumSenderThread { // - shared memory path // - texture width // - texture height - for (i, resolution) in resolutions.iter().enumerate() { + for (source_idx, resolution) in resolutions.iter().enumerate() { let Resolution { width, height } = resolution.unwrap_or_else(|| Resolution { width: 0, height: 0, }); - process_message.write_string(index, shared_memory[i].to_path_string()); + + process_message.write_string(index, state.shared_memory(source_idx)?.to_path_string()); process_message.write_int(index + 1, width as i32); process_message.write_int(index + 2, height as i32); @@ -128,34 +127,46 @@ impl ChromiumSenderThread { fn ensure_shared_memory( &self, state: &mut ThreadState, - node_id: NodeId, resolutions: Vec>, ) -> Result<(), ChromiumSenderThreadError> { - state.shared_memory.entry(node_id).or_default(); - - let frame = state.browser.main_frame()?; - let shared_memory = state.shared_memory.get_mut(&node_id).unwrap(); - for (source_idx, resolution) in resolutions.into_iter().enumerate() { - let size = match resolution { + fn size_from_resolution(resolution: Option) -> usize { + match resolution { Some(res) => 4 * res.width * res.height, None => 1, - }; + } + } - match shared_memory.get_mut(source_idx) { - Some(shmem) => { - shmem.ensure_size(size, &frame)?; - } - None => { - shared_memory.push(SharedMemory::new( - &state.shared_memory_root_path, - &node_id, - source_idx, - size, - )?); - } + let frame = state.browser.main_frame()?; + let shared_memory = &mut state.shared_memory; + + // Ensure size for already existing shared memory + for (resolution, shmem) in resolutions.iter().zip(shared_memory.iter_mut()) { + let size = size_from_resolution(*resolution); + if shmem.len() != size { + // TODO: This should be synchronised + let mut process_message = cef::ProcessMessage::new(UNEMBED_SOURCE_FRAMES_MESSAGE); + process_message.write_string(0, shmem.to_path_string()); + frame.send_process_message(cef::ProcessId::Renderer, process_message)?; + // ----- + + shmem.resize(size)?; } } + // Create additional shared memory + for (source_idx, resolution) in resolutions + .into_iter() + .enumerate() + .skip(shared_memory.len()) + { + let size = size_from_resolution(resolution); + shared_memory.push(SharedMemory::new( + &state.shared_memory_root_path, + source_idx, + size, + )?); + } + Ok(()) } @@ -165,7 +176,7 @@ impl ChromiumSenderThread { state: &mut ThreadState, info: UpdateSharedMemoryInfo, ) -> Result<(), ChromiumSenderThreadError> { - let shared_memory = state.shared_memory(&info.node_id, info.source_idx)?; + let shared_memory = state.shared_memory(info.source_idx)?; // Writes buffer data to shared memory { @@ -198,7 +209,7 @@ impl ChromiumSenderThread { struct ThreadState { browser: cef::Browser, - shared_memory: HashMap>, + shared_memory: Vec, shared_memory_root_path: PathBuf, } @@ -211,9 +222,10 @@ impl Drop for ThreadState { } impl ThreadState { - fn new(browser: cef::Browser, renderer_id: &str) -> Self { - let shared_memory_root_path = WebRenderer::shared_memory_root_path(renderer_id); - let shared_memory = HashMap::new(); + fn new(browser: cef::Browser, compositor_id: &str, web_renderer_id: &RendererId) -> Self { + let shared_memory_root_path = + WebRenderer::shared_memory_root_path(compositor_id, &web_renderer_id.to_string()); + let shared_memory = Vec::new(); Self { browser, @@ -224,15 +236,11 @@ impl ThreadState { fn shared_memory( &mut self, - node_id: &NodeId, source_idx: usize, ) -> Result<&mut SharedMemory, ChromiumSenderThreadError> { - let node_shared_memory = self - .shared_memory - .get_mut(node_id) - .ok_or_else(|| ChromiumSenderThreadError::SharedMemoryNotAllocated(*node_id))?; - - Ok(&mut node_shared_memory[source_idx]) + self.shared_memory + .get_mut(source_idx) + .ok_or(ChromiumSenderThreadError::SharedMemoryNotAllocated { source_idx }) } } @@ -247,6 +255,6 @@ enum ChromiumSenderThreadError { #[error(transparent)] SharedMemoryError(#[from] SharedMemoryError), - #[error("Shared memory should already be allocated for all inputs of node \"{0}\"")] - SharedMemoryNotAllocated(NodeId), + #[error("Shared memory should already be allocated for \"{source_idx}\" input")] + SharedMemoryNotAllocated { source_idx: usize }, } diff --git a/compositor_render/src/transformations/web_renderer/embedder.rs b/compositor_render/src/transformations/web_renderer/embedder.rs index 8c7a2b4a8..126bdaf49 100644 --- a/compositor_render/src/transformations/web_renderer/embedder.rs +++ b/compositor_render/src/transformations/web_renderer/embedder.rs @@ -33,14 +33,11 @@ impl EmbeddingHelper { pub fn prepare_embedding( &self, - node_id: &NodeId, sources: &[(&NodeId, &NodeTexture)], buffers: &[Arc], ) -> Result<(), EmbedError> { match self.embedding_method { - WebEmbeddingMethod::ChromiumEmbedding => { - self.chromium_embedding(node_id, sources, buffers)? - } + WebEmbeddingMethod::ChromiumEmbedding => self.chromium_embedding(sources, buffers)?, WebEmbeddingMethod::NativeEmbeddingOverContent | WebEmbeddingMethod::NativeEmbeddingUnderContent => { self.chromium_sender.request_frame_positions(sources)? @@ -53,12 +50,10 @@ impl EmbeddingHelper { /// Send sources to chromium and render them on canvases via JS API fn chromium_embedding( &self, - node_id: &NodeId, sources: &[(&NodeId, &NodeTexture)], buffers: &[Arc], ) -> Result<(), EmbedError> { - self.chromium_sender - .ensure_shared_memory(*node_id, sources)?; + self.chromium_sender.ensure_shared_memory(sources)?; self.copy_sources_to_buffers(sources, buffers)?; let mut pending_downloads = Vec::new(); @@ -67,12 +62,7 @@ impl EmbeddingHelper { continue; }; let size = texture_state.rgba_texture().size(); - pending_downloads.push(self.copy_buffer_to_shmem( - *node_id, - source_idx, - size, - buffer.clone(), - )); + pending_downloads.push(self.copy_buffer_to_shmem(source_idx, size, buffer.clone())); } self.wgpu_ctx.device.poll(wgpu::Maintain::Wait); @@ -82,7 +72,7 @@ impl EmbeddingHelper { } self.chromium_sender - .embed_sources(*node_id, sources) + .embed_sources(sources) .map_err(EmbedError::ChromiumSenderError) } @@ -111,7 +101,6 @@ impl EmbeddingHelper { fn copy_buffer_to_shmem( &self, - node_id: NodeId, source_idx: usize, size: wgpu::Extent3d, source: Arc, @@ -129,7 +118,7 @@ impl EmbeddingHelper { r.recv().unwrap()?; self.chromium_sender - .update_shared_memory(node_id, source_idx, source.clone(), size)?; + .update_shared_memory(source_idx, source.clone(), size)?; source.unmap(); Ok(()) diff --git a/compositor_render/src/transformations/web_renderer/node.rs b/compositor_render/src/transformations/web_renderer/node.rs index 69673b001..a86386c3d 100644 --- a/compositor_render/src/transformations/web_renderer/node.rs +++ b/compositor_render/src/transformations/web_renderer/node.rs @@ -16,15 +16,13 @@ use super::WebRenderer; pub struct WebRendererNode { renderer: Arc, - node_id: NodeId, buffers: Vec>, } impl WebRendererNode { - pub fn new(node_id: &NodeId, renderer: Arc) -> Self { + pub fn new(renderer: Arc) -> Self { Self { renderer, - node_id: *node_id, buffers: Vec::new(), } } @@ -37,10 +35,7 @@ impl WebRendererNode { ) { self.ensure_buffers(ctx.wgpu_ctx, sources); - if let Err(err) = self - .renderer - .render(ctx, &self.node_id, sources, &self.buffers, target) - { + if let Err(err) = self.renderer.render(ctx, sources, &self.buffers, target) { error!( "Failed to run web render: {}", ErrorStack::new(&err).into_string() diff --git a/compositor_render/src/transformations/web_renderer/renderer.rs b/compositor_render/src/transformations/web_renderer/renderer.rs index bbf85c648..0f30c82ab 100644 --- a/compositor_render/src/transformations/web_renderer/renderer.rs +++ b/compositor_render/src/transformations/web_renderer/renderer.rs @@ -52,7 +52,7 @@ impl WebRenderer { source_transforms.clone(), spec.resolution, ); - let chromium_sender = ChromiumSender::new(ctx, spec.url.clone(), client); + let chromium_sender = ChromiumSender::new(ctx, &spec, client); let embedding_helper = EmbeddingHelper::new(ctx, chromium_sender, spec.embedding_method); let render_website_shader = WebRendererShader::new(&ctx.wgpu_ctx)?; let website_texture = BGRATexture::new(&ctx.wgpu_ctx, spec.resolution); @@ -70,13 +70,12 @@ impl WebRenderer { pub fn render( &self, ctx: &RenderCtx, - node_id: &NodeId, sources: &[(&NodeId, &NodeTexture)], buffers: &[Arc], target: &mut NodeTexture, ) -> Result<(), RenderWebsiteError> { self.embedding_helper - .prepare_embedding(node_id, sources, buffers) + .prepare_embedding(sources, buffers) .map_err(|err| RenderWebsiteError::EmbeddingFailed(self.spec.url.clone(), err))?; if let Some(frame) = self.retrieve_frame() { @@ -139,10 +138,11 @@ impl WebRenderer { self.spec.resolution } - pub fn shared_memory_root_path(renderer_id: &str) -> PathBuf { + pub fn shared_memory_root_path(compositor_instance_id: &str, web_renderer_id: &str) -> PathBuf { env::temp_dir() .join("video_compositor") - .join(format!("instance_{}", renderer_id)) + .join(format!("instance_{compositor_instance_id}")) + .join(web_renderer_id) } pub fn fallback_strategy(&self) -> FallbackStrategy { diff --git a/compositor_render/src/transformations/web_renderer/shared_memory.rs b/compositor_render/src/transformations/web_renderer/shared_memory.rs index c76a216f0..7f0d95386 100644 --- a/compositor_render/src/transformations/web_renderer/shared_memory.rs +++ b/compositor_render/src/transformations/web_renderer/shared_memory.rs @@ -1,12 +1,9 @@ -use crate::state::render_graph::NodeId; use compositor_chromium::cef; use log::error; use shared_memory::{Shmem, ShmemConf, ShmemError}; use std::path::{Path, PathBuf}; use std::{fs, io}; -use super::UNEMBED_SOURCE_FRAMES_MESSAGE; - pub struct SharedMemory { inner: Option, path: PathBuf, @@ -15,14 +12,11 @@ pub struct SharedMemory { impl SharedMemory { pub fn new( root_path: &Path, - node_id: &NodeId, source_idx: usize, size: usize, ) -> Result { - let path = root_path.join(node_id.to_string()); - Self::init_shared_memory_folder(&path)?; - - Self::from_path(path.join(source_idx.to_string()), size) + Self::init_shared_memory_folder(root_path)?; + Self::from_path(root_path.join(source_idx.to_string()), size) } pub fn from_path(path: PathBuf, size: usize) -> Result { @@ -42,22 +36,11 @@ impl SharedMemory { self.path.display().to_string() } - pub fn ensure_size( - &mut self, - size: usize, - frame: &cef::Frame, - ) -> Result<(), SharedMemoryError> { - let shmem_len = self.inner.as_ref().map(|shmem| shmem.len()).unwrap(); - if shmem_len == size { - return Ok(()); - } - - // TODO: This should be synchronised - let mut process_message = cef::ProcessMessage::new(UNEMBED_SOURCE_FRAMES_MESSAGE); - process_message.write_string(0, self.path.display().to_string()); - frame.send_process_message(cef::ProcessId::Renderer, process_message)?; - // ----- + pub fn len(&self) -> usize { + self.inner.as_ref().map(|shmem| shmem.len()).unwrap() + } + pub fn resize(&mut self, size: usize) -> Result<(), SharedMemoryError> { // Releasing the current `Shmem` instance to ensure it does not erase the shared memory descriptor from the file system // This is critical to ensure when a new `Shmem` is created at the same location, it doesn't conflict with the old descriptor drop(self.inner.take()); diff --git a/examples/web_view.html b/examples/web_view.html index 660df772c..9d9cd7a44 100644 --- a/examples/web_view.html +++ b/examples/web_view.html @@ -37,8 +37,8 @@ -

Frame embedding:

- +

Frame embedding:

+