rust-api: Add custom modelines

Still need to do the Lua side

api/protocol/pinnacle/output/v0alpha1/output.proto

@@ -36,6 +36,21 @@ message SetModeRequest {
   optional uint32 refresh_rate_millihz = 4;
 }
 
+message SetModelineRequest {
+  optional string output_name = 1;
+  optional float clock = 2;
+  optional uint32 hdisplay = 3;
+  optional uint32 hsync_start = 4;
+  optional uint32 hsync_end = 5;
+  optional uint32 htotal = 6;
+  optional uint32 vdisplay = 7;
+  optional uint32 vsync_start = 8;
+  optional uint32 vsync_end = 9;
+  optional uint32 vtotal = 10;
+  optional bool hsync_pos = 11;
+  optional bool vsync_pos = 12;
+}
+
 message SetScaleRequest {
   optional string output_name = 1;
   oneof absolute_or_relative {
@@ -106,6 +121,7 @@ message GetPropertiesResponse {
 service OutputService {
   rpc SetLocation(SetLocationRequest) returns (google.protobuf.Empty);
   rpc SetMode(SetModeRequest) returns (google.protobuf.Empty);
+  rpc SetModeline(SetModelineRequest) returns (google.protobuf.Empty);
   rpc SetScale(SetScaleRequest) returns (google.protobuf.Empty);
   rpc SetTransform(SetTransformRequest) returns (google.protobuf.Empty);
   rpc SetPowered(SetPoweredRequest) returns (google.protobuf.Empty);

api/rust/src/output.rs

@@ -9,14 +9,14 @@
 //! This module provides [`Output`], which allows you to get [`OutputHandle`]s for different
 //! connected monitors and set them up.
 
-use std::{num::NonZeroU32, sync::OnceLock};
+use std::{num::NonZeroU32, str::FromStr, sync::OnceLock};
 
 use futures::FutureExt;
 use pinnacle_api_defs::pinnacle::output::{
     self,
     v0alpha1::{
         output_service_client::OutputServiceClient, set_scale_request::AbsoluteOrRelative,
-        SetLocationRequest, SetModeRequest, SetPoweredRequest, SetScaleRequest,
+        SetLocationRequest, SetModeRequest, SetModelineRequest, SetPoweredRequest, SetScaleRequest,
         SetTransformRequest,
     },
 };
@@ -812,6 +812,37 @@ impl OutputHandle {
         .unwrap();
     }
 
+    /// Set a custom modeline for this output.
+    ///
+    /// See `xorg.conf(5)` for more information.
+    ///
+    /// You can parse a modeline from a string of the form
+    /// `<clock> <hdisplay> <hsync_start> <hsync_end> <htotal> <vdisplay> <vsync_start> <vsync_end> <hsync> <vsync>`.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// output.set_modeline("173.00 1920 2048 2248 2576 1080 1083 1088 1120 -hsync +vsync".parse()?);
+    /// ```
+    pub fn set_modeline(&self, modeline: Modeline) {
+        let mut client = self.output_client.clone();
+        block_on_tokio(client.set_modeline(SetModelineRequest {
+            output_name: Some(self.name.clone()),
+            clock: Some(modeline.clock),
+            hdisplay: Some(modeline.hdisplay),
+            hsync_start: Some(modeline.hsync_start),
+            hsync_end: Some(modeline.hsync_end),
+            htotal: Some(modeline.htotal),
+            vdisplay: Some(modeline.vdisplay),
+            vsync_start: Some(modeline.vsync_start),
+            vsync_end: Some(modeline.vsync_end),
+            vtotal: Some(modeline.vtotal),
+            hsync_pos: Some(modeline.hsync),
+            vsync_pos: Some(modeline.vsync),
+        }))
+        .unwrap();
+    }
+
     /// Set this output's scaling factor.
     ///
     /// # Examples
@@ -1262,3 +1293,151 @@ pub struct OutputProperties {
     /// This output's window keyboard focus stack.
     pub keyboard_focus_stack: Vec<WindowHandle>,
 }
+
+/// A custom modeline.
+#[allow(missing_docs)]
+#[derive(Copy, Clone, Debug, PartialEq, Default)]
+pub struct Modeline {
+    pub clock: f32,
+    pub hdisplay: u32,
+    pub hsync_start: u32,
+    pub hsync_end: u32,
+    pub htotal: u32,
+    pub vdisplay: u32,
+    pub vsync_start: u32,
+    pub vsync_end: u32,
+    pub vtotal: u32,
+    pub hsync: bool,
+    pub vsync: bool,
+}
+
+/// Error for the `FromStr` implementation for [`Modeline`].
+#[derive(Debug)]
+pub struct ParseModelineError(ParseModelineErrorKind);
+
+#[derive(Debug)]
+enum ParseModelineErrorKind {
+    NoClock,
+    InvalidClock,
+    NoHdisplay,
+    InvalidHdisplay,
+    NoHsyncStart,
+    InvalidHsyncStart,
+    NoHsyncEnd,
+    InvalidHsyncEnd,
+    NoHtotal,
+    InvalidHtotal,
+    NoVdisplay,
+    InvalidVdisplay,
+    NoVsyncStart,
+    InvalidVsyncStart,
+    NoVsyncEnd,
+    InvalidVsyncEnd,
+    NoVtotal,
+    InvalidVtotal,
+    NoHsync,
+    InvalidHsync,
+    NoVsync,
+    InvalidVsync,
+}
+
+impl std::fmt::Display for ParseModelineError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        std::fmt::Debug::fmt(&self.0, f)
+    }
+}
+
+impl From<ParseModelineErrorKind> for ParseModelineError {
+    fn from(value: ParseModelineErrorKind) -> Self {
+        Self(value)
+    }
+}
+
+impl FromStr for Modeline {
+    type Err = ParseModelineError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        let mut args = s.split_whitespace();
+
+        let clock = args
+            .next()
+            .ok_or(ParseModelineErrorKind::NoClock)?
+            .parse()
+            .map_err(|_| ParseModelineErrorKind::InvalidClock)?;
+        let hdisplay = args
+            .next()
+            .ok_or(ParseModelineErrorKind::NoHdisplay)?
+            .parse()
+            .map_err(|_| ParseModelineErrorKind::InvalidHdisplay)?;
+        let hsync_start = args
+            .next()
+            .ok_or(ParseModelineErrorKind::NoHsyncStart)?
+            .parse()
+            .map_err(|_| ParseModelineErrorKind::InvalidHsyncStart)?;
+        let hsync_end = args
+            .next()
+            .ok_or(ParseModelineErrorKind::NoHsyncEnd)?
+            .parse()
+            .map_err(|_| ParseModelineErrorKind::InvalidHsyncEnd)?;
+        let htotal = args
+            .next()
+            .ok_or(ParseModelineErrorKind::NoHtotal)?
+            .parse()
+            .map_err(|_| ParseModelineErrorKind::InvalidHtotal)?;
+        let vdisplay = args
+            .next()
+            .ok_or(ParseModelineErrorKind::NoVdisplay)?
+            .parse()
+            .map_err(|_| ParseModelineErrorKind::InvalidVdisplay)?;
+        let vsync_start = args
+            .next()
+            .ok_or(ParseModelineErrorKind::NoVsyncStart)?
+            .parse()
+            .map_err(|_| ParseModelineErrorKind::InvalidVsyncStart)?;
+        let vsync_end = args
+            .next()
+            .ok_or(ParseModelineErrorKind::NoVsyncEnd)?
+            .parse()
+            .map_err(|_| ParseModelineErrorKind::InvalidVsyncEnd)?;
+        let vtotal = args
+            .next()
+            .ok_or(ParseModelineErrorKind::NoVtotal)?
+            .parse()
+            .map_err(|_| ParseModelineErrorKind::InvalidVtotal)?;
+
+        let hsync = match args
+            .next()
+            .ok_or(ParseModelineErrorKind::NoHsync)?
+            .to_lowercase()
+            .as_str()
+        {
+            "+hsync" => true,
+            "-hsync" => false,
+            _ => Err(ParseModelineErrorKind::InvalidHsync)?,
+        };
+        let vsync = match args
+            .next()
+            .ok_or(ParseModelineErrorKind::NoVsync)?
+            .to_lowercase()
+            .as_str()
+        {
+            "+vsync" => true,
+            "-vsync" => false,
+            _ => Err(ParseModelineErrorKind::InvalidVsync)?,
+        };
+
+        Ok(Modeline {
+            clock,
+            hdisplay,
+            hsync_start,
+            hsync_end,
+            htotal,
+            vdisplay,
+            vsync_start,
+            vsync_end,
+            vtotal,
+            hsync,
+            vsync,
+        })
+    }
+}

src/api.rs

@@ -16,7 +16,8 @@ use pinnacle_api_defs::pinnacle::{
         self,
         v0alpha1::{
             output_service_server, set_scale_request::AbsoluteOrRelative, SetLocationRequest,
-            SetModeRequest, SetPoweredRequest, SetScaleRequest, SetTransformRequest,
+            SetModeRequest, SetModelineRequest, SetPoweredRequest, SetScaleRequest,
+            SetTransformRequest,
         },
     },
     process::v0alpha1::{process_service_server, SetEnvRequest, SpawnRequest, SpawnResponse},
@@ -52,10 +53,10 @@ use tonic::{Request, Response, Status, Streaming};
 use tracing::{debug, error, info, trace, warn};
 
 use crate::{
-    backend::BackendData,
+    backend::{udev::drm_mode_from_api_modeline, BackendData},
     config::ConnectorSavedState,
     input::ModifierMask,
-    output::OutputName,
+    output::{OutputMode, OutputName},
     render::util::snapshot::capture_snapshots_on_output,
     state::{State, WithState},
     tag::{Tag, TagId},
@@ -1080,7 +1081,45 @@ impl output_service_server::OutputService for OutputService {
             state.pinnacle.change_output_state(
                 &mut state.backend,
                 &output,
-                Some(mode),
+                Some(OutputMode::Smithay(mode)),
+                None,
+                None,
+                None,
+            );
+            state.pinnacle.request_layout(&output);
+            state
+                .pinnacle
+                .output_management_manager_state
+                .update::<State>();
+        })
+        .await
+    }
+
+    async fn set_modeline(
+        &self,
+        request: Request<SetModelineRequest>,
+    ) -> Result<Response<()>, Status> {
+        let request = request.into_inner();
+
+        run_unary_no_response(&self.sender, |state| {
+            let Some(output) = request
+                .output_name
+                .clone()
+                .map(OutputName)
+                .and_then(|name| name.output(&state.pinnacle))
+            else {
+                return;
+            };
+
+            let Some(mode) = drm_mode_from_api_modeline(request) else {
+                // TODO: raise error
+                return;
+            };
+
+            state.pinnacle.change_output_state(
+                &mut state.backend,
+                &output,
+                Some(OutputMode::Drm(mode)),
                 None,
                 None,
                 None,

src/backend.rs

@@ -34,6 +34,7 @@ use smithay::{
 use tracing::error;
 
 use crate::{
+    output::OutputMode,
     state::{Pinnacle, State, SurfaceDmabufFeedback, WithState},
     window::WindowElement,
 };
@@ -152,7 +153,7 @@ pub trait BackendData: 'static {
     // INFO: only for udev in anvil, maybe shouldn't be a trait fn?
     fn early_import(&mut self, surface: &WlSurface);
 
-    fn set_output_mode(&mut self, output: &Output, mode: smithay::output::Mode);
+    fn set_output_mode(&mut self, output: &Output, mode: OutputMode);
 }
 
 impl BackendData for Backend {
@@ -183,7 +184,7 @@ impl BackendData for Backend {
         }
     }
 
-    fn set_output_mode(&mut self, output: &Output, mode: smithay::output::Mode) {
+    fn set_output_mode(&mut self, output: &Output, mode: OutputMode) {
         match self {
             Backend::Winit(winit) => winit.set_output_mode(output, mode),
             Backend::Udev(udev) => udev.set_output_mode(output, mode),

src/backend/dummy.rs

@@ -10,6 +10,7 @@ use smithay::{
     utils::Transform,
 };
 
+use crate::output::OutputMode;
 use crate::state::{Pinnacle, State, WithState};
 
 use super::BackendData;
@@ -50,8 +51,8 @@ impl BackendData for Dummy {
 
     fn early_import(&mut self, _surface: &WlSurface) {}
 
-    fn set_output_mode(&mut self, output: &Output, mode: smithay::output::Mode) {
+    fn set_output_mode(&mut self, output: &Output, mode: OutputMode) {
-        output.change_current_state(Some(mode), None, None, None);
+        output.change_current_state(Some(mode.into()), None, None, None);
     }
 }
 

src/backend/udev.rs

@@ -3,6 +3,8 @@
 mod drm;
 mod gamma;
 
+pub use drm::util::drm_mode_from_api_modeline;
+
 use std::{
     collections::{HashMap, HashSet},
     path::Path,
@@ -82,7 +84,7 @@ use tracing::{debug, error, info, trace, warn};
 use crate::{
     backend::Backend,
     config::ConnectorSavedState,
-    output::{BlankingState, OutputName},
+    output::{BlankingState, OutputMode, OutputName},
     render::{
         pointer::PointerElement, pointer_render_elements, take_presentation_feedback,
         OutputRenderElement, CLEAR_COLOR, CLEAR_COLOR_LOCKED,
@@ -664,7 +666,7 @@ impl BackendData for Udev {
         }
     }
 
-    fn set_output_mode(&mut self, output: &Output, mode: smithay::output::Mode) {
+    fn set_output_mode(&mut self, output: &Output, mode: OutputMode) {
         let drm_mode = self
             .backends
             .iter()
@@ -681,18 +683,24 @@ impl BackendData for Udev {
                     .and_then(|(info, _)| {
                         info.modes()
                             .iter()
-                            .find(|m| smithay::output::Mode::from(**m) == mode)
+                            .find(|m| smithay::output::Mode::from(**m) == mode.into())
                     })
                     .copied()
             })
             .unwrap_or_else(|| {
                 info!("Unknown mode for {}, creating new one", output.name());
+                match mode {
+                    OutputMode::Smithay(mode) => {
                         create_drm_mode(mode.size.w, mode.size.h, Some(mode.refresh as u32))
+                    }
+                    OutputMode::Drm(mode) => mode,
+                }
             });
 
         if let Some(render_surface) = render_surface_for_output(output, &mut self.backends) {
             match render_surface.compositor.use_mode(drm_mode) {
                 Ok(()) => {
+                    let mode = smithay::output::Mode::from(mode);
                     info!(
                         "Set {}'s mode to {}x{}@{:.3}Hz",
                         output.name(),
@@ -1141,7 +1149,14 @@ impl Udev {
 
         device.surfaces.insert(crtc, surface);
 
-        pinnacle.change_output_state(self, &output, Some(wl_mode), None, None, Some(position));
+        pinnacle.change_output_state(
+            self,
+            &output,
+            Some(OutputMode::Smithay(wl_mode)),
+            None,
+            None,
+            Some(position),
+        );
 
         // If there is saved connector state, the connector was previously plugged in.
         // In this case, restore its tags and location.

src/backend/udev/drm/util.rs

@@ -1,6 +1,6 @@
 use std::{ffi::CString, io::Write, mem::MaybeUninit, num::NonZeroU32};
 
-use anyhow::{bail, Context};
+use anyhow::Context;
 use drm_sys::{
     drm_mode_modeinfo, DRM_MODE_FLAG_NHSYNC, DRM_MODE_FLAG_NVSYNC, DRM_MODE_FLAG_PHSYNC,
     DRM_MODE_FLAG_PVSYNC, DRM_MODE_TYPE_USERDEF,
@@ -9,6 +9,7 @@ use libdisplay_info_sys::cvt::{
     di_cvt_compute, di_cvt_options, di_cvt_reduced_blanking_version_DI_CVT_REDUCED_BLANKING_NONE,
     di_cvt_timing,
 };
+use pinnacle_api_defs::pinnacle::output::v0alpha1::SetModelineRequest;
 use smithay::reexports::drm::{
     self,
     control::{connector, property, Device, ResourceHandle},
@@ -141,94 +142,72 @@ pub(super) fn get_drm_property(
     anyhow::bail!("No prop found for {}", name)
 }
 
-// From https://github.com/swaywm/sway/blob/2e9139df664f1e2dbe14b5df4a9646411b924c66/sway/commands/output/mode.c#L64
+pub fn drm_mode_from_api_modeline(modeline: SetModelineRequest) -> Option<drm::control::Mode> {
-fn parse_modeline_string(modeline: &str) -> anyhow::Result<drm_mode_modeinfo> {
+    let SetModelineRequest {
-    let mut args = modeline.split_whitespace();
+        output_name: _,
+        clock: Some(clock),
+        hdisplay: Some(hdisplay),
+        hsync_start: Some(hsync_start),
+        hsync_end: Some(hsync_end),
+        htotal: Some(htotal),
+        vdisplay: Some(vdisplay),
+        vsync_start: Some(vsync_start),
+        vsync_end: Some(vsync_end),
+        vtotal: Some(vtotal),
+        hsync_pos: Some(hsync_pos),
+        vsync_pos: Some(vsync_pos),
+    } = modeline
+    else {
+        return None;
+    };
 
-    let clock = args
+    let clock = clock * 1000.0;
-        .next()
+
-        .context("no clock specified")?
+    let vrefresh = (clock * 1000.0 * 1000.0 / htotal as f32 / vtotal as f32) as u32;
-        .parse::<u32>()
-        .context("failed to parse clock")?
-        * 1000;
-    let hdisplay = args
-        .next()
-        .context("no hdisplay specified")?
-        .parse()
-        .context("failed to parse hdisplay")?;
-    let hsync_start = args
-        .next()
-        .context("no hsync_start specified")?
-        .parse()
-        .context("failed to parse hsync_start")?;
-    let hsync_end = args
-        .next()
-        .context("no hsync_end specified")?
-        .parse()
-        .context("failed to parse hsync_end")?;
-    let htotal = args
-        .next()
-        .context("no htotal specified")?
-        .parse()
-        .context("failed to parse htotal")?;
-    let vdisplay = args
-        .next()
-        .context("no vdisplay specified")?
-        .parse()
-        .context("failed to parse vdisplay")?;
-    let vsync_start = args
-        .next()
-        .context("no vsync_start specified")?
-        .parse()
-        .context("failed to parse vsync_start")?;
-    let vsync_end = args
-        .next()
-        .context("no vsync_end specified")?
-        .parse()
-        .context("failed to parse vsync_end")?;
-    let vtotal = args
-        .next()
-        .context("no vtotal specified")?
-        .parse()
-        .context("failed to parse vtotal")?;
-    let vrefresh = clock * 1000 * 1000 / htotal as u32 / vtotal as u32;
 
     let mut flags = 0;
-    match args.next().context("no +/-hsync specified")? {
+    match hsync_pos {
-        "+hsync" => flags |= DRM_MODE_FLAG_PHSYNC,
+        true => flags |= DRM_MODE_FLAG_PHSYNC,
-        "-hsync" => flags |= DRM_MODE_FLAG_NHSYNC,
+        false => flags |= DRM_MODE_FLAG_NHSYNC,
-        _ => bail!("invalid hsync specifier"),
     };
-    match args.next().context("no +/-vsync specified")? {
+    match vsync_pos {
-        "+vsync" => flags |= DRM_MODE_FLAG_PVSYNC,
+        true => flags |= DRM_MODE_FLAG_PVSYNC,
-        "-vsync" => flags |= DRM_MODE_FLAG_NVSYNC,
+        false => flags |= DRM_MODE_FLAG_NVSYNC,
-        _ => bail!("invalid vsync specifier"),
     };
 
     let type_ = DRM_MODE_TYPE_USERDEF;
 
-    let name = CString::new(format!("{}x{}@{}", hdisplay, vdisplay, vrefresh / 1000)).unwrap();
+    let name = CString::new(format!(
+        "{}x{}@{:.3}",
+        hdisplay,
+        vdisplay,
+        vrefresh as f64 / 1000.0
+    ))
+    .unwrap();
     let mut name_buf = [0u8; 32];
     let _ = name_buf.as_mut_slice().write_all(name.as_bytes_with_nul());
     let name: [i8; 32] = bytemuck::cast(name_buf);
 
-    Ok(drm_mode_modeinfo {
+    Some(
-        clock,
+        drm_mode_modeinfo {
-        hdisplay,
+            clock: clock as u32,
-        hsync_start,
+            hdisplay: hdisplay as u16,
-        hsync_end,
+            hsync_start: hsync_start as u16,
-        htotal,
+            hsync_end: hsync_end as u16,
+            htotal: htotal as u16,
             hskew: 0,
-        vdisplay,
+            vdisplay: vdisplay as u16,
-        vsync_start,
+            vsync_start: vsync_start as u16,
-        vsync_end,
+            vsync_end: vsync_end as u16,
-        vtotal,
+            vtotal: vtotal as u16,
             vscan: 0,
             vrefresh,
             flags,
             type_,
             name,
-    })
+        }
+        .into(),
+    )
 }
 
 /// Create a new drm mode from a given width, height, and optional refresh rate (defaults to 60Hz).

src/backend/winit.rs

@@ -36,7 +36,7 @@ use smithay::{
 use tracing::{debug, error, trace, warn};
 
 use crate::{
-    output::BlankingState,
+    output::{BlankingState, OutputMode},
     render::{
         pointer::PointerElement, pointer_render_elements, take_presentation_feedback, CLEAR_COLOR,
         CLEAR_COLOR_LOCKED,
@@ -68,8 +68,8 @@ impl BackendData for Winit {
 
     fn early_import(&mut self, _surface: &WlSurface) {}
 
-    fn set_output_mode(&mut self, output: &Output, mode: smithay::output::Mode) {
+    fn set_output_mode(&mut self, output: &Output, mode: OutputMode) {
-        output.change_current_state(Some(mode), None, None, None);
+        output.change_current_state(Some(mode.into()), None, None, None);
     }
 }
 
@@ -207,7 +207,7 @@ impl Winit {
                             state.pinnacle.change_output_state(
                                 &mut state.backend,
                                 &output,
-                                Some(mode),
+                                Some(OutputMode::Smithay(mode)),
                                 None,
                                 Some(Scale::Fractional(scale_factor)),
                                 // None,

src/handlers.rs

@@ -77,6 +77,7 @@ use crate::{
     delegate_output_power_management, delegate_screencopy,
     focus::{keyboard::KeyboardFocusTarget, pointer::PointerFocusTarget},
     handlers::xdg_shell::snapshot_pre_commit_hook,
+    output::OutputMode,
     protocol::{
         foreign_toplevel::{self, ForeignToplevelHandler, ForeignToplevelManagerState},
         gamma_control::{GammaControlHandler, GammaControlManagerState},
@@ -979,7 +980,7 @@ impl OutputManagementHandler for State {
                     self.pinnacle.change_output_state(
                         &mut self.backend,
                         &output,
-                        mode,
+                        mode.map(OutputMode::Smithay),
                         transform,
                         scale.map(Scale::Fractional),
                         position,

src/output.rs

@@ -8,7 +8,7 @@ use pinnacle_api_defs::pinnacle::signal::v0alpha1::{
 use smithay::{
     desktop::layer_map_for_output,
     output::{Mode, Output, Scale},
-    reexports::calloop::LoopHandle,
+    reexports::{calloop::LoopHandle, drm},
     utils::{Logical, Point, Transform},
     wayland::session_lock::LockSurface,
 };
@@ -122,12 +122,27 @@ impl OutputState {
     }
 }
 
+#[derive(Debug, Clone, Copy)]
+pub enum OutputMode {
+    Smithay(Mode),
+    Drm(drm::control::Mode),
+}
+
+impl From<OutputMode> for Mode {
+    fn from(value: OutputMode) -> Self {
+        match value {
+            OutputMode::Smithay(mode) => mode,
+            OutputMode::Drm(mode) => Mode::from(mode),
+        }
+    }
+}
+
 impl Pinnacle {
     pub fn change_output_state(
         &mut self,
         backend: &mut impl BackendData,
         output: &Output,
-        mode: Option<Mode>,
+        mode: Option<OutputMode>,
         transform: Option<Transform>,
         scale: Option<Scale>,
         location: Option<Point<i32, Logical>>,