From 5174f9b27488902e7862aeb470aa0f0375c95e46 Mon Sep 17 00:00:00 2001 From: Christian Duerr Date: Sun, 14 Apr 2019 15:37:58 +0000 Subject: [PATCH] Fix duplicate resize events If a resize event is identical to the current size, it is no longer propagated but the resize is discarded immediately. To further prevent resizes when not necessary, the list of monitors is enumerated and the DPR of the first display is assumed to be the target DPR. This allows spawning a window with dimensions when the config has columns and lines specified and the window only needs to be resized if the estimated DPR is not correct. Fixes #1825. Fixes #204. --- CHANGELOG.md | 2 + font/src/rusttype/mod.rs | 2 +- src/config/mod.rs | 9 ++-- src/display.rs | 106 +++++++++++++++++++++++++++----------- src/renderer/mod.rs | 43 ++++++++++++---- src/tty/windows/conpty.rs | 2 +- src/window.rs | 39 +++++++++----- 7 files changed, 142 insertions(+), 61 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7624faf9..74a97501 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Reset scrolling region when the RIS escape sequence is received - Subprocess spawning on macos +- Unnecessary resize at startup +- Text getting blurry after live-reloading shaders with padding active ## Version 0.3.0 diff --git a/font/src/rusttype/mod.rs b/font/src/rusttype/mod.rs index fc69d6e9..74a5baa5 100644 --- a/font/src/rusttype/mod.rs +++ b/font/src/rusttype/mod.rs @@ -77,7 +77,7 @@ impl crate::Rasterize for RustTypeRasterizer { FontCollection::from_bytes( system_fonts::get(&fp.build()).ok_or_else(|| Error::MissingFont(desc.clone()))?.0, ) - .and_then(|fc| fc.into_font()) + .and_then(FontCollection::into_font) .map_err(|_| Error::UnsupportedFont)?, ); Ok(FontKey { token: (self.fonts.len() - 1) as u16 }) diff --git a/src/config/mod.rs b/src/config/mod.rs index 14ebc87d..3cd33eac 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -485,6 +485,10 @@ impl WindowConfig { pub fn start_maximized(&self) -> bool { self.start_maximized } + + pub fn position(&self) -> Option> { + self.position + } } /// Top-level config type @@ -1794,11 +1798,6 @@ impl Config { self.dimensions.unwrap_or(self.window.dimensions) } - #[inline] - pub fn position(&self) -> Option> { - self.window.position - } - /// Get window config #[inline] pub fn window(&self) -> &WindowConfig { diff --git a/src/display.rs b/src/display.rs index 8dca2f64..ed88f957 100644 --- a/src/display.rs +++ b/src/display.rs @@ -18,6 +18,7 @@ use std::f64; use std::sync::mpsc; use glutin::dpi::{PhysicalPosition, PhysicalSize}; +use glutin::EventsLoop; use parking_lot::MutexGuard; use crate::cli; @@ -137,16 +138,24 @@ impl Display { // Extract some properties from config let render_timer = config.render_timer(); - // Create the window where Alacritty will be displayed - let mut window = Window::new(&options, config.window())?; + // Guess DPR based on first monitor + let event_loop = EventsLoop::new(); + let estimated_dpr = + event_loop.get_available_monitors().next().map(|m| m.get_hidpi_factor()).unwrap_or(1.); - // TODO: replace `set_position` with `with_position` once available - // Upstream issue: https://github.com/tomaka/winit/issues/806 - // Set window position early so it doesn't "teleport" - let position = options.position().or_else(|| config.position()); - if let Some(position) = position { - window.set_position(position.x, position.y); - } + // Guess the target window dimensions + let metrics = GlyphCache::static_metrics(config, estimated_dpr as f32)?; + let (cell_width, cell_height) = Self::compute_cell_size(config, &metrics); + let dimensions = + Self::calculate_dimensions(config, options, estimated_dpr, cell_width, cell_height); + + debug!("Estimated DPR: {}", estimated_dpr); + debug!("Estimated Cell Size: {} x {}", cell_width, cell_height); + debug!("Estimated Dimensions: {:?}", dimensions); + + // Create the window where Alacritty will be displayed + let logical = dimensions.map(|d| PhysicalSize::new(d.0, d.1).to_logical(estimated_dpr)); + let mut window = Window::new(event_loop, &options, config.window(), logical)?; let dpr = window.hidpi_factor(); info!("Device pixel ratio: {}", dpr); @@ -156,43 +165,39 @@ impl Display { window.inner_size_pixels().expect("glutin returns window size").to_physical(dpr); // Create renderer - let mut renderer = QuadRenderer::new(viewport_size)?; + let mut renderer = QuadRenderer::new()?; let (glyph_cache, cell_width, cell_height) = Self::new_glyph_cache(dpr, &mut renderer, config)?; - let dimensions = options.dimensions().unwrap_or_else(|| config.dimensions()); - let mut padding_x = f64::from(config.padding().x) * dpr; let mut padding_y = f64::from(config.padding().y) * dpr; - if dimensions.columns_u32() > 0 - && dimensions.lines_u32() > 0 - && !config.window().start_maximized() + if let Some((width, height)) = + Self::calculate_dimensions(config, options, dpr, cell_width, cell_height) { - // Calculate new size based on cols/lines specified in config - let width = cell_width as u32 * dimensions.columns_u32(); - let height = cell_height as u32 * dimensions.lines_u32(); - padding_x = padding_x.floor(); - padding_y = padding_y.floor(); - - viewport_size = PhysicalSize::new( - f64::from(width) + 2. * padding_x, - f64::from(height) + 2. * padding_y, - ); + if dimensions == Some((width, height)) { + info!("Estimated DPR correctly, skipping resize"); + } else { + viewport_size = PhysicalSize::new(width, height); + window.set_inner_size(viewport_size.to_logical(dpr)); + } } else if config.window().dynamic_padding() { // Make sure additional padding is spread evenly let cw = f64::from(cell_width); let ch = f64::from(cell_height); - padding_x = (padding_x + (viewport_size.width - 2. * padding_x) % cw / 2.).floor(); - padding_y = (padding_y + (viewport_size.height - 2. * padding_y) % ch / 2.).floor(); + padding_x = padding_x + (viewport_size.width - 2. * padding_x) % cw / 2.; + padding_y = padding_y + (viewport_size.height - 2. * padding_y) % ch / 2.; } - window.set_inner_size(viewport_size.to_logical(dpr)); + padding_x = padding_x.floor(); + padding_y = padding_y.floor(); + + // Update OpenGL projection renderer.resize(viewport_size, padding_x as f32, padding_y as f32); - info!("Cell Size: ({} x {})", cell_width, cell_height); - info!("Padding: ({} x {})", padding_x, padding_y); + info!("Cell Size: {} x {}", cell_width, cell_height); + info!("Padding: {} x {}", padding_x, padding_y); let size_info = SizeInfo { dpr, @@ -227,12 +232,41 @@ impl Display { tx, rx, meter: Meter::new(), - font_size: font::Size::new(0.), + font_size: config.font().size(), size_info, last_message: None, }) } + fn calculate_dimensions( + config: &Config, + options: &cli::Options, + dpr: f64, + cell_width: f32, + cell_height: f32, + ) -> Option<(f64, f64)> { + let dimensions = options.dimensions().unwrap_or_else(|| config.dimensions()); + + if dimensions.columns_u32() == 0 + || dimensions.lines_u32() == 0 + || config.window().start_maximized() + { + return None; + } + + let padding_x = f64::from(config.padding().x) * dpr; + let padding_y = f64::from(config.padding().y) * dpr; + + // Calculate new size based on cols/lines specified in config + let grid_width = cell_width as u32 * dimensions.columns_u32(); + let grid_height = cell_height as u32 * dimensions.lines_u32(); + + let width = (f64::from(grid_width) + 2. * padding_x).floor(); + let height = (f64::from(grid_height) + 2. * padding_y).floor(); + + Some((width, height)) + } + fn new_glyph_cache( dpr: f64, renderer: &mut QuadRenderer, @@ -321,6 +355,16 @@ impl Display { let font_changed = terminal.font_size != self.font_size || (dpr - self.size_info.dpr).abs() > f64::EPSILON; + // Skip resize if nothing changed + if let Some(new_size) = new_size { + if !font_changed + && (new_size.width - f64::from(self.size_info.width)).abs() < f64::EPSILON + && (new_size.height - f64::from(self.size_info.height)).abs() < f64::EPSILON + { + return; + } + } + if font_changed || self.last_message != terminal.message_buffer_mut().message() { if new_size == None { // Force a resize to refresh things diff --git a/src/renderer/mod.rs b/src/renderer/mod.rs index b7b4f21a..47851224 100644 --- a/src/renderer/mod.rs +++ b/src/renderer/mod.rs @@ -327,6 +327,21 @@ impl GlyphCache { Ok(()) } + + // Calculate font metrics without access to a glyph cache + // + // This should only be used *before* OpenGL is initialized and the glyph cache can be filled. + pub fn static_metrics(config: &Config, dpr: f32) -> Result { + let font = config.font().clone(); + + let mut rasterizer = font::Rasterizer::new(dpr, config.use_thin_strokes())?; + let regular_desc = + GlyphCache::make_desc(&font.normal(), font::Slant::Normal, font::Weight::Normal); + let regular = rasterizer.load_font(®ular_desc, font.size())?; + rasterizer.get_glyph(GlyphKey { font_key: regular, c: 'm', size: font.size() })?; + + rasterizer.metrics(regular, font.size()) + } } #[derive(Debug)] @@ -475,8 +490,8 @@ const BATCH_MAX: usize = 0x1_0000; const ATLAS_SIZE: i32 = 1024; impl QuadRenderer { - pub fn new(size: PhysicalSize) -> Result { - let program = TextShaderProgram::new(size)?; + pub fn new() -> Result { + let program = TextShaderProgram::new()?; let rect_program = RectShaderProgram::new()?; let mut vao: GLuint = 0; @@ -723,8 +738,7 @@ impl QuadRenderer { { // Flush message queue if let Ok(Msg::ShaderReload) = self.rx.try_recv() { - let size = PhysicalSize::new(f64::from(props.width), f64::from(props.height)); - self.reload_shaders(size); + self.reload_shaders(props); } while let Ok(_) = self.rx.try_recv() {} @@ -773,11 +787,22 @@ impl QuadRenderer { }) } - pub fn reload_shaders(&mut self, size: PhysicalSize) { - warn!("Reloading shaders..."); - let result = (TextShaderProgram::new(size), RectShaderProgram::new()); + pub fn reload_shaders(&mut self, props: &term::SizeInfo) { + info!("Reloading shaders..."); + let result = (TextShaderProgram::new(), RectShaderProgram::new()); let (program, rect_program) = match result { (Ok(program), Ok(rect_program)) => { + unsafe { + gl::UseProgram(program.id); + program.update_projection( + props.width, + props.height, + props.padding_x, + props.padding_y, + ); + gl::UseProgram(0); + } + info!("... successfully reloaded shaders"); (program, rect_program) }, @@ -1077,7 +1102,7 @@ impl<'a> Drop for RenderApi<'a> { } impl TextShaderProgram { - pub fn new(size: PhysicalSize) -> Result { + pub fn new() -> Result { let (vertex_src, fragment_src) = if cfg!(feature = "live-shader-reload") { (None, None) } else { @@ -1127,8 +1152,6 @@ impl TextShaderProgram { u_background: background, }; - shader.update_projection(size.width as f32, size.height as f32, 0., 0.); - unsafe { gl::UseProgram(0); } diff --git a/src/tty/windows/conpty.rs b/src/tty/windows/conpty.rs index 70ca55fe..45718ed2 100644 --- a/src/tty/windows/conpty.rs +++ b/src/tty/windows/conpty.rs @@ -143,7 +143,7 @@ pub fn new<'a>( let mut startup_info_ex: STARTUPINFOEXW = Default::default(); - let title = options.title.as_ref().map(|w| w.as_str()).unwrap_or("Alacritty"); + let title = options.title.as_ref().map(String::as_str).unwrap_or("Alacritty"); let title = U16CString::from_str(title).unwrap(); startup_info_ex.StartupInfo.lpTitle = title.as_ptr() as LPWSTR; diff --git a/src/window.rs b/src/window.rs index 224eb7c3..b471bbd2 100644 --- a/src/window.rs +++ b/src/window.rs @@ -116,10 +116,15 @@ impl From for Error { } fn create_gl_window( - window: WindowBuilder, + mut window: WindowBuilder, event_loop: &EventsLoop, srgb: bool, + dimensions: Option, ) -> Result> { + if let Some(dimensions) = dimensions { + window = window.with_dimensions(dimensions); + } + let windowed_context = ContextBuilder::new() .with_srgb(srgb) .with_vsync(true) @@ -136,14 +141,18 @@ impl Window { /// Create a new window /// /// This creates a window and fully initializes a window. - pub fn new(options: &Options, window_config: &WindowConfig) -> Result { - let event_loop = EventsLoop::new(); - + pub fn new( + event_loop: EventsLoop, + options: &Options, + window_config: &WindowConfig, + dimensions: Option, + ) -> Result { let title = options.title.as_ref().map_or(DEFAULT_NAME, |t| t); let class = options.class.as_ref().map_or(DEFAULT_NAME, |c| c); let window_builder = Window::get_platform_window(title, class, window_config); - let windowed_context = create_gl_window(window_builder.clone(), &event_loop, false) - .or_else(|_| create_gl_window(window_builder, &event_loop, true))?; + let windowed_context = + create_gl_window(window_builder.clone(), &event_loop, false, dimensions) + .or_else(|_| create_gl_window(window_builder, &event_loop, true, dimensions))?; let window = windowed_context.window(); window.show(); @@ -155,6 +164,17 @@ impl Window { } } + // Set window position + // + // TODO: replace `set_position` with `with_position` once available + // Upstream issue: https://github.com/tomaka/winit/issues/806 + let position = options.position().or_else(|| window_config.position()); + if let Some(position) = position { + let physical = PhysicalPosition::from((position.x, position.y)); + let logical = physical.to_logical(window.get_hidpi_factor()); + window.set_position(logical); + } + // Text cursor window.set_cursor(MouseCursor::Text); @@ -185,13 +205,6 @@ impl Window { self.window().set_inner_size(size); } - // TODO: use `with_position` once available - // Upstream issue: https://github.com/tomaka/winit/issues/806 - pub fn set_position(&mut self, x: i32, y: i32) { - let logical = PhysicalPosition::from((x, y)).to_logical(self.window().get_hidpi_factor()); - self.window().set_position(logical); - } - #[inline] pub fn hidpi_factor(&self) -> f64 { self.window().get_hidpi_factor()