Skip to content

browser: Reduce spurious rendering and lock contention #56

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.

Sign up for GitHub

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

Open
Yabuku-xD wants to merge 1 commit into
base: main
Choose a base branch
from
Open

browser: Reduce spurious rendering and lock contention #56

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 26 additions & 21 deletions crates/browser/src/browser_view.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -848,7 +848,9 @@ impl BrowserView {
match event {
#[cfg(target_os = "macos")]
TabEvent::FrameReady => {
cx.notify();
if self.surface_state == BrowserSurfaceState::Visible {
cx.notify();
}
}
TabEvent::NavigateToUrl(url) => {
let url = url.clone();
Expand Down Expand Up @@ -1493,26 +1495,29 @@ impl Render for BrowserView {
.into_any_element();

#[cfg(not(target_os = "macos"))]
let element = match self.tab_bar_mode {
TabBarMode::Horizontal => element
.flex_col()
.child(div().mt(px(-1.)).child(self.render_tab_strip(cx)))
.child(self.bookmark_bar.clone())
.child(self.render_browser_content(window, cx))
.into_any_element(),
TabBarMode::Sidebar => element
.flex_row()
.child(self.render_sidebar(cx))
.child(
div()
.flex_1()
.flex()
.flex_col()
.overflow_hidden()
.child(self.bookmark_bar.clone())
.child(self.render_browser_content(window, cx)),
)
.into_any_element(),
let element = {
let pinned_count = self.pinned_tab_count(cx);
match self.tab_bar_mode {
TabBarMode::Horizontal => element
.flex_col()
.child(div().mt(px(-1.)).child(self.render_tab_strip(pinned_count, cx)))
.child(self.bookmark_bar.clone())
.child(self.render_browser_content(window, cx))
.into_any_element(),
TabBarMode::Sidebar => element
.flex_row()
.child(self.render_sidebar(pinned_count, cx))
.child(
div()
.flex_1()
.flex()
.flex_col()
.overflow_hidden()
.child(self.bookmark_bar.clone())
.child(self.render_browser_content(window, cx)),
)
.into_any_element(),
}
};

div()
Expand Down
20 changes: 15 additions & 5 deletions crates/browser/src/browser_view/tab_strip.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,13 +242,20 @@ impl Render for BrowserSidebarPanel {

impl BrowserView {
#[cfg(not(target_os = "macos"))]
pub(super) fn render_tab_strip(&mut self, cx: &mut Context<Self>) -> impl IntoElement {
pub(super) fn pinned_tab_count(&self, cx: &mut gpui::Context<Self>) -> usize {
self.tabs.iter().filter(|t| t.read(cx).is_pinned()).count()
}

#[cfg(not(target_os = "macos"))]
pub(super) fn render_tab_strip(
&mut self,
pinned_count: usize,
cx: &mut Context<Self>,
) -> impl IntoElement {
let theme = cx.theme();
let active_index = self.active_tab_index;
let view = cx.entity().downgrade();

let pinned_count = self.tabs.iter().filter(|t| t.read(cx).is_pinned()).count();

h_flex()
.w_full()
.h(px(34.))
Expand Down Expand Up @@ -613,12 +620,15 @@ impl BrowserView {
}

#[cfg(not(target_os = "macos"))]
pub(super) fn render_sidebar(&mut self, cx: &mut Context<Self>) -> impl IntoElement {
pub(super) fn render_sidebar(
&mut self,
pinned_count: usize,
cx: &mut Context<Self>,
) -> impl IntoElement {
let theme = cx.theme();
let active_index = self.active_tab_index;
let view = cx.entity().downgrade();

let pinned_count = self.tabs.iter().filter(|t| t.read(cx).is_pinned()).count();
let grid_cols = pinned_count.min(3);

v_flex()
Expand Down
27 changes: 24 additions & 3 deletions crates/browser/src/client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ use crate::permission_handler::{OsrPermissionHandler, PermissionHandlerBuilder};
use crate::render_handler::{OsrRenderHandler, RenderHandlerBuilder, RenderState};
use crate::request_handler::{OsrRequestHandler, RequestHandlerBuilder};
use crate::text_input::extract_text_input_state_from_message;
#[cfg(target_os = "macos")]
use core_video::pixel_buffer::CVPixelBuffer;
use parking_lot::Mutex;
use std::sync::Arc;
use std::sync::atomic::{AtomicBool, Ordering};
Expand Down Expand Up @@ -189,27 +191,46 @@ wrap_client! {
}

impl ClientBuilder {
pub fn build(render_state: Arc<Mutex<RenderState>>, event_sender: EventSender) -> cef::Client {
Self::build_inner(render_state, event_sender, KeyboardHandlerBuilder::build())
pub fn build(
render_state: Arc<Mutex<RenderState>>,
#[cfg(target_os = "macos")] current_frame: Arc<Mutex<Option<CVPixelBuffer>>>,
event_sender: EventSender,
) -> cef::Client {
Self::build_inner(
render_state,
#[cfg(target_os = "macos")]
current_frame,
event_sender,
KeyboardHandlerBuilder::build(),
)
}

pub fn build_for_popup(
render_state: Arc<Mutex<RenderState>>,
#[cfg(target_os = "macos")] current_frame: Arc<Mutex<Option<CVPixelBuffer>>>,
event_sender: EventSender,
) -> cef::Client {
Self::build_inner(
render_state,
#[cfg(target_os = "macos")]
current_frame,
event_sender,
PopupKeyboardHandlerBuilder::build(),
)
}

fn build_inner(
render_state: Arc<Mutex<RenderState>>,
#[cfg(target_os = "macos")] current_frame: Arc<Mutex<Option<CVPixelBuffer>>>,
event_sender: EventSender,
keyboard_handler: cef::KeyboardHandler,
) -> cef::Client {
let render_handler = OsrRenderHandler::new(render_state, event_sender.clone());
let render_handler = OsrRenderHandler::new(
render_state,
#[cfg(target_os = "macos")]
current_frame,
event_sender.clone(),
);
let load_handler = OsrLoadHandler::new(event_sender.clone());
let display_handler = OsrDisplayHandler::new(event_sender.clone());
let life_span_handler = OsrLifeSpanHandler::new(event_sender.clone());
Expand Down
11 changes: 10 additions & 1 deletion crates/browser/src/life_span_handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ use cef::{
Browser, ImplLifeSpanHandler, LifeSpanHandler, WrapLifeSpanHandler, rc::Rc as _,
wrap_life_span_handler,
};
#[cfg(target_os = "macos")]
use core_video::pixel_buffer::CVPixelBuffer;
use parking_lot::Mutex;
use std::sync::Arc;

Expand All @@ -26,8 +28,15 @@ impl OsrLifeSpanHandler {

fn popup_client() -> cef::Client {
let render_state = Arc::new(Mutex::new(RenderState::default()));
#[cfg(target_os = "macos")]
let current_frame: Arc<Mutex<Option<CVPixelBuffer>>> = Arc::new(Mutex::new(None));
let (popup_sender, _popup_receiver) = crate::events::event_channel();
ClientBuilder::build_for_popup(render_state, popup_sender)
ClientBuilder::build_for_popup(
render_state,
#[cfg(target_os = "macos")]
current_frame,
popup_sender,
)
}
}

Expand Down
22 changes: 15 additions & 7 deletions crates/browser/src/render_handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ use io_surface::IOSurface;
use parking_lot::Mutex;
use std::sync::Arc;

/// Viewport geometry shared between the GPUI resize path and CEF's view_rect/screen_info callbacks.
/// Kept separate from the frame buffer so geometry reads never contend with 60fps frame writes.
pub struct RenderState {
pub width: u32,
pub height: u32,
pub scale_factor: f32,
#[cfg(target_os = "macos")]
pub current_frame: Option<CVPixelBuffer>,
}

impl Default for RenderState {
Expand All @@ -34,8 +34,6 @@ impl Default for RenderState {
width: 800,
height: 600,
scale_factor: 1.0,
#[cfg(target_os = "macos")]
current_frame: None,
}
}
}
Expand All @@ -44,14 +42,24 @@ impl Default for RenderState {
pub struct OsrRenderHandler {
state: Arc<Mutex<RenderState>>,
#[cfg(target_os = "macos")]
current_frame: Arc<Mutex<Option<CVPixelBuffer>>>,
#[cfg(target_os = "macos")]
sender: EventSender,
}

impl OsrRenderHandler {
pub fn new(state: Arc<Mutex<RenderState>>, sender: EventSender) -> Self {
pub fn new(
state: Arc<Mutex<RenderState>>,
#[cfg(target_os = "macos")] current_frame: Arc<Mutex<Option<CVPixelBuffer>>>,
sender: EventSender,
) -> Self {
#[cfg(target_os = "macos")]
{
Self { state, sender }
Self {
state,
current_frame,
sender,
}
}

#[cfg(not(target_os = "macos"))]
Expand Down Expand Up @@ -153,7 +161,7 @@ wrap_render_handler! {
}
};

self.handler.state.lock().current_frame = Some(pixel_buffer);
*self.handler.current_frame.lock() = Some(pixel_buffer);
let _ = self.handler.sender.send(BrowserEvent::FrameReady);
}

Expand Down
32 changes: 27 additions & 5 deletions crates/browser/src/tab.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ pub struct BrowserTab {
browser_id: Option<i32>,
client: cef::Client,
render_state: Arc<Mutex<RenderState>>,
#[cfg(target_os = "macos")]
current_frame: Arc<Mutex<Option<CVPixelBuffer>>>,
event_receiver: EventReceiver,
url: String,
title: String,
Expand All @@ -127,13 +129,22 @@ impl EventEmitter<TabEvent> for BrowserTab {}
impl BrowserTab {
pub fn new(_cx: &mut Context<Self>) -> Self {
let render_state = Arc::new(Mutex::new(RenderState::default()));
#[cfg(target_os = "macos")]
let current_frame: Arc<Mutex<Option<CVPixelBuffer>>> = Arc::new(Mutex::new(None));
let (sender, receiver) = events::event_channel();
let client = ClientBuilder::build(render_state.clone(), sender);
let client = ClientBuilder::build(
render_state.clone(),
#[cfg(target_os = "macos")]
current_frame.clone(),
sender,
);

Self {
browser_id: None,
client,
render_state,
#[cfg(target_os = "macos")]
current_frame,
event_receiver: receiver,
url: String::from("glass://newtab"),
title: String::from("New Tab"),
Expand All @@ -160,13 +171,22 @@ impl BrowserTab {
_cx: &mut Context<Self>,
) -> Self {
let render_state = Arc::new(Mutex::new(RenderState::default()));
#[cfg(target_os = "macos")]
let current_frame: Arc<Mutex<Option<CVPixelBuffer>>> = Arc::new(Mutex::new(None));
let (sender, receiver) = events::event_channel();
let client = ClientBuilder::build(render_state.clone(), sender);
let client = ClientBuilder::build(
render_state.clone(),
#[cfg(target_os = "macos")]
current_frame.clone(),
sender,
);

Self {
browser_id: None,
client,
render_state,
#[cfg(target_os = "macos")]
current_frame,
event_receiver: receiver,
url,
title,
Expand Down Expand Up @@ -225,7 +245,9 @@ impl BrowserTab {
}
#[cfg(target_os = "macos")]
BrowserEvent::FrameReady => {
cx.emit(TabEvent::FrameReady);
if !is_suspended {
cx.emit(TabEvent::FrameReady);
}
}
BrowserEvent::BrowserCreated => {}
BrowserEvent::LoadError { url, error_text } => {
Expand Down Expand Up @@ -672,7 +694,7 @@ impl BrowserTab {

#[cfg(target_os = "macos")]
pub fn current_frame(&self) -> Option<CVPixelBuffer> {
self.render_state.lock().current_frame.clone()
self.current_frame.lock().clone()
}

pub fn url(&self) -> &str {
Expand Down Expand Up @@ -768,7 +790,7 @@ impl BrowserTab {
}
#[cfg(target_os = "macos")]
{
self.render_state.lock().current_frame = None;
*self.current_frame.lock() = None;
}
}

Expand Down
Loading