Desktop: Add an 'Enable V-Sync' preference on Mac (#3887)

* add vsync pref

* account for physical scale in pixel preview passthru check

* change allow to expect attr

* Update user-facing v-sync text

---------

Co-authored-by: Keavon Chambers <keavon@keavon.com>

desktop/src/app.rs

@@ -21,7 +21,7 @@ use crate::persist::PersistentData;
 use crate::preferences;
 use crate::render::{RenderError, RenderState};
 use crate::window::Window;
-use crate::wrapper::messages::{DesktopFrontendMessage, DesktopWrapperMessage, InputMessage, MouseKeys, MouseState};
+use crate::wrapper::messages::{DesktopFrontendMessage, DesktopWrapperMessage, InputMessage, MouseKeys, MouseState, Preferences};
 use crate::wrapper::{DesktopWrapper, NodeGraphExecutionResult, WgpuContext, serialize_frontend_messages};
 
 pub(crate) struct App {
@@ -46,6 +46,8 @@ pub(crate) struct App {
 	web_communication_initialized: bool,
 	web_communication_startup_buffer: Vec<Vec<u8>>,
 	persistent_data: PersistentData,
+	#[cfg_attr(not(target_os = "macos"), expect(unused))]
+	preferences: Preferences,
 	cli: Cli,
 	startup_time: Option<Instant>,
 	exiting: Arc<AtomicBool>,
@@ -63,6 +65,7 @@ impl App {
 		wgpu_context: WgpuContext,
 		app_event_receiver: Receiver<AppEvent>,
 		app_event_scheduler: AppEventScheduler,
+		preferences: Preferences,
 		cli: Cli,
 	) -> Self {
 		let ctrlc_app_event_scheduler = app_event_scheduler.clone();
@@ -116,6 +119,7 @@ impl App {
 			web_communication_initialized: false,
 			web_communication_startup_buffer: Vec::new(),
 			persistent_data,
+			preferences,
 			cli,
 			startup_time: None,
 			exiting,
@@ -516,7 +520,12 @@ impl ApplicationHandler for App {
 		let window = Window::new(event_loop, self.app_event_scheduler.clone());
 		self.window = Some(window);
 
-		let render_state = RenderState::new(self.window.as_ref().unwrap(), self.wgpu_context.clone());
+		#[cfg(not(target_os = "macos"))]
+		let present_mode = None;
+		#[cfg(target_os = "macos")]
+		let present_mode = if !self.preferences.vsync { Some(wgpu::PresentMode::Immediate) } else { None };
+
+		let render_state = RenderState::new(self.window.as_ref().unwrap(), self.wgpu_context.clone(), present_mode);
 		self.render_state = Some(render_state);
 
 		if let Some(window) = &self.window.as_ref() {

desktop/src/lib.rs

@@ -62,6 +62,8 @@ pub fn start() {
 		}
 	};
 
+	let prefs = preferences::read();
+
 	// Must be called before event loop initialization or native window integrations will break
 	App::init();
 
@@ -73,7 +75,7 @@ pub fn start() {
 
 	let (cef_view_info_sender, cef_view_info_receiver) = std::sync::mpsc::channel();
 
-	let disable_ui_acceleration = preferences::read().disable_ui_acceleration || cli.disable_ui_acceleration;
+	let disable_ui_acceleration = prefs.disable_ui_acceleration || cli.disable_ui_acceleration;
 	if disable_ui_acceleration {
 		println!("UI acceleration is disabled");
 	}
@@ -95,7 +97,7 @@ pub fn start() {
 		}
 	};
 
-	let app = App::new(Box::new(cef_context), cef_view_info_sender, wgpu_context, app_event_receiver, app_event_scheduler, cli);
+	let app = App::new(Box::new(cef_context), cef_view_info_sender, wgpu_context, app_event_receiver, app_event_scheduler, prefs, cli);
 
 	let exit_reason = app.run(event_loop);
 
@@ -111,15 +113,15 @@ pub fn start() {
 	drop(lock);
 
 	match exit_reason {
-		#[cfg(target_os = "linux")]
 		app::ExitReason::Restart | app::ExitReason::UiAccelerationFailure => {
 			tracing::error!("Restarting application");
 			let mut command = std::process::Command::new(std::env::current_exe().unwrap());
 			#[cfg(target_family = "unix")]
 			let _ = std::os::unix::process::CommandExt::exec(&mut command);
+			#[cfg(target_family = "unix")]
+			tracing::error!("Failed to restart application");
 			#[cfg(not(target_family = "unix"))]
 			let _ = command.spawn();
-			tracing::error!("Failed to restart application");
 		}
 		_ => {}
 	}

desktop/src/render/state.rs

@@ -1,4 +1,5 @@
 use std::borrow::Cow;
+use wgpu::PresentMode;
 
 use crate::window::Window;
 use crate::wrapper::{TargetTexture, WgpuContext, WgpuExecutor};
@@ -27,7 +28,7 @@ pub(crate) struct RenderState {
 }
 
 impl RenderState {
-	pub(crate) fn new(window: &Window, context: WgpuContext) -> Self {
+	pub(crate) fn new(window: &Window, context: WgpuContext, present_mode: Option<PresentMode>) -> Self {
 		let size = window.surface_size();
 		let surface = window.create_surface(context.instance.clone());
 
@@ -39,10 +40,7 @@ impl RenderState {
 			format: surface_format,
 			width: size.width,
 			height: size.height,
-			#[cfg(not(target_os = "macos"))]
-			present_mode: surface_caps.present_modes[0],
-			#[cfg(target_os = "macos")]
-			present_mode: wgpu::PresentMode::Immediate,
+			present_mode: present_mode.unwrap_or(surface_caps.present_modes[0]),
 			alpha_mode: surface_caps.alpha_modes[0],
 			view_formats: vec![],
 			desired_maximum_frame_latency: 1,

desktop/src/window/mac/menu.rs

@@ -77,7 +77,7 @@ fn menu_items_from_wrapper(entries: Vec<WrapperMenuItem>) -> Vec<MenuItemKind> {
 			}
 			WrapperMenuItem::SubMenu { text: name, items, .. } => {
 				let items = menu_items_from_wrapper(items);
-				let items = items.iter().map(|item| menu_item_kind_to_dyn(item)).collect::<Vec<&dyn IsMenuItem>>();
+				let items = items.iter().map(menu_item_kind_to_dyn).collect::<Vec<&dyn IsMenuItem>>();
 				let submenu = Submenu::with_items(name, true, &items).unwrap();
 				menu_items.push(MenuItemKind::Submenu(submenu));
 			}
@@ -106,7 +106,7 @@ fn replace_children<'a, T: Into<MenuContainer<'a>>>(menu: T, new_items: Vec<Menu
 	for item in items.iter() {
 		menu.remove(menu_item_kind_to_dyn(item)).unwrap();
 	}
-	let items = new_items.iter().map(|item| menu_item_kind_to_dyn(item)).collect::<Vec<&dyn IsMenuItem>>();
+	let items = new_items.iter().map(menu_item_kind_to_dyn).collect::<Vec<&dyn IsMenuItem>>();
 	menu.append_items(items.as_ref()).unwrap();
 }
 

desktop/wrapper/src/utils.rs

@@ -19,7 +19,7 @@ pub(crate) mod menu {
 			panic!("Menu bar layout group is supposed to be a row");
 		};
 		widgets
-			.into_iter()
+			.iter()
 			.map(|widget| {
 				let text_button = match widget.widget.as_ref() {
 					Widget::TextButton(text_button) => text_button,
@@ -79,7 +79,7 @@ pub(crate) mod menu {
 		let enabled = !*disabled;
 
 		if !children.is_empty() {
-			let items = convert_menu_bar_entry_children_to_menu_items(&children, root_widget_id, path.clone());
+			let items = convert_menu_bar_entry_children_to_menu_items(children, root_widget_id, path.clone());
 			return MenuItem::SubMenu { id, text, enabled, items };
 		}
 

editor/src/messages/dialog/preferences_dialog/preferences_dialog_message_handler.rs

@@ -387,9 +387,44 @@ impl PreferencesDialogMessageHandler {
 
 				rows.push(ui_acceleration);
 			}
+
+			#[cfg(target_os = "macos")]
+			{
+				let vsync_description = "
+					Render frames with vertical synchronization (v-sync) to prevent visual tearing within Graphite and the operating system compositor. This introduces increased input latency which is more noticeable on lower refresh rate displays. Future versions of Graphite will aim to reduce the macOS-specific latency without tearing artifacts.\n\
+					\n\
+					The application will restart for this change to take effect.\n\
+					\n\
+					*Default: Off.*
+					"
+				.trim();
+
+				let checkbox_id = CheckboxId::new();
+				let vsync_checked = preferences.vsync;
+
+				let vsync = vec![
+					Separator::new(SeparatorStyle::Unrelated).widget_instance(),
+					Separator::new(SeparatorStyle::Unrelated).widget_instance(),
+					CheckboxInput::new(vsync_checked)
+						.tooltip_label("Enable V-Sync")
+						.tooltip_description(vsync_description)
+						.on_update(|checkbox_input: &CheckboxInput| Message::Batched {
+							messages: Box::new([PreferencesDialogMessage::MayRequireRestart.into(), PreferencesMessage::VSync { vsync: checkbox_input.checked }.into()]),
+						})
+						.for_label(checkbox_id)
+						.widget_instance(),
+					TextLabel::new("Enable V-Sync")
+						.tooltip_label("Enable V-Sync")
+						.tooltip_description(vsync_description)
+						.for_checkbox(checkbox_id)
+						.widget_instance(),
+				];
+
+				rows.push(vsync);
+			}
 		}
 
-		Layout(rows.into_iter().map(|r| LayoutGroup::row(r)).collect())
+		Layout(rows.into_iter().map(LayoutGroup::row).collect())
 	}
 
 	pub fn send_layout(&self, responses: &mut VecDeque<Message>, layout_target: LayoutTarget, preferences: &PreferencesMessageHandler) {

editor/src/messages/preferences/preferences_message.rs

@@ -6,16 +6,38 @@ use crate::messages::prelude::*;
 #[derive(PartialEq, Clone, Debug, serde::Serialize, serde::Deserialize)]
 pub enum PreferencesMessage {
 	// Management messages
-	Load { preferences: PreferencesMessageHandler },
+	Load {
+		preferences: PreferencesMessageHandler,
+	},
 	ResetToDefaults,
 
 	// Per-preference messages
-	SelectionMode { selection_mode: SelectionMode },
-	BrushTool { enabled: bool },
-	ModifyLayout { zoom_with_scroll: bool },
-	GraphWireStyle { style: GraphWireStyle },
-	ViewportZoomWheelRate { rate: f64 },
-	UIScale { scale: f64 },
-	DisableUIAcceleration { disable_ui_acceleration: bool },
-	MaxRenderRegionSize { size: u32 },
+	SelectionMode {
+		selection_mode: SelectionMode,
+	},
+	BrushTool {
+		enabled: bool,
+	},
+	ModifyLayout {
+		zoom_with_scroll: bool,
+	},
+	GraphWireStyle {
+		style: GraphWireStyle,
+	},
+	ViewportZoomWheelRate {
+		rate: f64,
+	},
+	UIScale {
+		scale: f64,
+	},
+	MaxRenderRegionSize {
+		size: u32,
+	},
+	DisableUIAcceleration {
+		disable_ui_acceleration: bool,
+	},
+	#[cfg(target_os = "macos")]
+	VSync {
+		vsync: bool,
+	},
 }

editor/src/messages/preferences/preferences_message_handler.rs

@@ -21,15 +21,23 @@ pub struct PreferencesMessageHandler {
 	pub graph_wire_style: GraphWireStyle,
 	pub viewport_zoom_wheel_rate: f64,
 	pub ui_scale: f64,
-	pub disable_ui_acceleration: bool,
 	pub max_render_region_size: u32,
+	pub disable_ui_acceleration: bool,
+	#[cfg(target_os = "macos")]
+	pub vsync: bool,
 }
 
 impl PreferencesMessageHandler {
+	#[cfg(not(target_os = "macos"))]
 	pub fn needs_restart(&self, other: &Self) -> bool {
 		self.disable_ui_acceleration != other.disable_ui_acceleration
 	}
 
+	#[cfg(target_os = "macos")]
+	pub fn needs_restart(&self, other: &Self) -> bool {
+		self.disable_ui_acceleration != other.disable_ui_acceleration || self.vsync != other.vsync
+	}
+
 	pub fn get_selection_mode(&self) -> SelectionMode {
 		self.selection_mode
 	}
@@ -54,8 +62,10 @@ impl Default for PreferencesMessageHandler {
 			graph_wire_style: GraphWireStyle::default(),
 			viewport_zoom_wheel_rate: VIEWPORT_ZOOM_WHEEL_RATE,
 			ui_scale: UI_SCALE_DEFAULT,
-			disable_ui_acceleration: false,
 			max_render_region_size: EditorPreferences::default().max_render_region_size,
+			disable_ui_acceleration: false,
+			#[cfg(target_os = "macos")]
+			vsync: false,
 		}
 	}
 }
@@ -112,14 +122,18 @@ impl MessageHandler<PreferencesMessage, PreferencesMessageContext<'_>> for Prefe
 				self.ui_scale = scale;
 				responses.add(FrontendMessage::UpdateUIScale { scale: self.ui_scale });
 			}
-			PreferencesMessage::DisableUIAcceleration { disable_ui_acceleration } => {
-				self.disable_ui_acceleration = disable_ui_acceleration;
-			}
 			PreferencesMessage::MaxRenderRegionSize { size } => {
 				self.max_render_region_size = size;
 				responses.add(PortfolioMessage::EditorPreferences);
 				responses.add(NodeGraphMessage::RunDocumentGraph);
 			}
+			PreferencesMessage::DisableUIAcceleration { disable_ui_acceleration } => {
+				self.disable_ui_acceleration = disable_ui_acceleration;
+			}
+			#[cfg(target_os = "macos")]
+			PreferencesMessage::VSync { vsync } => {
+				self.vsync = vsync;
+			}
 		}
 
 		responses.add(FrontendMessage::TriggerSavePreferences { preferences: self.clone() });

node-graph/nodes/gstd/src/pixel_preview.rs

@@ -22,7 +22,7 @@ pub async fn pixel_preview<'a: 'n>(
 	let physical_scale = render_params.scale;
 
 	let footprint = *ctx.footprint();
-	let viewport_zoom = footprint.decompose_scale().x;
+	let viewport_zoom = footprint.decompose_scale().x * physical_scale;
 
 	if render_params.render_mode != RenderMode::PixelPreview || !matches!(render_params.render_output_type, RenderOutputTypeRequest::Vello) || viewport_zoom <= 1. {
 		let context = OwnedContextImpl::from(ctx).into_context();