Merge 48805de4d1 into 2f73dd5b59

niri-config/src/lib.rs

@@ -324,11 +324,31 @@ pub struct Output {
     #[knuffel(child, unwrap(argument, str))]
     pub mode: Option<ConfiguredMode>,
     #[knuffel(child)]
-    pub variable_refresh_rate: bool,
+    pub variable_refresh_rate: Option<Vrr>,
     #[knuffel(child, default = DEFAULT_BACKGROUND_COLOR)]
     pub background_color: Color,
 }
 
+pub trait VrrOutput {
+    fn is_always_on(&self) -> bool;
+    fn is_on_demand(&self) -> bool;
+    fn is_always_off(&self) -> bool;
+}
+
+impl VrrOutput for Output {
+    fn is_always_on(&self) -> bool {
+        self.variable_refresh_rate == Some(Vrr { on_demand: false })
+    }
+
+    fn is_on_demand(&self) -> bool {
+        self.variable_refresh_rate == Some(Vrr { on_demand: true })
+    }
+
+    fn is_always_off(&self) -> bool {
+        self.variable_refresh_rate.is_none()
+    }
+}
+
 impl Default for Output {
     fn default() -> Self {
         Self {
@@ -338,7 +358,7 @@ impl Default for Output {
             transform: Transform::Normal,
             position: None,
             mode: None,
-            variable_refresh_rate: false,
+            variable_refresh_rate: None,
             background_color: DEFAULT_BACKGROUND_COLOR,
         }
     }
@@ -352,6 +372,12 @@ pub struct Position {
     pub y: i32,
 }
 
+#[derive(knuffel::Decode, Debug, Clone, PartialEq, Default)]
+pub struct Vrr {
+    #[knuffel(property, default = false)]
+    pub on_demand: bool,
+}
+
 // MIN and MAX generics are only used during parsing to check the value.
 #[derive(Debug, Default, Clone, Copy, PartialEq)]
 pub struct FloatOrInt<const MIN: i32, const MAX: i32>(pub f64);
@@ -896,6 +922,8 @@ pub struct WindowRule {
     pub clip_to_geometry: Option<bool>,
     #[knuffel(child, unwrap(argument))]
     pub block_out_from: Option<BlockOutFrom>,
+    #[knuffel(child, unwrap(argument))]
+    pub variable_refresh_rate: Option<bool>,
 }
 
 // Remember to update the PartialEq impl when adding fields!
@@ -2705,7 +2733,7 @@ mod tests {
                 transform "flipped-90"
                 position x=10 y=20
                 mode "1920x1080@144"
-                variable-refresh-rate
+                variable-refresh-rate on-demand=true
                 background-color "rgba(25, 25, 102, 1.0)"
             }
 
@@ -2889,7 +2917,7 @@ mod tests {
                         height: 1080,
                         refresh: Some(144.),
                     }),
-                    variable_refresh_rate: true,
+                    variable_refresh_rate: Some(Vrr { on_demand: true }),
                     background_color: Color::from_rgba8_unpremul(25, 25, 102, 255),
                 }]),
                 layout: Layout {

niri-ipc/src/lib.rs

@@ -352,18 +352,11 @@ pub enum OutputAction {
         #[cfg_attr(feature = "clap", command(subcommand))]
         position: PositionToSet,
     },
-    /// Toggle variable refresh rate.
+    /// Set the variable refresh rate mode.
     Vrr {
-        /// Whether to enable variable refresh rate.
-        #[cfg_attr(
-            feature = "clap",
-            arg(
-                value_name = "ON|OFF",
-                action = clap::ArgAction::Set,
-                value_parser = clap::builder::BoolishValueParser::new(),
-            ),
-        )]
-        enable: bool,
+        /// Variable refresh rate mode to set.
+        #[cfg_attr(feature = "clap", command(flatten))]
+        vrr: VrrToSet,
     },
 }
 
@@ -425,6 +418,27 @@ pub struct ConfiguredPosition {
     pub y: i32,
 }
 
+/// Output VRR to set.
+#[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq)]
+#[cfg_attr(feature = "clap", derive(clap::Args))]
+#[cfg_attr(feature = "json-schema", derive(schemars::JsonSchema))]
+pub struct VrrToSet {
+    /// Whether to enable variable refresh rate.
+    #[cfg_attr(
+        feature = "clap",
+        arg(
+            value_name = "ON|OFF",
+            action = clap::ArgAction::Set,
+            value_parser = clap::builder::BoolishValueParser::new(),
+            hide_possible_values = true,
+        ),
+    )]
+    pub vrr: bool,
+    /// Only enable if there is a matched VRR window rule visible on the output.
+    #[cfg_attr(feature = "clap", arg(long))]
+    pub on_demand: bool,
+}
+
 /// Connected output.
 #[derive(Debug, Serialize, Deserialize, Clone)]
 #[cfg_attr(feature = "json-schema", derive(schemars::JsonSchema))]

src/backend/mod.rs

@@ -153,6 +153,13 @@ impl Backend {
         }
     }
 
+    pub fn set_output_vrr(&mut self, niri: &mut Niri, output: &Output, enable_vrr: bool) {
+        match self {
+            Backend::Tty(tty) => tty.set_output_vrr(niri, output, enable_vrr),
+            Backend::Winit(_) => (),
+        }
+    }
+
     pub fn on_output_config_changed(&mut self, niri: &mut Niri) {
         match self {
             Backend::Tty(tty) => tty.on_output_config_changed(niri),

src/backend/tty.rs

@@ -14,7 +14,7 @@ use std::{io, mem};
 use anyhow::{anyhow, bail, ensure, Context};
 use bytemuck::cast_slice_mut;
 use libc::dev_t;
-use niri_config::Config;
+use niri_config::{Config, VrrOutput};
 use smithay::backend::allocator::dmabuf::Dmabuf;
 use smithay::backend::allocator::format::FormatSet;
 use smithay::backend::allocator::gbm::{GbmAllocator, GbmBufferFlags, GbmDevice};
@@ -832,15 +832,13 @@ impl Tty {
         let mut vrr_enabled = false;
         if let Some(capable) = is_vrr_capable(&device.drm, connector.handle()) {
             if capable {
-                let word = if config.variable_refresh_rate {
-                    "enabling"
-                } else {
-                    "disabling"
-                };
+                // Even if on-demand, we still disable it until later checks.
+                let vrr = config.is_always_on();
+                let word = if vrr { "enabling" } else { "disabling" };
 
-                match set_vrr_enabled(&device.drm, crtc, config.variable_refresh_rate) {
+                match set_vrr_enabled(&device.drm, crtc, vrr) {
                     Ok(enabled) => {
-                        if enabled != config.variable_refresh_rate {
+                        if enabled != vrr {
                             warn!("failed {} VRR", word);
                         }
 
@@ -851,13 +849,13 @@ impl Tty {
                     }
                 }
             } else {
-                if config.variable_refresh_rate {
+                if !config.is_always_off() {
                     warn!("cannot enable VRR because connector is not vrr_capable");
                 }
 
                 // Try to disable it anyway to work around a bug where resetting DRM state causes
                 // vrr_capable to be reset to 0, potentially leaving VRR_ENABLED at 1.
-                let res = set_vrr_enabled(&device.drm, crtc, config.variable_refresh_rate);
+                let res = set_vrr_enabled(&device.drm, crtc, false);
                 if matches!(res, Ok(true)) {
                     warn!("error disabling VRR");
 
@@ -865,7 +863,7 @@ impl Tty {
                     vrr_enabled = true;
                 }
             }
-        } else if config.variable_refresh_rate {
+        } else if !config.is_always_off() {
             warn!("cannot enable VRR because connector is not vrr_capable");
         }
 
@@ -1661,6 +1659,39 @@ impl Tty {
         }
     }
 
+    pub fn set_output_vrr(&mut self, niri: &mut Niri, output: &Output, enable_vrr: bool) {
+        let _span = tracy_client::span!("Tty::set_output_vrr");
+        for (&node, device) in self.devices.iter_mut() {
+            for (&crtc, surface) in device.surfaces.iter_mut() {
+                let tty_state: &TtyOutputState = output.user_data().get().unwrap();
+                if tty_state.node == node && tty_state.crtc == crtc {
+                    let Some(connector) =
+                        surface.compositor.pending_connectors().into_iter().next()
+                    else {
+                        error!("surface pending connectors is empty");
+                        return;
+                    };
+                    let Some(connector) = device.drm_scanner.connectors().get(&connector) else {
+                        error!("missing enabled connector in drm_scanner");
+                        return;
+                    };
+                    let output_state = niri.output_state.get_mut(output).unwrap();
+
+                    try_to_change_vrr(
+                        &device.drm,
+                        connector,
+                        crtc,
+                        surface,
+                        output_state,
+                        enable_vrr,
+                    );
+                    self.refresh_ipc_outputs(niri);
+                    return;
+                }
+            }
+        }
+    }
+
     pub fn on_output_config_changed(&mut self, niri: &mut Niri) {
         let _span = tracy_client::span!("Tty::on_output_config_changed");
 
@@ -1707,7 +1738,7 @@ impl Tty {
                 };
 
                 let change_mode = surface.compositor.pending_mode() != mode;
-                let change_vrr = surface.vrr_enabled != config.variable_refresh_rate;
+                let change_vrr = surface.vrr_enabled != config.is_always_on();
                 if !change_mode && !change_vrr {
                     continue;
                 }
@@ -1736,7 +1767,7 @@ impl Tty {
                         crtc,
                         surface,
                         output_state,
-                        config.variable_refresh_rate,
+                        !surface.vrr_enabled,
                     );
                 }
 

src/frame_clock.rs

@@ -40,6 +40,10 @@ impl FrameClock {
         self.last_presentation_time = None;
     }
 
+    pub fn vrr(&self) -> bool {
+        self.vrr
+    }
+
     pub fn presented(&mut self, presentation_time: Duration) {
         if presentation_time.is_zero() {
             // Not interested in these.

src/niri.rs

@@ -12,7 +12,7 @@ use _server_decoration::server::org_kde_kwin_server_decoration_manager::Mode as
 use anyhow::{bail, ensure, Context};
 use calloop::futures::Scheduler;
 use niri_config::{
-    Config, FloatOrInt, Key, Modifiers, PreviewRender, TrackLayout, WorkspaceReference,
+    Config, FloatOrInt, Key, Modifiers, PreviewRender, TrackLayout, VrrOutput, WorkspaceReference,
     DEFAULT_BACKGROUND_COLOR,
 };
 use niri_ipc::Workspace;
@@ -1202,8 +1202,14 @@ impl State {
                         }
                     }
                 }
-                niri_ipc::OutputAction::Vrr { enable } => {
-                    config.variable_refresh_rate = enable;
+                niri_ipc::OutputAction::Vrr { vrr } => {
+                    config.variable_refresh_rate = if vrr.vrr {
+                        Some(niri_config::Vrr {
+                            on_demand: vrr.on_demand,
+                        })
+                    } else {
+                        None
+                    }
                 }
             }
         }
@@ -3089,6 +3095,8 @@ impl Niri {
             lock_state => self.lock_state = lock_state,
         }
 
+        self.refresh_on_demand_vrr(backend, output);
+
         // Send the frame callbacks.
         //
         // FIXME: The logic here could be a bit smarter. Currently, during an animation, the
@@ -3118,6 +3126,49 @@ impl Niri {
         });
     }
 
+    pub fn refresh_on_demand_vrr(&mut self, backend: &mut Backend, output: &Output) {
+        let _span = tracy_client::span!("Niri::refresh_on_demand_vrr");
+        let Some(on_demand) = self
+            .config
+            .borrow()
+            .outputs
+            .find(&output.name())
+            .map(|output| output.is_on_demand())
+        else {
+            warn!("error getting output config for {}", output.name());
+            return;
+        };
+        if on_demand {
+            let last = self.output_state[output].frame_clock.vrr();
+            let current = self.layout.windows_for_output(output).any(|mapped| {
+                mapped.rules().variable_refresh_rate == Some(true) && {
+                    let mut visible = false;
+                    mapped.window.with_surfaces(|_, states| {
+                        if !visible {
+                            let surface_primary_scanout_output = states
+                                .data_map
+                                .get_or_insert_threadsafe(Mutex::<PrimaryScanoutOutput>::default);
+                            if surface_primary_scanout_output
+                                .lock()
+                                .unwrap()
+                                .current_output()
+                                .as_ref()
+                                == Some(output)
+                            {
+                                visible = true;
+                            }
+                        }
+                    });
+                    visible
+                }
+            });
+
+            if last != current {
+                backend.set_output_vrr(self, output, current);
+            }
+        }
+    }
+
     pub fn update_primary_scanout_output(
         &self,
         output: &Output,
@@ -3181,13 +3232,9 @@ impl Niri {
             let offscreen_id = offscreen_id.as_ref();
 
             win.with_surfaces(|surface, states| {
-                states
-                    .data_map
-                    .insert_if_missing_threadsafe(Mutex::<PrimaryScanoutOutput>::default);
                 let surface_primary_scanout_output = states
                     .data_map
-                    .get::<Mutex<PrimaryScanoutOutput>>()
-                    .unwrap();
+                    .get_or_insert_threadsafe(Mutex::<PrimaryScanoutOutput>::default);
                 surface_primary_scanout_output
                     .lock()
                     .unwrap()

src/protocols/output_management.rs

@@ -3,7 +3,7 @@ use std::collections::HashMap;
 use std::iter::zip;
 use std::mem;
 
-use niri_config::FloatOrInt;
+use niri_config::{FloatOrInt, Vrr};
 use niri_ipc::Transform;
 use smithay::reexports::wayland_protocols_wlr::output_management::v1::server::{
     zwlr_output_configuration_head_v1, zwlr_output_configuration_v1, zwlr_output_head_v1,
@@ -693,9 +693,9 @@ where
                 new_config.scale = Some(FloatOrInt(scale));
             }
             zwlr_output_configuration_head_v1::Request::SetAdaptiveSync { state } => {
-                let enabled = match state {
-                    WEnum::Value(AdaptiveSyncState::Enabled) => true,
-                    WEnum::Value(AdaptiveSyncState::Disabled) => false,
+                let vrr = match state {
+                    WEnum::Value(AdaptiveSyncState::Enabled) => Some(Vrr { on_demand: false }),
+                    WEnum::Value(AdaptiveSyncState::Disabled) => None,
                     _ => {
                         warn!("SetAdaptativeSync: unknown requested adaptative sync");
                         conf_head.post_error(
@@ -705,7 +705,7 @@ where
                         return;
                     }
                 };
-                new_config.variable_refresh_rate = enabled;
+                new_config.variable_refresh_rate = vrr;
             }
             _ => unreachable!(),
         }

src/window/mod.rs

@@ -72,6 +72,9 @@ pub struct ResolvedWindowRules {
 
     /// Whether to block out this window from certain render targets.
     pub block_out_from: Option<BlockOutFrom>,
+
+    /// Whether to enable VRR on this window's primary output if it is on-demand.
+    pub variable_refresh_rate: Option<bool>,
 }
 
 impl<'a> WindowRef<'a> {
@@ -132,6 +135,7 @@ impl ResolvedWindowRules {
             geometry_corner_radius: None,
             clip_to_geometry: None,
             block_out_from: None,
+            variable_refresh_rate: None,
         }
     }
 
@@ -231,6 +235,9 @@ impl ResolvedWindowRules {
                 if let Some(x) = rule.block_out_from {
                     resolved.block_out_from = Some(x);
                 }
+                if let Some(x) = rule.variable_refresh_rate {
+                    resolved.variable_refresh_rate = Some(x);
+                }
             }
 
             resolved.open_on_output = open_on_output.map(|x| x.to_owned());

wiki/Configuration:-Named-Workspaces.md

@@ -24,7 +24,7 @@ workspace "chat" {
     open-on-output "DP-2"
 }
 
-// Open Fractal on the "chat" workspace at niri startup.
+// Open Fractal on the "chat" workspace, if it runs at niri startup.
 window-rule {
     match at-startup=true app-id=r#"^org\.gnome\.Fractal$"#
     open-on-workspace "chat"

wiki/Design-Principles.md

@@ -22,3 +22,9 @@ And here are some more principles I try to follow throughout niri.
 1. Eye-candy features should not cause unreasonable excessive rendering.
     - For example, clip-to-geometry will prevent direct scanout in many cases (since the window surface is not completely visible). But in the cases where the surface or the subsurface *is* completely visible (fully within the clipped region), it will still allow for direct scanout.
     - For example, animations *can* cause damage and even draw to an offscreen every frame, because they are expected to be short (and can be disabled). However, something like the rounded corners shader should not offscreen or cause excessive damage every frame, because it is long-running and constantly active.
+1. Be mindful of invisible state.
+
+    This is niri state that is not immediately apparent from looking at the screen. This is not bad per se, but you should carefully consider how to reduce the surprise factor.
+
+    - For example, when a monitor disconnects, all its workspaces move to another connected monitor. In order to be able to restore these workspaces when the first monitor connects again, these workspaces keep the knowledge of which was their *original monitor*—this is an example of invisible state, since you can't tell it in any way by looking at the screen. This can have surprising consequences: imagine disconnecting a monitor at home, going to work, completely rearranging the windows there, then coming back home, and suddenly some random workspaces end up on your home monitor. In order to reduce this surprise factor, whenever a new window appears on a workspace, that workspace resets its *original monitor* to its current monitor. This way, the workspaces you actively worked on remain where they were.
+    - For example, niri preserves the view position whenever a window appears, or whenever a window goes full-screen, to restore it afterward. This way, dealing with temporary things like dialogs opening and closing, or toggling full-screen, becomes less annoying, since it doesn't mess up the view position. This is also invisible state, as you cannot tell by looking at the screen where closing a window will restore the view position. If taken to the extreme (previous view position saved forever for every open window), this can be surprising, as closing long-running windows would result in the view shifting around pretty much randomly. To reduce this surprise factor, niri remembers only one last view position per workspace, and forgets this stored view position upon window focus change.