Refactor scaling code (#793)

* refactor(scaling): Refactor scaling code, scale only font size

* chore: Add debug logging for scale factor

* fix scaling for default font

src/renderer/fonts/caching_shaper.rs

@@ -26,15 +26,17 @@ pub struct CachingShaper {
     font_loader: FontLoader,
     blob_cache: LruCache<ShapeKey, Vec<TextBlob>>,
     shape_context: ShapeContext,
+    scale_factor: f32,
 }
 
 impl CachingShaper {
-    pub fn new() -> CachingShaper {
+    pub fn new(scale_factor: f32) -> CachingShaper {
         CachingShaper {
             options: None,
-            font_loader: FontLoader::new(DEFAULT_FONT_SIZE),
+            font_loader: FontLoader::new(DEFAULT_FONT_SIZE * scale_factor),
             blob_cache: LruCache::new(10000),
             shape_context: ShapeContext::new(),
+            scale_factor,
         }
     }
 
@@ -69,13 +71,15 @@ impl CachingShaper {
             .as_ref()
             .map(|options| options.size)
             .unwrap_or(DEFAULT_FONT_SIZE)
+            * self.scale_factor
     }
 
     pub fn update_font(&mut self, guifont_setting: &str) -> bool {
         let new_options = FontOptions::parse(guifont_setting, DEFAULT_FONT_SIZE);
 
         if new_options != self.options && new_options.is_some() {
-            self.font_loader = FontLoader::new(new_options.as_ref().unwrap().size);
+            self.font_loader =
+                FontLoader::new(new_options.as_ref().unwrap().size * self.scale_factor);
             self.blob_cache.clear();
             self.options = new_options;
 

src/renderer/mod.rs

@@ -2,7 +2,7 @@ use std::collections::HashMap;
 use std::sync::mpsc::Receiver;
 use std::sync::Arc;
 
-use log::{error, trace};
+use log::error;
 use skia_safe::{colors, dash_path_effect, BlendMode, Canvas, Color, Paint, Rect};
 
 pub mod animation_utils;
@@ -54,17 +54,18 @@ pub struct Renderer {
 }
 
 impl Renderer {
-    pub fn new(batched_draw_command_receiver: Receiver<Vec<DrawCommand>>) -> Renderer {
+    pub fn new(
+        batched_draw_command_receiver: Receiver<Vec<DrawCommand>>,
+        scale_factor: f64,
+    ) -> Renderer {
         let rendered_windows = HashMap::new();
         let cursor_renderer = CursorRenderer::new();
 
         let current_mode = EditorMode::Unknown(String::from(""));
         let mut paint = Paint::new(colors::WHITE, None);
         paint.set_anti_alias(false);
-        let mut shaper = CachingShaper::new();
-        let (font_width_raw, font_height_raw) = shaper.font_base_dimensions();
-        let font_width = font_width_raw;
-        let font_height = font_height_raw;
+        let mut shaper = CachingShaper::new(scale_factor as f32);
+        let (font_width, font_height) = shaper.font_base_dimensions();
         let default_style = Arc::new(Style::new(Colors::new(
             Some(colors::WHITE),
             Some(colors::BLACK),
@@ -199,12 +200,7 @@ impl Renderer {
         canvas.restore();
     }
 
-    pub fn handle_draw_command(
-        &mut self,
-        root_canvas: &mut Canvas,
-        draw_command: DrawCommand,
-        scaling: f32,
-    ) {
+    pub fn handle_draw_command(&mut self, root_canvas: &mut Canvas, draw_command: DrawCommand) {
         match draw_command {
             DrawCommand::Window {
                 grid_id,
@@ -214,8 +210,7 @@ impl Renderer {
             }
             DrawCommand::Window { grid_id, command } => {
                 if let Some(rendered_window) = self.rendered_windows.remove(&grid_id) {
-                    let rendered_window =
-                        rendered_window.handle_window_draw_command(self, command, scaling);
+                    let rendered_window = rendered_window.handle_window_draw_command(self, command);
                     self.rendered_windows.insert(grid_id, rendered_window);
                 } else if let WindowDrawCommand::Position {
                     grid_position: (grid_left, grid_top),
@@ -230,7 +225,6 @@ impl Renderer {
                         (grid_left as f32, grid_top as f32).into(),
                         width,
                         height,
-                        scaling,
                     );
                     self.rendered_windows.insert(grid_id, new_window);
                 } else {
@@ -254,8 +248,7 @@ impl Renderer {
     }
 
     #[allow(clippy::needless_collect)]
-    pub fn draw_frame(&mut self, root_canvas: &mut Canvas, dt: f32, scaling: f32) -> bool {
-        trace!("Drawing Frame at {} scale", scaling);
+    pub fn draw_frame(&mut self, root_canvas: &mut Canvas, dt: f32) -> bool {
         let mut font_changed = false;
 
         let draw_commands: Vec<_> = self
@@ -269,7 +262,7 @@ impl Renderer {
             if let DrawCommand::FontChanged(_) = draw_command {
                 font_changed = true;
             }
-            self.handle_draw_command(root_canvas, draw_command, scaling);
+            self.handle_draw_command(root_canvas, draw_command);
         }
 
         root_canvas.clear(self.default_style.colors.background.unwrap().to_color());
@@ -277,7 +270,6 @@ impl Renderer {
         root_canvas.save();
 
         root_canvas.reset_matrix();
-        root_canvas.scale((scaling, scaling));
 
         if let Some(root_window) = self.rendered_windows.get(&1) {
             let clip_rect = root_window.pixel_region(self.font_width, self.font_height);

src/renderer/rendered_window.rs

@@ -47,10 +47,9 @@ fn build_window_surface_with_grid_size(
     renderer: &Renderer,
     grid_width: u64,
     grid_height: u64,
-    scaling: f32,
 ) -> Surface {
-    let pixel_width = ((grid_width * renderer.font_width) as f32 * scaling) as u64;
-    let pixel_height = ((grid_height * renderer.font_height) as f32 * scaling) as u64;
+    let pixel_width = ((grid_width * renderer.font_width) as f32) as u64;
+    let pixel_height = ((grid_height * renderer.font_height) as f32) as u64;
     let mut surface = build_window_surface(parent_canvas, pixel_width, pixel_height);
 
     let canvas = surface.canvas();
@@ -75,15 +74,9 @@ impl LocatedSurface {
         grid_width: u64,
         grid_height: u64,
         top_line: u64,
-        scaling: f32,
     ) -> LocatedSurface {
-        let surface = build_window_surface_with_grid_size(
-            parent_canvas,
-            renderer,
-            grid_width,
-            grid_height,
-            scaling,
-        );
+        let surface =
+            build_window_surface_with_grid_size(parent_canvas, renderer, grid_width, grid_height);
 
         LocatedSurface { surface, top_line }
     }
@@ -134,10 +127,9 @@ impl RenderedWindow {
         grid_position: Point,
         grid_width: u64,
         grid_height: u64,
-        scaling: f32,
     ) -> RenderedWindow {
         let current_surface =
-            LocatedSurface::new(parent_canvas, renderer, grid_width, grid_height, 0, scaling);
+            LocatedSurface::new(parent_canvas, renderer, grid_width, grid_height, 0);
 
         RenderedWindow {
             snapshots: VecDeque::new(),
@@ -302,7 +294,6 @@ impl RenderedWindow {
         mut self,
         renderer: &mut Renderer,
         draw_command: WindowDrawCommand,
-        scaling: f32,
     ) -> Self {
         match draw_command {
             WindowDrawCommand::Position {
@@ -334,7 +325,6 @@ impl RenderedWindow {
                         renderer,
                         grid_width,
                         grid_height,
-                        scaling,
                     );
                     old_surface.draw(
                         self.current_surface.surface.canvas(),
@@ -367,7 +357,6 @@ impl RenderedWindow {
 
                 let canvas = self.current_surface.surface.canvas();
                 canvas.save();
-                canvas.scale((scaling, scaling));
                 renderer.draw_background(canvas, grid_position, width, &style);
                 renderer.draw_foreground(canvas, &cells, grid_position, width, &style);
                 canvas.restore();
@@ -380,25 +369,24 @@ impl RenderedWindow {
                 rows,
                 cols,
             } => {
+                let font_width = renderer.font_width as f32;
+                let font_height = renderer.font_height as f32;
                 let scrolled_region = Rect::new(
-                    (left * renderer.font_width) as f32 * scaling,
-                    (top * renderer.font_height) as f32 * scaling,
-                    (right * renderer.font_width) as f32 * scaling,
-                    (bottom * renderer.font_height) as f32 * scaling,
+                    left as f32 * font_width,
+                    top as f32 * font_height,
+                    right as f32 * font_width,
+                    bottom as f32 * font_height,
                 );
 
                 let mut translated_region = scrolled_region;
-                translated_region.offset((
-                    (-cols * renderer.font_width as i64) as f32 * scaling,
-                    (-rows * renderer.font_height as i64) as f32 * scaling,
-                ));
+                translated_region.offset((-cols as f32 * font_width, -rows as f32 * font_height));
 
                 let snapshot = self.current_surface.surface.image_snapshot();
                 let canvas = self.current_surface.surface.canvas();
 
                 canvas.save();
-                canvas.clip_rect(scrolled_region, None, Some(false));
 
+                canvas.clip_rect(scrolled_region, None, Some(false));
                 canvas.draw_image_rect(
                     snapshot,
                     Some((&scrolled_region, SrcRectConstraint::Fast)),
@@ -414,7 +402,6 @@ impl RenderedWindow {
                     renderer,
                     self.grid_width,
                     self.grid_height,
-                    scaling,
                 );
 
                 self.snapshots.clear();

src/window/mod.rs

@@ -10,7 +10,7 @@ use crate::{
     settings::SETTINGS,
     INITIAL_DIMENSIONS,
 };
-use glutin::dpi::LogicalSize;
+use glutin::dpi::PhysicalSize;
 use std::sync::{atomic::AtomicBool, mpsc::Receiver, Arc};
 
 pub use window_wrapper::start_loop;
@@ -68,7 +68,7 @@ fn windows_fix_dpi() {
 }
 
 fn handle_new_grid_size(
-    new_size: LogicalSize<u32>,
+    new_size: PhysicalSize<u32>,
     renderer: &Renderer,
     ui_command_sender: &LoggingTx<UiCommand>,
 ) {
@@ -88,22 +88,14 @@ pub fn create_window(
     ui_command_sender: LoggingTx<UiCommand>,
     running: Arc<AtomicBool>,
 ) {
-    let (width, height) = window_geometry_or_default();
-
-    let renderer = Renderer::new(batched_draw_command_receiver);
-    let logical_size = (
-        (width * renderer.font_width) as u64,
-        (height * renderer.font_height + 1) as u64,
-    );
-
     #[cfg(target_os = "windows")]
     windows_fix_dpi();
 
     start_loop(
+        batched_draw_command_receiver,
         window_command_receiver,
         ui_command_sender,
         running,
-        logical_size,
-        renderer,
+        window_geometry_or_default(),
     );
 }

src/window/window_wrapper/mod.rs

@@ -13,7 +13,7 @@ use std::{
 
 use glutin::{
     self,
-    dpi::{LogicalSize, PhysicalSize},
+    dpi::PhysicalSize,
     event::{Event, WindowEvent},
     event_loop::{ControlFlow, EventLoop},
     window::{self, Fullscreen, Icon},
@@ -25,8 +25,9 @@ use glutin::platform::unix::WindowBuilderExtUnix;
 
 use super::{handle_new_grid_size, settings::WindowSettings};
 use crate::{
-    bridge::UiCommand, channel_utils::*, cmd_line::CmdLineSettings, editor::WindowCommand,
-    redraw_scheduler::REDRAW_SCHEDULER, renderer::Renderer, settings::SETTINGS,
+    bridge::UiCommand, channel_utils::*, cmd_line::CmdLineSettings, editor::DrawCommand,
+    editor::WindowCommand, redraw_scheduler::REDRAW_SCHEDULER, renderer::Renderer,
+    settings::SETTINGS,
 };
 use image::{load_from_memory, GenericImageView, Pixel};
 use keyboard_manager::KeyboardManager;
@@ -149,36 +150,25 @@ impl GlutinWindowWrapper {
 
     pub fn draw_frame(&mut self, dt: f32) {
         let window = self.windowed_context.window();
-        let scaling = window.scale_factor();
         let current_size = window.inner_size();
         let previous_size = self.saved_inner_size;
 
         if previous_size != current_size {
             self.saved_inner_size = current_size;
-            handle_new_grid_size(
-                current_size.to_logical(scaling),
-                &self.renderer,
-                &self.ui_command_sender,
-            );
+            handle_new_grid_size(current_size, &self.renderer, &self.ui_command_sender);
             self.skia_renderer.resize(&self.windowed_context);
         }
 
         let ui_command_sender = self.ui_command_sender.clone();
 
         if REDRAW_SCHEDULER.should_draw() || SETTINGS.get::<WindowSettings>().no_idle {
-            log::debug!("Render triggered using scale factor: {}", scaling);
-
             let renderer = &mut self.renderer;
 
             {
                 let canvas = self.skia_renderer.canvas();
 
-                if renderer.draw_frame(canvas, dt, scaling as f32) {
-                    handle_new_grid_size(
-                        current_size.to_logical(scaling),
-                        renderer,
-                        &ui_command_sender,
-                    );
+                if renderer.draw_frame(canvas, dt) {
+                    handle_new_grid_size(current_size, renderer, &ui_command_sender);
                 }
             }
 
@@ -189,11 +179,11 @@ impl GlutinWindowWrapper {
 }
 
 pub fn start_loop(
+    batched_draw_command_receiver: Receiver<Vec<DrawCommand>>,
     window_command_receiver: Receiver<WindowCommand>,
     ui_command_sender: LoggingTx<UiCommand>,
     running: Arc<AtomicBool>,
-    logical_size: (u64, u64),
-    renderer: Renderer,
+    initial_size: (u64, u64),
 ) {
     let icon = {
         let icon_data = Asset::get("neovide.ico").expect("Failed to read icon data");
@@ -208,11 +198,9 @@ pub fn start_loop(
     log::info!("icon created");
 
     let event_loop = EventLoop::new();
-    let (width, height) = logical_size;
-    let logical_size: LogicalSize<u32> = (width as u32, height as u32).into();
+
     let winit_window_builder = window::WindowBuilder::new()
         .with_title("Neovide")
-        .with_inner_size(logical_size)
         .with_window_icon(Some(icon))
         .with_maximized(SETTINGS.get::<CmdLineSettings>().maximized)
         .with_decorations(!SETTINGS.get::<CmdLineSettings>().frameless);
@@ -229,11 +217,25 @@ pub fn start_loop(
         .build_windowed(winit_window_builder, &event_loop)
         .unwrap();
     let windowed_context = unsafe { windowed_context.make_current().unwrap() };
-    let initial_size = windowed_context.window().inner_size();
-
-    log::info!("window created");
 
     let skia_renderer = SkiaRenderer::new(&windowed_context);
+    let scale_factor = windowed_context.window().scale_factor();
+    let renderer = Renderer::new(batched_draw_command_receiver, scale_factor);
+    let window = windowed_context.window();
+
+    let initial_inner_size =
+        get_initial_window_size(initial_size, (renderer.font_width, renderer.font_height));
+
+    window.set_inner_size(initial_inner_size);
+
+    log::info!(
+        "window created (scale_factor: {}, font_size: {}x{})",
+        scale_factor,
+        renderer.font_width,
+        renderer.font_height,
+    );
+
+    let saved_inner_size = window.inner_size();
 
     let mut window_wrapper = GlutinWindowWrapper {
         windowed_context,
@@ -243,7 +245,7 @@ pub fn start_loop(
         mouse_manager: MouseManager::new(ui_command_sender.clone()),
         title: String::from("Neovide"),
         fullscreen: false,
-        saved_inner_size: initial_size,
+        saved_inner_size,
         ui_command_sender,
         window_command_receiver,
     };
@@ -274,3 +276,12 @@ pub fn start_loop(
         *control_flow = ControlFlow::WaitUntil(previous_frame_start + frame_duration)
     });
 }
+
+fn get_initial_window_size(
+    dimensions: (u64, u64),
+    font_dimesions: (u64, u64),
+) -> PhysicalSize<u32> {
+    let (width, height) = dimensions;
+    let (font_width, font_height) = font_dimesions;
+    PhysicalSize::new((width * font_width) as u32, (height * font_height) as u32)
+}

src/window/window_wrapper/mouse_manager.rs

@@ -1,6 +1,6 @@
 use glutin::{
     self,
-    dpi::{LogicalPosition, PhysicalPosition},
+    dpi::PhysicalPosition,
     event::{ElementState, Event, MouseButton, MouseScrollDelta, WindowEvent},
     PossiblyCurrent, WindowedContext,
 };
@@ -13,12 +13,12 @@ use crate::settings::SETTINGS;
 use crate::window::WindowSettings;
 
 fn clamp_position(
-    position: LogicalPosition<f32>,
+    position: PhysicalPosition<f32>,
     region: Rect,
     font_width: u64,
     font_height: u64,
-) -> LogicalPosition<f32> {
-    LogicalPosition::new(
+) -> PhysicalPosition<f32> {
+    PhysicalPosition::new(
         position
             .x
             .min(region.right - font_width as f32)
@@ -31,11 +31,11 @@ fn clamp_position(
 }
 
 fn to_grid_coords(
-    position: LogicalPosition<f32>,
+    position: PhysicalPosition<f32>,
     font_width: u64,
     font_height: u64,
-) -> LogicalPosition<u32> {
-    LogicalPosition::new(
+) -> PhysicalPosition<u32> {
+    PhysicalPosition::new(
         (position.x as u64 / font_width) as u32,
         (position.y as u64 / font_height) as u32,
     )
@@ -45,9 +45,9 @@ pub struct MouseManager {
     command_sender: LoggingTx<UiCommand>,
     dragging: bool,
     has_moved: bool,
-    position: LogicalPosition<u32>,
-    relative_position: LogicalPosition<u32>,
-    drag_position: LogicalPosition<u32>,
+    position: PhysicalPosition<u32>,
+    relative_position: PhysicalPosition<u32>,
+    drag_position: PhysicalPosition<u32>,
     window_details_under_mouse: Option<WindowDrawDetails>,
     pub enabled: bool,
 }
@@ -58,9 +58,9 @@ impl MouseManager {
             command_sender,
             dragging: false,
             has_moved: false,
-            position: LogicalPosition::new(0, 0),
-            relative_position: LogicalPosition::new(0, 0),
-            drag_position: LogicalPosition::new(0, 0),
+            position: PhysicalPosition::new(0, 0),
+            relative_position: PhysicalPosition::new(0, 0),
+            drag_position: PhysicalPosition::new(0, 0),
             window_details_under_mouse: None,
             enabled: true,
         }
@@ -78,8 +78,7 @@ impl MouseManager {
             return;
         }
 
-        let logical_position: LogicalPosition<f32> = PhysicalPosition::new(x as u32, y as u32)
-            .to_logical(windowed_context.window().scale_factor());
+        let position: PhysicalPosition<f32> = PhysicalPosition::new(x as f32, y as f32);
 
         // If dragging, the relevant window (the one which we send all commands to) is the one
         // which the mouse drag started on. Otherwise its the top rendered window
@@ -99,10 +98,10 @@ impl MouseManager {
                 .window_regions
                 .iter()
                 .filter(|details| {
-                    logical_position.x >= details.region.left
-                        && logical_position.x < details.region.right
-                        && logical_position.y >= details.region.top
-                        && logical_position.y < details.region.bottom
+                    position.x >= details.region.left
+                        && position.x < details.region.right
+                        && position.y >= details.region.top
+                        && position.y < details.region.bottom
                 })
                 .last()
         };
@@ -111,7 +110,7 @@ impl MouseManager {
             .map(|details| details.region)
             .unwrap_or_else(|| Rect::from_wh(size.width as f32, size.height as f32));
         let clamped_position = clamp_position(
-            logical_position,
+            position,
             global_bounds,
             renderer.font_width,
             renderer.font_height,
@@ -120,7 +119,7 @@ impl MouseManager {
         self.position = to_grid_coords(clamped_position, renderer.font_width, renderer.font_height);
 
         if let Some(relevant_window_details) = relevant_window_details {
-            let relative_position = LogicalPosition::new(
+            let relative_position = PhysicalPosition::new(
                 clamped_position.x - relevant_window_details.region.left,
                 clamped_position.y - relevant_window_details.region.top,
             );
@@ -268,11 +267,11 @@ impl MouseManager {
             Event::WindowEvent {
                 event:
                     WindowEvent::MouseWheel {
-                        delta: MouseScrollDelta::PixelDelta(logical_position),
+                        delta: MouseScrollDelta::PixelDelta(position),
                         ..
                     },
                 ..
-            } => self.handle_mouse_wheel(logical_position.x as f32, logical_position.y as f32),
+            } => self.handle_mouse_wheel(position.x as f32, position.y as f32),
             Event::WindowEvent {
                 event:
                     WindowEvent::MouseInput {