Skip to content

Commit

Permalink
Revert FPS counter (#17485)
Browse files Browse the repository at this point in the history
**UPDATE**: Response so far seems to be that this fixes the performance
issues on Intel MacBooks. So we're going to go ahead and merge it.

This reverts the FPS counter added in 1175391 (#16422) because in this
issue someone bisected recent performance regressions down to this
commit:

- #16729

Another issue that's possibly related:

-
#17305 (comment)

We're reverting this in a PR to create a bundle that people can try out.

Assets:

- Universal Binary:
https://github.com/zed-industries/zed/actions/runs/10735702994/artifacts/1900460781
- x86/Intel:
https://github.com/zed-industries/zed/actions/runs/10735702994/artifacts/1900461236
- Apple Silicon:
https://github.com/zed-industries/zed/actions/runs/10735702994/artifacts/1900460978


Release Notes:

- Removed the recently-added FPS counter since the changes it made to
the Metal renderer on macOS could lead to performance regressions on
Intel MacBooks.

Co-authored-by: Bennet <[email protected]>
  • Loading branch information
mrnugget and bennetbo authored Sep 6, 2024
1 parent 54dd408 commit 938c90f
Show file tree
Hide file tree
Showing 19 changed files with 60 additions and 524 deletions.
16 changes: 0 additions & 16 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ members = [
"crates/outline",
"crates/outline_panel",
"crates/paths",
"crates/performance",
"crates/picker",
"crates/prettier",
"crates/project",
Expand Down Expand Up @@ -246,7 +245,6 @@ open_ai = { path = "crates/open_ai" }
outline = { path = "crates/outline" }
outline_panel = { path = "crates/outline_panel" }
paths = { path = "crates/paths" }
performance = { path = "crates/performance" }
picker = { path = "crates/picker" }
plugin = { path = "crates/plugin" }
plugin_macros = { path = "crates/plugin_macros" }
Expand Down
27 changes: 1 addition & 26 deletions crates/gpui/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::{
path::{Path, PathBuf},
rc::{Rc, Weak},
sync::{atomic::Ordering::SeqCst, Arc},
time::{Duration, Instant},
time::Duration,
};

use anyhow::{anyhow, Result};
Expand Down Expand Up @@ -142,12 +142,6 @@ impl App {
self
}

/// Sets a start time for tracking time to first window draw.
pub fn measure_time_to_first_window_draw(self, start: Instant) -> Self {
self.0.borrow_mut().time_to_first_window_draw = Some(TimeToFirstWindowDraw::Pending(start));
self
}

/// Start the application. The provided callback will be called once the
/// app is fully launched.
pub fn run<F>(self, on_finish_launching: F)
Expand Down Expand Up @@ -253,7 +247,6 @@ pub struct AppContext {
pub(crate) layout_id_buffer: Vec<LayoutId>, // We recycle this memory across layout requests.
pub(crate) propagate_event: bool,
pub(crate) prompt_builder: Option<PromptBuilder>,
pub(crate) time_to_first_window_draw: Option<TimeToFirstWindowDraw>,
}

impl AppContext {
Expand Down Expand Up @@ -307,7 +300,6 @@ impl AppContext {
layout_id_buffer: Default::default(),
propagate_event: true,
prompt_builder: Some(PromptBuilder::Default),
time_to_first_window_draw: None,
}),
});

Expand Down Expand Up @@ -1310,14 +1302,6 @@ impl AppContext {

(task, is_first)
}

/// Returns the time to first window draw, if available.
pub fn time_to_first_window_draw(&self) -> Option<Duration> {
match self.time_to_first_window_draw {
Some(TimeToFirstWindowDraw::Done(duration)) => Some(duration),
_ => None,
}
}
}

impl Context for AppContext {
Expand Down Expand Up @@ -1481,15 +1465,6 @@ impl<G: Global> DerefMut for GlobalLease<G> {
}
}

/// Represents the initialization duration of the application.
#[derive(Clone, Copy)]
pub enum TimeToFirstWindowDraw {
/// The application is still initializing, and contains the start time.
Pending(Instant),
/// The application has finished initializing, and contains the total duration.
Done(Duration),
}

/// Contains state associated with an active drag operation, started by dragging an element
/// within the window or by dragging into the app from the underlying platform.
pub struct AnyDrag {
Expand Down
5 changes: 1 addition & 4 deletions crates/gpui/src/platform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ mod blade;
#[cfg(any(test, feature = "test-support"))]
mod test;

mod fps;
#[cfg(target_os = "windows")]
mod windows;

Expand Down Expand Up @@ -52,7 +51,6 @@ use strum::EnumIter;
use uuid::Uuid;

pub use app_menu::*;
pub use fps::*;
pub use keystroke::*;

#[cfg(target_os = "linux")]
Expand Down Expand Up @@ -356,7 +354,7 @@ pub(crate) trait PlatformWindow: HasWindowHandle + HasDisplayHandle {
fn on_should_close(&self, callback: Box<dyn FnMut() -> bool>);
fn on_close(&self, callback: Box<dyn FnOnce()>);
fn on_appearance_changed(&self, callback: Box<dyn FnMut()>);
fn draw(&self, scene: &Scene, on_complete: Option<oneshot::Sender<()>>);
fn draw(&self, scene: &Scene);
fn completed_frame(&self) {}
fn sprite_atlas(&self) -> Arc<dyn PlatformAtlas>;

Expand All @@ -381,7 +379,6 @@ pub(crate) trait PlatformWindow: HasWindowHandle + HasDisplayHandle {
}
fn set_client_inset(&self, _inset: Pixels) {}
fn gpu_specs(&self) -> Option<GPUSpecs>;
fn fps(&self) -> Option<f32>;

fn update_ime_position(&self, _bounds: Bounds<Pixels>);

Expand Down
14 changes: 1 addition & 13 deletions crates/gpui/src/platform/blade/blade_renderer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use crate::{
};
use bytemuck::{Pod, Zeroable};
use collections::HashMap;
use futures::channel::oneshot;
#[cfg(target_os = "macos")]
use media::core_video::CVMetalTextureCache;
#[cfg(target_os = "macos")]
Expand Down Expand Up @@ -543,12 +542,7 @@ impl BladeRenderer {
self.gpu.destroy_command_encoder(&mut self.command_encoder);
}

pub fn draw(
&mut self,
scene: &Scene,
// Required to compile on macOS, but not currently supported.
_on_complete: Option<oneshot::Sender<()>>,
) {
pub fn draw(&mut self, scene: &Scene) {
self.command_encoder.start();
self.atlas.before_frame(&mut self.command_encoder);
self.rasterize_paths(scene.paths());
Expand Down Expand Up @@ -777,10 +771,4 @@ impl BladeRenderer {
self.wait_for_gpu();
self.last_sync_point = Some(sync_point);
}

/// Required to compile on macOS, but not currently supported.
#[cfg_attr(any(target_os = "linux", target_os = "windows"), allow(dead_code))]
pub fn fps(&self) -> f32 {
0.0
}
}
94 changes: 0 additions & 94 deletions crates/gpui/src/platform/fps.rs

This file was deleted.

12 changes: 4 additions & 8 deletions crates/gpui/src/platform/linux/wayland/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::sync::Arc;

use blade_graphics as gpu;
use collections::HashMap;
use futures::channel::oneshot;
use futures::channel::oneshot::Receiver;

use raw_window_handle as rwh;
use wayland_backend::client::ObjectId;
Expand Down Expand Up @@ -831,7 +831,7 @@ impl PlatformWindow for WaylandWindow {
_msg: &str,
_detail: Option<&str>,
_answers: &[&str],
) -> Option<oneshot::Receiver<usize>> {
) -> Option<Receiver<usize>> {
None
}

Expand Down Expand Up @@ -938,9 +938,9 @@ impl PlatformWindow for WaylandWindow {
self.0.callbacks.borrow_mut().appearance_changed = Some(callback);
}

fn draw(&self, scene: &Scene, on_complete: Option<oneshot::Sender<()>>) {
fn draw(&self, scene: &Scene) {
let mut state = self.borrow_mut();
state.renderer.draw(scene, on_complete);
state.renderer.draw(scene);
}

fn completed_frame(&self) {
Expand Down Expand Up @@ -1018,10 +1018,6 @@ impl PlatformWindow for WaylandWindow {
fn gpu_specs(&self) -> Option<GPUSpecs> {
self.borrow().renderer.gpu_specs().into()
}

fn fps(&self) -> Option<f32> {
None
}
}

fn update_window(mut state: RefMut<WaylandWindowState>) {
Expand Down
13 changes: 4 additions & 9 deletions crates/gpui/src/platform/linux/x11/window.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use anyhow::Context;

use crate::{
platform::blade::{BladeRenderer, BladeSurfaceConfig},
px, size, AnyWindowHandle, Bounds, Decorations, DevicePixels, ForegroundExecutor, GPUSpecs,
Expand All @@ -7,9 +9,7 @@ use crate::{
X11ClientStatePtr,
};

use anyhow::Context;
use blade_graphics as gpu;
use futures::channel::oneshot;
use raw_window_handle as rwh;
use util::{maybe, ResultExt};
use x11rb::{
Expand Down Expand Up @@ -1210,10 +1210,9 @@ impl PlatformWindow for X11Window {
self.0.callbacks.borrow_mut().appearance_changed = Some(callback);
}

// TODO: on_complete not yet supported for X11 windows
fn draw(&self, scene: &Scene, on_complete: Option<oneshot::Sender<()>>) {
fn draw(&self, scene: &Scene) {
let mut inner = self.0.state.borrow_mut();
inner.renderer.draw(scene, on_complete);
inner.renderer.draw(scene);
}

fn sprite_atlas(&self) -> Arc<dyn PlatformAtlas> {
Expand Down Expand Up @@ -1406,8 +1405,4 @@ impl PlatformWindow for X11Window {
fn gpu_specs(&self) -> Option<GPUSpecs> {
self.0.state.borrow().renderer.gpu_specs().into()
}

fn fps(&self) -> Option<f32> {
None
}
}
Loading

0 comments on commit 938c90f

Please sign in to comment.