From fc6b51c3586b3eafa084aac1dc77ac72a844ebb2 Mon Sep 17 00:00:00 2001 From: Glenn Watson Date: Wed, 15 Apr 2020 04:18:25 +0000 Subject: [PATCH 1/4] Bug 1629724 - Improve the GC strategy of the render target pool. r=nical Only drop targets from the render target pool when the size of the pool is larger than an arbitrary threshold (this is 32 MB for now), _and_ the render target hasn't been used in the last 60 frames of rendering. This reduces the number of allocation thrashing of textures in the render target pool on most pages. Differential Revision: https://phabricator.services.mozilla.com/D70782 [ghsync] From https://hg.mozilla.org/mozilla-central/rev/c6ed30c910c03820a01baec4dd7e4884e8e37cac --- webrender/src/device/gl.rs | 4 +++ webrender/src/renderer.rs | 74 +++++++++++++++++++++++++++++++------- 2 files changed, 65 insertions(+), 13 deletions(-) diff --git a/webrender/src/device/gl.rs b/webrender/src/device/gl.rs index f77bbe388a..fa5e7b8b4d 100644 --- a/webrender/src/device/gl.rs +++ b/webrender/src/device/gl.rs @@ -595,6 +595,10 @@ impl Texture { !self.fbos_with_depth.is_empty() } + pub fn last_frame_used(&self) -> GpuFrameId { + self.last_frame_used + } + pub fn used_in_frame(&self, frame_id: GpuFrameId) -> bool { self.last_frame_used == frame_id } diff --git a/webrender/src/renderer.rs b/webrender/src/renderer.rs index 4fdb8e51ff..e0d079f8c3 100644 --- a/webrender/src/renderer.rs +++ b/webrender/src/renderer.rs @@ -1100,16 +1100,21 @@ impl TextureResolver { self.return_to_pool(device, target); } - // GC the render target pool. + // GC the render target pool, if it's currently > 32 MB in size. // // We use a simple scheme whereby we drop any texture that hasn't been used - // in the last 30 frames. This should generally prevent any sustained build- - // up of unused textures, unless we don't generate frames for a long period. - // This can happen when the window is minimized, and we probably want to - // flush all the WebRender caches in that case [1]. + // in the last 60 frames, until we are below the size threshold. This should + // generally prevent any sustained build-up of unused textures, unless we don't + // generate frames for a long period. This can happen when the window is + // minimized, and we probably want to flush all the WebRender caches in that case [1]. // // [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1494099 - self.retain_targets(device, |texture| texture.used_recently(frame_id, 30)); + self.gc_targets( + device, + frame_id, + 32 * 1024 * 1024, + 60, + ); } /// Transfers ownership of a render target back to the pool. @@ -1118,18 +1123,59 @@ impl TextureResolver { self.render_target_pool.push(target); } + /// Frees any memory possible, in the event of a memory pressure signal. + fn on_memory_pressure( + &mut self, + device: &mut Device, + ) { + // Clear all textures in the render target pool + for target in self.render_target_pool.drain(..) { + device.delete_texture(target); + } + } + /// Drops all targets from the render target pool that do not satisfy the predicate. - pub fn retain_targets bool>(&mut self, device: &mut Device, f: F) { + pub fn gc_targets( + &mut self, + device: &mut Device, + current_frame_id: GpuFrameId, + total_bytes_threshold: usize, + frames_threshold: usize, + ) { + // Get the total GPU memory size used by the current render target pool + let mut rt_pool_size_in_bytes: usize = self.render_target_pool + .iter() + .map(|t| t.size_in_bytes()) + .sum(); + + // If the total size of the pool is less than the threshold, don't bother + // trying to GC any targets + if rt_pool_size_in_bytes <= total_bytes_threshold { + return; + } + + // Sort the current pool by age, so that we remove oldest textures first + self.render_target_pool.sort_by_key(|t| t.last_frame_used()); + // We can't just use retain() because `Texture` requires manual cleanup. - let mut tmp = SmallVec::<[Texture; 8]>::new(); + let mut retained_targets = SmallVec::<[Texture; 8]>::new(); + for target in self.render_target_pool.drain(..) { - if f(&target) { - tmp.push(target); - } else { + // Drop oldest textures until we are under the allowed size threshold. + // However, if it's been used in very recently, it is always kept around, + // which ensures we don't thrash texture allocations on pages that do + // require a very large render target pool and are regularly changing. + if rt_pool_size_in_bytes > total_bytes_threshold && + !target.used_recently(current_frame_id, frames_threshold) + { + rt_pool_size_in_bytes -= target.size_in_bytes(); device.delete_texture(target); + } else { + retained_targets.push(target); } } - self.render_target_pool.extend(tmp); + + self.render_target_pool.extend(retained_targets); } fn end_pass( @@ -2799,7 +2845,9 @@ impl Renderer { // the device module asserts if we delete textures while // not in a frame. if memory_pressure { - self.texture_resolver.retain_targets(&mut self.device, |_| false); + self.texture_resolver.on_memory_pressure( + &mut self.device, + ); } self.device.end_frame(); From e0211690a2d48f9fb3afd56a159d54137727a7e7 Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Wed, 15 Apr 2020 04:18:35 +0000 Subject: [PATCH 2/4] Bug 1627299 - Remove the separation between API messages and payloads. r=kvark Differential Revision: https://phabricator.services.mozilla.com/D69979 [ghsync] From https://hg.mozilla.org/mozilla-central/rev/68688c8c3bc8497915171bed64a204c0b9a2f020 --- webrender/src/render_backend.rs | 34 +++++--------- webrender/src/renderer.rs | 5 +-- webrender_api/src/api.rs | 75 ++++++++++--------------------- wrench/src/binary_frame_reader.rs | 28 +++--------- 4 files changed, 40 insertions(+), 102 deletions(-) diff --git a/webrender/src/render_backend.rs b/webrender/src/render_backend.rs index 52377dec01..c0a18bf5f8 100644 --- a/webrender/src/render_backend.rs +++ b/webrender/src/render_backend.rs @@ -14,8 +14,9 @@ use api::{IdNamespace, MemoryReport, PipelineId, RenderNotifier, SceneMsg, Scrol use api::{ScrollLocation, TransactionMsg, ResourceUpdate, BlobImageKey}; use api::{NotificationRequest, Checkpoint, QualitySettings}; use api::{ClipIntern, FilterDataIntern, PrimitiveKeyKind}; +use api::channel::Payload; use api::units::*; -use api::channel::{MsgReceiver, MsgSender, Payload}; +use api::channel::{MsgReceiver, MsgSender}; #[cfg(feature = "capture")] use api::CaptureBits; #[cfg(feature = "replay")] @@ -750,14 +751,11 @@ struct PlainRenderBackend { /// The render backend operates on its own thread. pub struct RenderBackend { api_rx: MsgReceiver, - payload_rx: Receiver, result_tx: Sender, scene_tx: Sender, low_priority_scene_tx: Sender, scene_rx: Receiver, - payload_buffer: Vec, - default_device_pixel_ratio: f32, gpu_cache: GpuCache, @@ -782,7 +780,6 @@ pub struct RenderBackend { impl RenderBackend { pub fn new( api_rx: MsgReceiver, - payload_rx: Receiver, result_tx: Sender, scene_tx: Sender, low_priority_scene_tx: Sender, @@ -799,12 +796,10 @@ impl RenderBackend { ) -> RenderBackend { RenderBackend { api_rx, - payload_rx, result_tx, scene_tx, low_priority_scene_tx, scene_rx, - payload_buffer: Vec::new(), default_device_pixel_ratio, resource_cache, gpu_cache: GpuCache::new(), @@ -856,31 +851,21 @@ impl RenderBackend { viewport_size, content_size, list_descriptor, + list_data, preserve_frame_state, } => { profile_scope!("SetDisplayList"); - let data = if let Some(idx) = self.payload_buffer.iter().position(|data| - data.epoch == epoch && data.pipeline_id == pipeline_id - ) { - self.payload_buffer.swap_remove(idx) - } else { - loop { - let data = self.payload_rx.recv().unwrap(); - if data.epoch == epoch && data.pipeline_id == pipeline_id { - break data; - } else { - self.payload_buffer.push(data); - } - } - }; - if let Some(ref mut r) = self.recorder { - r.write_payload(frame_counter, &data.to_data()); + r.write_payload(frame_counter, &Payload::construct_data( + epoch, + pipeline_id, + &list_data, + )); } let built_display_list = - BuiltDisplayList::from_data(data.display_list_data, list_descriptor); + BuiltDisplayList::from_data(list_data, list_descriptor); if !preserve_frame_state { doc.discard_frame_state_for_pipeline(pipeline_id); @@ -1258,6 +1243,7 @@ impl RenderBackend { let pipeline = &doc.loaded_scene.pipelines[&pipeline_id]; let scene_msg = SceneMsg::SetDisplayList { list_descriptor: pipeline.display_list.descriptor().clone(), + list_data: pipeline.display_list.data().to_vec(), epoch, pipeline_id, background: pipeline.background_color, diff --git a/webrender/src/renderer.rs b/webrender/src/renderer.rs index e0d079f8c3..8aa2027fc5 100644 --- a/webrender/src/renderer.rs +++ b/webrender/src/renderer.rs @@ -2190,7 +2190,6 @@ impl Renderer { HAS_BEEN_INITIALIZED.store(true, Ordering::SeqCst); let (api_tx, api_rx) = channel::msg_channel()?; - let (payload_tx, payload_rx) = channel::payload_channel()?; let (result_tx, result_rx) = channel(); let gl_type = gl.get_type(); @@ -2431,7 +2430,6 @@ impl Renderer { let device_pixel_ratio = options.device_pixel_ratio; let debug_flags = options.debug_flags; - let payload_rx_for_backend = payload_rx.to_mpsc_receiver(); let size_of_op = options.size_of_op; let enclosing_size_of_op = options.enclosing_size_of_op; let make_size_of_ops = @@ -2556,7 +2554,6 @@ impl Renderer { let mut backend = RenderBackend::new( api_rx, - payload_rx_for_backend, result_tx, scene_tx, low_priority_scene_tx, @@ -2675,7 +2672,7 @@ impl Renderer { // to ensure any potential transition when enabling a flag is run. renderer.set_debug_flags(debug_flags); - let sender = RenderApiSender::new(api_tx, payload_tx); + let sender = RenderApiSender::new(api_tx); Ok((renderer, sender)) } diff --git a/webrender_api/src/api.rs b/webrender_api/src/api.rs index e322e947c7..95ac9e5917 100644 --- a/webrender_api/src/api.rs +++ b/webrender_api/src/api.rs @@ -6,7 +6,7 @@ extern crate serde_bytes; -use crate::channel::{self, MsgReceiver, MsgSender, Payload, PayloadSender}; +use crate::channel::{self, MsgReceiver, MsgSender}; use peek_poke::PeekPoke; use std::cell::Cell; use std::fmt; @@ -128,9 +128,6 @@ pub struct Transaction { /// Operations affecting the generation of frames (applied after scene building). frame_ops: Vec, - /// Additional display list data. - payloads: Vec, - notifications: Vec, /// Persistent resource updates to apply as part of this transaction. @@ -156,7 +153,6 @@ impl Transaction { scene_ops: Vec::new(), frame_ops: Vec::new(), resource_updates: Vec::new(), - payloads: Vec::new(), notifications: Vec::new(), use_scene_builder_thread: true, generate_frame: false, @@ -254,7 +250,7 @@ impl Transaction { (pipeline_id, content_size, display_list): (PipelineId, LayoutSize, BuiltDisplayList), preserve_frame_state: bool, ) { - let (display_list_data, list_descriptor) = display_list.into_data(); + let (list_data, list_descriptor) = display_list.into_data(); self.scene_ops.push( SceneMsg::SetDisplayList { epoch, @@ -263,10 +259,10 @@ impl Transaction { viewport_size, content_size, list_descriptor, + list_data, preserve_frame_state, } ); - self.payloads.push(Payload { epoch, pipeline_id, display_list_data }); } /// Add a set of persistent resource updates to apply as part of this transaction. @@ -392,20 +388,17 @@ impl Transaction { self.frame_ops } - fn finalize(self) -> (TransactionMsg, Vec) { - ( - TransactionMsg { - scene_ops: self.scene_ops, - frame_ops: self.frame_ops, - resource_updates: self.resource_updates, - notifications: self.notifications, - use_scene_builder_thread: self.use_scene_builder_thread, - generate_frame: self.generate_frame, - invalidate_rendered_frame: self.invalidate_rendered_frame, - low_priority: self.low_priority, - }, - self.payloads, - ) + fn finalize(self) -> TransactionMsg { + TransactionMsg { + scene_ops: self.scene_ops, + frame_ops: self.frame_ops, + resource_updates: self.resource_updates, + notifications: self.notifications, + use_scene_builder_thread: self.use_scene_builder_thread, + generate_frame: self.generate_frame, + invalidate_rendered_frame: self.invalidate_rendered_frame, + low_priority: self.low_priority, + } } /// See `ResourceUpdate::AddImage`. @@ -817,6 +810,8 @@ pub enum SceneMsg { SetDisplayList { /// list_descriptor: BuiltDisplayListDescriptor, + /// The serialized display list. + list_data: Vec, /// epoch: Epoch, /// @@ -1301,15 +1296,13 @@ pub enum ScrollClamping { #[derive(Clone, Deserialize, Serialize)] pub struct RenderApiSender { api_sender: MsgSender, - payload_sender: PayloadSender, } impl RenderApiSender { /// Used internally by the `Renderer`. - pub fn new(api_sender: MsgSender, payload_sender: PayloadSender) -> Self { + pub fn new(api_sender: MsgSender) -> Self { RenderApiSender { api_sender, - payload_sender, } } @@ -1333,7 +1326,6 @@ impl RenderApiSender { }; RenderApi { api_sender: self.api_sender.clone(), - payload_sender: self.payload_sender.clone(), namespace_id, next_id: Cell::new(ResourceId(0)), } @@ -1349,7 +1341,6 @@ impl RenderApiSender { self.api_sender.send(msg).expect("Failed to send CloneApiByClient message"); RenderApi { api_sender: self.api_sender.clone(), - payload_sender: self.payload_sender.clone(), namespace_id, next_id: Cell::new(ResourceId(0)), } @@ -1441,7 +1432,6 @@ bitflags! { /// The main entry point to interact with WebRender. pub struct RenderApi { api_sender: MsgSender, - payload_sender: PayloadSender, namespace_id: IdNamespace, next_id: Cell, } @@ -1454,7 +1444,7 @@ impl RenderApi { /// pub fn clone_sender(&self) -> RenderApiSender { - RenderApiSender::new(self.api_sender.clone(), self.payload_sender.clone()) + RenderApiSender::new(self.api_sender.clone()) } /// Add a document to the WebRender instance. @@ -1608,14 +1598,6 @@ impl RenderApi { self.api_sender.send(msg).unwrap(); } - // For use in Wrench only - #[doc(hidden)] - pub fn send_payload(&self, data: &[u8]) { - self.payload_sender - .send(Payload::from_data(data)) - .unwrap(); - } - /// A helper method to send document messages. fn send_scene_msg(&self, document_id: DocumentId, msg: SceneMsg) { // This assertion fails on Servo use-cases, because it creates different @@ -1638,28 +1620,17 @@ impl RenderApi { /// Send a transaction to WebRender. pub fn send_transaction(&self, document_id: DocumentId, transaction: Transaction) { - let (msg, payloads) = transaction.finalize(); - for payload in payloads { - self.payload_sender.send(payload).unwrap(); - } + let msg = transaction.finalize(); self.api_sender.send(ApiMsg::UpdateDocuments(vec![document_id], vec![msg])).unwrap(); } /// Send multiple transactions. pub fn send_transactions(&self, document_ids: Vec, mut transactions: Vec) { debug_assert!(document_ids.len() == transactions.len()); - let length = document_ids.len(); - let (msgs, mut document_payloads) = transactions.drain(..) - .fold((Vec::with_capacity(length), Vec::with_capacity(length)), - |(mut msgs, mut document_payloads), transaction| { - let (msg, payloads) = transaction.finalize(); - msgs.push(msg); - document_payloads.push(payloads); - (msgs, document_payloads) - }); - for payload in document_payloads.drain(..).flatten() { - self.payload_sender.send(payload).unwrap(); - } + let msgs = transactions.drain(..) + .map(|txn| txn.finalize()) + .collect(); + self.api_sender.send(ApiMsg::UpdateDocuments(document_ids, msgs)).unwrap(); } diff --git a/wrench/src/binary_frame_reader.rs b/wrench/src/binary_frame_reader.rs index 378b41267f..b6d4c01e66 100644 --- a/wrench/src/binary_frame_reader.rs +++ b/wrench/src/binary_frame_reader.rs @@ -14,12 +14,6 @@ use webrender::WEBRENDER_RECORDING_HEADER; use webrender::api::{ApiMsg, SceneMsg}; use crate::wrench::{Wrench, WrenchThing}; -#[derive(Clone)] -enum Item { - Message(ApiMsg), - Data(Vec), -} - pub struct BinaryFrameReader { file: File, eof: bool, @@ -29,7 +23,7 @@ pub struct BinaryFrameReader { replay_api: bool, play_through: bool, - frame_data: Vec, + frame_data: Vec, frame_num: u32, frame_read: bool, } @@ -128,7 +122,7 @@ impl WrenchThing for BinaryFrameReader { let mut found_frame_marker = false; let mut found_display_list = false; let mut found_pipeline = false; - while let Ok(mut len) = self.file.read_u32::() { + while let Ok(len) = self.file.read_u32::() { if len > 0 { let mut buffer = vec![0; len as usize]; self.file.read_exact(&mut buffer).unwrap(); @@ -168,7 +162,7 @@ impl WrenchThing for BinaryFrameReader { _ => {} } if store_message { - self.frame_data.push(Item::Message(msg)); + self.frame_data.push(msg); } // Frames are marked by the GenerateFrame message. // On the first frame, we additionally need to find at least @@ -178,11 +172,6 @@ impl WrenchThing for BinaryFrameReader { if found_frame_marker && (self.frame_num > 0 || (found_display_list && found_pipeline)) { break; } - } else { - len = self.file.read_u32::().unwrap(); - let mut buffer = vec![0; len as usize]; - self.file.read_exact(&mut buffer).unwrap(); - self.frame_data.push(Item::Data(buffer)); } } @@ -198,14 +187,9 @@ impl WrenchThing for BinaryFrameReader { if first_time || self.replay_api { wrench.begin_frame(); let frame_items = self.frame_data.clone(); - for item in frame_items { - match item { - Item::Message(msg) => if !self.should_skip_upload_msg(&msg) { - wrench.api.send_message(msg); - }, - Item::Data(buf) => { - wrench.api.send_payload(&buf); - } + for msg in frame_items { + if !self.should_skip_upload_msg(&msg) { + wrench.api.send_message(msg); } } } else if self.play_through { From 0b374eefde470f86c14b9a99ea4a1b1a5e7c62e2 Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Wed, 15 Apr 2020 04:18:45 +0000 Subject: [PATCH 3/4] Bug 1627299 - Remove MsgSender/MsgReceiver. r=kvark These just wrap regular std Sender/Receiver without providing any value. Serialize/Deserialize was implement manually for MsgSender/MsgReceiver to assert. Serde being amazing, it provides with annotations to not require the traits to be implemented on some enum variants and assert at runtime which functionally equivalent but less error-prone than the fake trait implementations. Removing the rest of channel.rs is coming in a followup. Differential Revision: https://phabricator.services.mozilla.com/D70021 [ghsync] From https://hg.mozilla.org/mozilla-central/rev/b0d79edece1314cc5962c1f625bdffb947419425 --- webrender/src/debug_server.rs | 5 +-- webrender/src/render_backend.rs | 9 ++-- webrender/src/renderer.rs | 12 +++-- webrender/src/scene_builder_thread.rs | 13 +++--- webrender_api/src/api.rs | 64 ++++++++++++++++----------- 5 files changed, 54 insertions(+), 49 deletions(-) diff --git a/webrender/src/debug_server.rs b/webrender/src/debug_server.rs index c5f109808d..c3cd29549a 100644 --- a/webrender/src/debug_server.rs +++ b/webrender/src/debug_server.rs @@ -3,7 +3,6 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use api::{ApiMsg, DebugCommand, DebugFlags}; -use api::channel::MsgSender; use api::units::DeviceIntSize; use crate::print_tree::PrintTreePrinter; use crate::renderer; @@ -27,7 +26,7 @@ enum DebugMsg { struct Server { ws: ws::Sender, debug_tx: Sender, - api_tx: MsgSender, + api_tx: Sender, debug_flags: DebugFlags, } @@ -111,7 +110,7 @@ pub struct DebugServerImpl { } impl DebugServerImpl { - pub fn new(api_tx: MsgSender) -> DebugServerImpl { + pub fn new(api_tx: Sender) -> DebugServerImpl { let (debug_tx, debug_rx) = channel(); let socket = ws::Builder::new() diff --git a/webrender/src/render_backend.rs b/webrender/src/render_backend.rs index c0a18bf5f8..e3a4fbb0cb 100644 --- a/webrender/src/render_backend.rs +++ b/webrender/src/render_backend.rs @@ -16,7 +16,6 @@ use api::{NotificationRequest, Checkpoint, QualitySettings}; use api::{ClipIntern, FilterDataIntern, PrimitiveKeyKind}; use api::channel::Payload; use api::units::*; -use api::channel::{MsgReceiver, MsgSender}; #[cfg(feature = "capture")] use api::CaptureBits; #[cfg(feature = "replay")] @@ -151,7 +150,7 @@ impl ::std::ops::Sub for FrameId { enum RenderBackendStatus { Continue, - ShutDown(Option>), + ShutDown(Option>), } /// Identifier to track a sequence of frames. @@ -750,7 +749,7 @@ struct PlainRenderBackend { /// /// The render backend operates on its own thread. pub struct RenderBackend { - api_rx: MsgReceiver, + api_rx: Receiver, result_tx: Sender, scene_tx: Sender, low_priority_scene_tx: Sender, @@ -779,7 +778,7 @@ pub struct RenderBackend { impl RenderBackend { pub fn new( - api_rx: MsgReceiver, + api_rx: Receiver, result_tx: Sender, scene_tx: Sender, low_priority_scene_tx: Sender, @@ -1733,7 +1732,7 @@ impl RenderBackend { serde_json::to_string(&debug_root).unwrap() } - fn report_memory(&mut self, tx: MsgSender>) { + fn report_memory(&mut self, tx: Sender>) { let mut report = Box::new(MemoryReport::default()); let ops = self.size_of_ops.as_mut().unwrap(); let op = ops.size_of_op; diff --git a/webrender/src/renderer.rs b/webrender/src/renderer.rs index 8aa2027fc5..31c8bfce32 100644 --- a/webrender/src/renderer.rs +++ b/webrender/src/renderer.rs @@ -42,10 +42,8 @@ use api::{DebugCommand, MemoryReport, VoidPtrToSizeFn, PremultipliedColorF}; use api::{RenderApiSender, RenderNotifier, TextureTarget}; #[cfg(feature = "replay")] use api::ExternalImage; -use api::channel; use api::units::*; pub use api::DebugFlags; -use api::channel::MsgSender; use crate::batch::{AlphaBatchContainer, BatchKind, BatchFeatures, BatchTextures, BrushBatchKind, ClipBatchList}; #[cfg(any(feature = "capture", feature = "replay"))] use crate::capture::{CaptureConfig, ExternalCaptureImage, PlainExternalImage}; @@ -110,7 +108,7 @@ use std::path::PathBuf; use std::rc::Rc; use std::sync::Arc; use std::sync::atomic::{AtomicBool, Ordering}; -use std::sync::mpsc::{channel, Receiver}; +use std::sync::mpsc::{channel, Sender, Receiver}; use std::thread; use std::cell::RefCell; use tracy_rs::register_thread_with_profiler; @@ -2189,7 +2187,7 @@ impl Renderer { HAS_BEEN_INITIALIZED.store(true, Ordering::SeqCst); - let (api_tx, api_rx) = channel::msg_channel()?; + let (api_tx, api_rx) = channel(); let (result_tx, result_rx) = channel(); let gl_type = gl.get_type(); @@ -6838,7 +6836,7 @@ pub trait DebugServer { struct NoopDebugServer; impl NoopDebugServer { - fn new(_: MsgSender) -> Self { + fn new(_: Sender) -> Self { NoopDebugServer } } @@ -6848,7 +6846,7 @@ impl DebugServer for NoopDebugServer { } #[cfg(feature = "debugger")] -fn new_debug_server(enable: bool, api_tx: MsgSender) -> Box { +fn new_debug_server(enable: bool, api_tx: Sender) -> Box { if enable { Box::new(debug_server::DebugServerImpl::new(api_tx)) } else { @@ -6857,7 +6855,7 @@ fn new_debug_server(enable: bool, api_tx: MsgSender) -> Box) -> Box { +fn new_debug_server(_enable: bool, api_tx: Sender) -> Box { Box::new(NoopDebugServer::new(api_tx)) } diff --git a/webrender/src/scene_builder_thread.rs b/webrender/src/scene_builder_thread.rs index e8d582b3fe..932d13ed67 100644 --- a/webrender/src/scene_builder_thread.rs +++ b/webrender/src/scene_builder_thread.rs @@ -6,7 +6,6 @@ use api::{AsyncBlobImageRasterizer, BlobImageRequest, BlobImageParams, BlobImage use api::{DocumentId, PipelineId, ApiMsg, FrameMsg, ResourceUpdate, ExternalEvent, Epoch}; use api::{BuiltDisplayList, ColorF, NotificationRequest, Checkpoint, IdNamespace}; use api::{ClipIntern, FilterDataIntern, MemoryReport, PrimitiveKeyKind}; -use api::channel::MsgSender; use api::units::LayoutSize; #[cfg(feature = "capture")] use crate::capture::CaptureConfig; @@ -151,13 +150,13 @@ pub enum SceneBuilderRequest { ExternalEvent(ExternalEvent), DeleteDocument(DocumentId), WakeUp, - Flush(MsgSender<()>), + Flush(Sender<()>), ClearNamespace(IdNamespace), SetFrameBuilderConfig(FrameBuilderConfig), SimulateLongSceneBuild(u32), SimulateLongLowPrioritySceneBuild(u32), Stop, - ReportMemory(Box, MsgSender>), + ReportMemory(Box, Sender>), #[cfg(feature = "capture")] SaveScene(CaptureConfig), #[cfg(feature = "replay")] @@ -169,7 +168,7 @@ pub enum SceneBuilderRequest { pub enum SceneBuilderResult { Transactions(Vec>, Option>), ExternalEvent(ExternalEvent), - FlushComplete(MsgSender<()>), + FlushComplete(Sender<()>), ClearNamespace(IdNamespace), Stopped, DocumentsForDebugger(String) @@ -263,7 +262,7 @@ pub struct SceneBuilderThread { documents: FastHashMap, rx: Receiver, tx: Sender, - api_tx: MsgSender, + api_tx: Sender, config: FrameBuilderConfig, size_of_ops: Option, hooks: Option>, @@ -274,12 +273,12 @@ pub struct SceneBuilderThread { pub struct SceneBuilderThreadChannels { rx: Receiver, tx: Sender, - api_tx: MsgSender, + api_tx: Sender, } impl SceneBuilderThreadChannels { pub fn new( - api_tx: MsgSender + api_tx: Sender ) -> (Self, Sender, Receiver) { let (in_tx, in_rx) = channel(); let (out_tx, out_rx) = channel(); diff --git a/webrender_api/src/api.rs b/webrender_api/src/api.rs index 95ac9e5917..cbb25a90c6 100644 --- a/webrender_api/src/api.rs +++ b/webrender_api/src/api.rs @@ -6,7 +6,6 @@ extern crate serde_bytes; -use crate::channel::{self, MsgReceiver, MsgSender}; use peek_poke::PeekPoke; use std::cell::Cell; use std::fmt; @@ -15,6 +14,7 @@ use std::os::raw::c_void; use std::path::PathBuf; use std::sync::Arc; use std::u32; +use std::sync::mpsc::{Sender, Receiver, channel}; // local imports use crate::{display_item as di, font}; use crate::color::{ColorU, ColorF}; @@ -845,9 +845,11 @@ pub enum FrameMsg { /// UpdateEpoch(PipelineId, Epoch), /// - HitTest(Option, WorldPoint, HitTestFlags, MsgSender), + #[serde(skip_serializing, skip_deserializing)] + HitTest(Option, WorldPoint, HitTestFlags, Sender), /// - RequestHitTester(MsgSender>), + #[serde(skip_serializing, skip_deserializing)] + RequestHitTester(Sender>), /// SetPan(DeviceIntPoint), /// @@ -855,7 +857,8 @@ pub enum FrameMsg { /// ScrollNodeWithId(LayoutPoint, di::ExternalScrollId, ScrollClamping), /// - GetScrollNodeState(MsgSender>), + #[serde(skip_serializing, skip_deserializing)] + GetScrollNodeState(Sender>), /// UpdateDynamicProperties(DynamicProperties), /// @@ -965,7 +968,8 @@ pub enum DebugCommand { /// Save a capture of all the documents state. SaveCapture(PathBuf, CaptureBits), /// Load a capture of all the documents state. - LoadCapture(PathBuf, MsgSender), + #[serde(skip_serializing, skip_deserializing)] + LoadCapture(PathBuf, Sender), /// Clear cached resources, forcing them to be re-uploaded from templates. ClearCaches(ClearCache), /// Enable/disable native compositor usage @@ -994,15 +998,18 @@ pub enum ApiMsg { /// Add/remove/update images and fonts. UpdateResources(Vec), /// Gets the glyph dimensions + #[serde(skip_serializing, skip_deserializing)] GetGlyphDimensions( font::FontInstanceKey, Vec, - MsgSender>>, + Sender>>, ), /// Gets the glyph indices from a string - GetGlyphIndices(font::FontKey, String, MsgSender>>), + #[serde(skip_serializing, skip_deserializing)] + GetGlyphIndices(font::FontKey, String, Sender>>), /// Adds a new document namespace. - CloneApi(MsgSender), + #[serde(skip_serializing, skip_deserializing)] + CloneApi(Sender), /// Adds a new document namespace. CloneApiByClient(IdNamespace), /// Adds a new document with given initial size. @@ -1020,7 +1027,8 @@ pub enum ApiMsg { /// Flush from the caches anything that isn't necessary, to free some memory. MemoryPressure, /// Collects a memory report. - ReportMemory(MsgSender>), + #[serde(skip_serializing, skip_deserializing)] + ReportMemory(Sender>), /// Change debugging options. DebugCommand(DebugCommand), /// Wakes the render backend's event loop up. Needed when an event is communicated @@ -1031,9 +1039,11 @@ pub enum ApiMsg { /// Block until a round-trip to the scene builder thread has completed. This /// ensures that any transactions (including ones deferred to the scene /// builder thread) have been processed. - FlushSceneBuilder(MsgSender<()>), + #[serde(skip_serializing, skip_deserializing)] + FlushSceneBuilder(Sender<()>), /// Shut the WebRender instance down. - ShutDown(Option>), + #[serde(skip_serializing, skip_deserializing)] + ShutDown(Option>), } impl fmt::Debug for ApiMsg { @@ -1293,14 +1303,15 @@ pub enum ScrollClamping { /// /// This object is created along with the `Renderer` and it's main use from a /// user perspective is to create one or several `RenderApi` objects. -#[derive(Clone, Deserialize, Serialize)] +#[derive(Clone)] pub struct RenderApiSender { - api_sender: MsgSender, + api_sender: Sender, + } impl RenderApiSender { /// Used internally by the `Renderer`. - pub fn new(api_sender: MsgSender) -> Self { + pub fn new(api_sender: Sender) -> Self { RenderApiSender { api_sender, } @@ -1308,8 +1319,7 @@ impl RenderApiSender { /// Creates a new resource API object with a dedicated namespace. pub fn create_api(&self) -> RenderApi { - let (sync_tx, sync_rx) = - channel::msg_channel().expect("Failed to create channel"); + let (sync_tx, sync_rx) = channel(); let msg = ApiMsg::CloneApi(sync_tx); self.api_sender.send(msg).expect("Failed to send CloneApi message"); let namespace_id = match sync_rx.recv() { @@ -1431,7 +1441,7 @@ bitflags! { /// The main entry point to interact with WebRender. pub struct RenderApi { - api_sender: MsgSender, + api_sender: Sender, namespace_id: IdNamespace, next_id: Cell, } @@ -1498,7 +1508,7 @@ impl RenderApi { font: font::FontInstanceKey, glyph_indices: Vec, ) -> Vec> { - let (tx, rx) = channel::msg_channel().unwrap(); + let (tx, rx) = channel(); let msg = ApiMsg::GetGlyphDimensions(font, glyph_indices, tx); self.api_sender.send(msg).unwrap(); rx.recv().unwrap() @@ -1507,7 +1517,7 @@ impl RenderApi { /// Gets the glyph indices for the supplied string. These /// can be used to construct GlyphKeys. pub fn get_glyph_indices(&self, font_key: font::FontKey, text: &str) -> Vec> { - let (tx, rx) = channel::msg_channel().unwrap(); + let (tx, rx) = channel(); let msg = ApiMsg::GetGlyphIndices(font_key, text.to_string(), tx); self.api_sender.send(msg).unwrap(); rx.recv().unwrap() @@ -1550,7 +1560,7 @@ impl RenderApi { /// Synchronously requests memory report. pub fn report_memory(&self) -> MemoryReport { - let (tx, rx) = channel::msg_channel().unwrap(); + let (tx, rx) = channel(); self.api_sender.send(ApiMsg::ReportMemory(tx)).unwrap(); *rx.recv().unwrap() } @@ -1564,7 +1574,7 @@ impl RenderApi { /// Shut the WebRender instance down. pub fn shut_down(&self, synchronously: bool) { if synchronously { - let (tx, rx) = channel::msg_channel().unwrap(); + let (tx, rx) = channel(); self.api_sender.send(ApiMsg::ShutDown(Some(tx))).unwrap(); rx.recv().unwrap(); } else { @@ -1646,7 +1656,7 @@ impl RenderApi { point: WorldPoint, flags: HitTestFlags) -> HitTestResult { - let (tx, rx) = channel::msg_channel().unwrap(); + let (tx, rx) = channel(); self.send_frame_msg( document_id, @@ -1657,7 +1667,7 @@ impl RenderApi { /// Synchronously request an object that can perform fast hit testing queries. pub fn request_hit_tester(&self, document_id: DocumentId) -> HitTesterRequest { - let (tx, rx) = channel::msg_channel().unwrap(); + let (tx, rx) = channel(); self.send_frame_msg( document_id, FrameMsg::RequestHitTester(tx) @@ -1696,7 +1706,7 @@ impl RenderApi { /// pub fn get_scroll_node_state(&self, document_id: DocumentId) -> Vec { - let (tx, rx) = channel::msg_channel().unwrap(); + let (tx, rx) = channel(); self.send_frame_msg(document_id, FrameMsg::GetScrollNodeState(tx)); rx.recv().unwrap() } @@ -1712,7 +1722,7 @@ impl RenderApi { /// ensures that any transactions (including ones deferred to the scene /// builder thread) have been processed. pub fn flush_scene_builder(&self) { - let (tx, rx) = channel::msg_channel().unwrap(); + let (tx, rx) = channel(); self.send_message(ApiMsg::FlushSceneBuilder(tx)); rx.recv().unwrap(); // block until done } @@ -1729,7 +1739,7 @@ impl RenderApi { // the capture we are about to load. self.flush_scene_builder(); - let (tx, rx) = channel::msg_channel().unwrap(); + let (tx, rx) = channel(); let msg = ApiMsg::DebugCommand(DebugCommand::LoadCapture(path, tx)); self.send_message(msg); @@ -1758,7 +1768,7 @@ impl Drop for RenderApi { /// /// The request should be resolved as late as possible to reduce the likelihood of blocking. pub struct HitTesterRequest { - rx: MsgReceiver>, + rx: Receiver>, } impl HitTesterRequest { From 13e3bfd6227078e8da8d3b2151493b87a02bd232 Mon Sep 17 00:00:00 2001 From: Jamie Nicol Date: Wed, 15 Apr 2020 04:18:55 +0000 Subject: [PATCH 4/4] Bug 1629835 - Ensure PBO upload strides are a multiple of 64 pixels on adreno r=kvark Previously we were using 256 bytes (which happens to be equal to 64 pixels for RGBA8 textures). A recent change to the GPU Cache uncovered the fact that the requirement is actually 64 pixels, eg 1024 bytes for RGBAF32 textures. Differential Revision: https://phabricator.services.mozilla.com/D70915 [ghsync] From https://hg.mozilla.org/mozilla-central/rev/f5be65b0b22b10f878d6eb297adb68eb3ab82e39 --- webrender/src/device/gl.rs | 46 ++++++++++++++++++++++++++------- webrender/src/renderer.rs | 4 +-- webrender/src/screen_capture.rs | 2 +- 3 files changed, 39 insertions(+), 13 deletions(-) diff --git a/webrender/src/device/gl.rs b/webrender/src/device/gl.rs index fa5e7b8b4d..e0c08b80c0 100644 --- a/webrender/src/device/gl.rs +++ b/webrender/src/device/gl.rs @@ -1038,6 +1038,26 @@ enum TexStorageUsage { Always, } +/// Describes a required alignment for a stride, +/// which can either be represented in bytes or pixels. +#[derive(Copy, Clone, Debug)] +pub enum StrideAlignment { + Bytes(NonZeroUsize), + Pixels(NonZeroUsize), +} + +impl StrideAlignment { + pub fn num_bytes(&self, format: ImageFormat) -> NonZeroUsize { + match *self { + Self::Bytes(bytes) => bytes, + Self::Pixels(pixels) => { + assert!(format.bytes_per_pixel() > 0); + NonZeroUsize::new(pixels.get() * format.bytes_per_pixel() as usize).unwrap() + } + } + } +} + // We get 24 bits of Z value - use up 22 bits of it to give us // 4 bits to account for GPU issues. This seems to manifest on // some GPUs under certain perspectives due to z interpolation @@ -1102,7 +1122,7 @@ pub struct Device { /// format, we fall back to glTexImage*. texture_storage_usage: TexStorageUsage, - optimal_pbo_stride: NonZeroUsize, + optimal_pbo_stride: StrideAlignment, /// Whether we must ensure the source strings passed to glShaderSource() /// are null-terminated, to work around driver bugs. @@ -1525,16 +1545,22 @@ impl Device { // strings are not null-terminated. See bug 1591945. let requires_null_terminated_shader_source = is_emulator; - // On Adreno GPUs PBO texture upload is only performed asynchronously - // if the stride of the data in the PBO is a multiple of 256 bytes. + let is_amd_macos = cfg!(target_os = "macos") && renderer_name.starts_with("AMD"); + + // On certain GPUs PBO texture upload is only performed asynchronously + // if the stride of the data is a multiple of a certain value. + // On Adreno it must be a multiple of 64 pixels, meaning value in bytes + // varies with the texture format. + // On AMD Mac, it must always be a multiple of 256 bytes. // Other platforms may have similar requirements and should be added // here. - // The default value should be 4. - let is_amd_macos = cfg!(target_os = "macos") && renderer_name.starts_with("AMD"); - let optimal_pbo_stride = if is_adreno || is_amd_macos { - NonZeroUsize::new(256).unwrap() + // The default value should be 4 bytes. + let optimal_pbo_stride = if is_adreno { + StrideAlignment::Pixels(NonZeroUsize::new(64).unwrap()) + } else if is_amd_macos { + StrideAlignment::Bytes(NonZeroUsize::new(256).unwrap()) } else { - NonZeroUsize::new(4).unwrap() + StrideAlignment::Bytes(NonZeroUsize::new(4).unwrap()) }; // On AMD Macs there is a driver bug which causes some texture uploads @@ -1665,7 +1691,7 @@ impl Device { return (self.max_depth_ids() - 1) as f32; } - pub fn optimal_pbo_stride(&self) -> NonZeroUsize { + pub fn optimal_pbo_stride(&self) -> StrideAlignment { self.optimal_pbo_stride } @@ -2863,7 +2889,7 @@ impl Device { let bytes_pp = format.bytes_per_pixel() as usize; let width_bytes = size.width as usize * bytes_pp; - let dst_stride = round_up_to_multiple(width_bytes, self.optimal_pbo_stride); + let dst_stride = round_up_to_multiple(width_bytes, self.optimal_pbo_stride.num_bytes(format)); // The size of the chunk should only need to be (height - 1) * dst_stride + width_bytes, // however, the android emulator will error unless it is height * dst_stride. diff --git a/webrender/src/renderer.rs b/webrender/src/renderer.rs index 31c8bfce32..e35d533fb1 100644 --- a/webrender/src/renderer.rs +++ b/webrender/src/renderer.rs @@ -2702,8 +2702,8 @@ impl Renderer { self.device.preferred_color_formats().external } - pub fn optimal_texture_stride_alignment(&self) -> usize { - self.device.optimal_pbo_stride().get() + pub fn optimal_texture_stride_alignment(&self, format: ImageFormat) -> usize { + self.device.optimal_pbo_stride().num_bytes(format).get() } pub fn flush_pipeline_info(&mut self) -> PipelineInfo { diff --git a/webrender/src/screen_capture.rs b/webrender/src/screen_capture.rs index 5941f4e8c5..56fdf45867 100644 --- a/webrender/src/screen_capture.rs +++ b/webrender/src/screen_capture.rs @@ -141,7 +141,7 @@ impl AsyncScreenshotGrabber { let read_size = match self.mode { AsyncScreenshotGrabberMode::ProfilerScreenshots => { let stride = (screenshot_size.width * image_format.bytes_per_pixel()) as usize; - let rounded = round_up_to_multiple(stride, device.optimal_pbo_stride()); + let rounded = round_up_to_multiple(stride, device.optimal_pbo_stride().num_bytes(image_format)); let optimal_width = rounded as i32 / image_format.bytes_per_pixel(); DeviceIntSize::new(