`8545cd9`

fix rotation by ensuring screen size before applying RandR configs

- Add ensure_screen_size() to resize virtual screen before CRTC changes
- Calculate effective dimensions after rotation (90/270 swap width/height)
- Prepare screen for all monitors before applying any configurations
- Shrink screen to fit after applying configurations
- Add prepare_screen_for_configs() for revert operation as well
- Add refresh_randr_outputs() to dynamically update mode information
- Add debug logging for timeout and RandR operations

Authored by mfwolffe <wolffemf@dukes.jmu.edu> 3 months ago

SHA: 8545cd922a7ee37535f4a75a2c9a78214885a709
Parents: 139af1d
Tree: 0497d5f

3 changed files

Status	File	+	-
M	`gardisplay/src/app.rs`	56	0
M	`gardisplay/src/randr/manager.rs`	199	2
M	`gardisplay/src/ui/confirm_overlay.rs`	22	8

gardisplay/src/app.rsmodified

+         }
+     }
 +    /// Refresh RandR outputs from the display server.
 +    fn refresh_randr_outputs(&mut self) {
 +        if self.demo_mode {
 +            return; // Demo mode uses static outputs
 +        }
++
 +        if let Some(ref randr) = self.randr {
 +            match randr.get_outputs() {
 +                Ok(outputs) => {
 +                    tracing::debug!("refresh_randr_outputs: found {} outputs", outputs.len());
 +                    for o in &outputs {
 +                        tracing::debug!(
 +                            "  {} connected={} modes={} current={:?}",
 +                            o.name,
 +                            o.connected,
 +                            o.modes.len(),
 +                            o.current_mode.as_ref().map(|m| format!("{}x{}@{:.0}Hz", m.width, m.height, m.refresh))
 +                        );
 +                    }
 +                    self.randr_outputs = outputs;
 +                }
 +                Err(e) => {
 +                    tracing::warn!("failed to refresh RandR outputs: {}", e);
 +                }
 +            }
 +        }
 +    }
++
      /// Sync the display panel with the selected monitor.
      fn sync_panel_selection(&mut self) {
 +        // Refresh outputs to get current mode information
 +        self.refresh_randr_outputs();
++
          if let Some(state) = self.monitor_view.selected_monitor() {
              // Find matching RandR output (may be None in demo mode)
              let output = self.randr_outputs.iter().find(|o| o.name == state.info.name);
              })
              .collect();
 +        // IMPORTANT: Prepare the screen size BEFORE applying any configurations
 +        // This is essential for rotation changes which may require a larger virtual screen
 +        if let Err(e) = randr.prepare_screen_for_configs(&configs) {
 +            tracing::error!("failed to prepare screen size: {}", e);
 +            self.set_status(&format!("Failed to prepare screen: {}", e));
 +            return;
 +        }
++
          let mut success_count = 0;
          let mut error_count = 0;
              tracing::error!("failed to flush: {}", e);
+         }
 +        // Try to shrink screen to fit (non-fatal if it fails)
 +        if let Err(e) = randr.shrink_screen_to_fit() {
 +            tracing::debug!("shrink_screen_to_fit failed (non-fatal): {}", e);
 +        }
++
          if error_count > 0 {
              self.set_status(&format!(
                  "Applied {} monitor(s), {} error(s)",
          if let Some(ref configs) = self.pre_change_config.take() {
              if let Some(ref randr) = self.randr {
 +                // IMPORTANT: Prepare screen size BEFORE reverting
 +                // The pre-change config may have different dimensions than current
 +                if let Err(e) = randr.prepare_screen_for_configs(configs) {
 +                    tracing::error!("failed to prepare screen for revert: {}", e);
 +                    // Continue anyway - we still want to try to revert
 +                }
++
                  for config in configs {
                      if let Err(e) = randr.apply_monitor(config) {
                          tracing::error!("failed to revert {}: {}", config.name, e);
+                     }
+                 }
                  let _ = randr.flush();
++
 +                // Try to shrink screen to fit
 +                if let Err(e) = randr.shrink_screen_to_fit() {
 +                    tracing::debug!("shrink_screen_to_fit failed during revert: {}", e);
 +                }
+             }
+         }

gardisplay/src/randr/manager.rsmodified

      #[allow(dead_code)] // Used for mode selection UI
      pub fn get_outputs(&self) -> Result<Vec<OutputInfo>> {
          let resources = self.get_resources()?;
 +        tracing::debug!(
 +            "get_outputs: resources has {} modes available, {} outputs",
 +            resources.modes.len(),
 +            resources.outputs.len()
 +        );
++
          let mut outputs = Vec::new();
          for &output in &resources.outputs {
              let name = String::from_utf8_lossy(&info.name).to_string();
              let connected = info.connection == randr::Connection::CONNECTED;
 +            tracing::debug!(
 +                "  output '{}': {} mode IDs in info.modes",
 +                name,
 +                info.modes.len()
 +            );
++
              // Get available modes
              let modes: Vec<ModeInfo> = info
                  .modes
                  .iter()
 -                .filter_map(|&mode_id| self.get_mode_info(&resources, mode_id))
 +                .filter_map(|&mode_id| {
 +                    let result = self.get_mode_info(&resources, mode_id);
 +                    if result.is_none() {
 +                        tracing::debug!("    mode_id {} not found in resources.modes", mode_id);
 +                    }
 +                    result
 +                })
                  .collect();
 +            tracing::debug!("    resolved {} modes for '{}'", modes.len(), name);
++
              // Get current mode and position if CRTC is set
              let (crtc, current_mode, position) = if info.crtc != 0 {
                  let crtc_info = self
          Err(RandrError::OutputNotFound(name.to_string()))
+     }
 +    /// Ensure the virtual screen is large enough to contain the given bounds.
 +    /// This must be called before applying configurations that might exceed the current screen size.
 +    pub fn ensure_screen_size(&self, required_width: u32, required_height: u32) -> Result<()> {
 +        let _resources = self.get_resources()?;
++
 +        // Get current screen info
 +        let screen_info = self
 +            .conn
 +            .inner()
 +            .randr_get_screen_info(self.root)?
 +            .reply()?;
++
 +        let current_size = screen_info
 +            .sizes
 +            .get(screen_info.size_id as usize)
 +            .map(|s| (s.width as u32, s.height as u32))
 +            .unwrap_or((0, 0));
++
 +        tracing::debug!(
 +            "ensure_screen_size: current={}x{}, required={}x{}",
 +            current_size.0,
 +            current_size.1,
 +            required_width,
 +            required_height
 +        );
++
 +        // Only resize if needed
 +        if current_size.0 >= required_width && current_size.1 >= required_height {
 +            return Ok(());
 +        }
++
 +        let new_width = required_width.max(current_size.0);
 +        let new_height = required_height.max(current_size.1);
++
 +        // Calculate physical size in mm (approximate based on 96 DPI)
 +        let mm_width = (new_width as f64 * 25.4 / 96.0) as u32;
 +        let mm_height = (new_height as f64 * 25.4 / 96.0) as u32;
++
 +        tracing::info!(
 +            "resizing virtual screen to {}x{} ({}x{}mm)",
 +            new_width,
 +            new_height,
 +            mm_width,
 +            mm_height
 +        );
++
 +        self.conn.inner().randr_set_screen_size(
 +            self.root,
 +            new_width as u16,
 +            new_height as u16,
 +            mm_width,
 +            mm_height,
 +        )?;
++
 +        self.conn.inner().flush()?;
 +        Ok(())
 +    }
++
 +    /// Calculate the effective dimensions after rotation.
 +    fn rotated_dimensions(width: u32, height: u32, rotation: u32) -> (u32, u32) {
 +        match rotation {
 +            90 | 270 => (height, width), // Swap dimensions for 90/270 rotation
 +            _ => (width, height),        // 0 or 180 keeps same dimensions
 +        }
 +    }
++
 +    /// Calculate the required screen size to contain all given monitor configurations.
 +    pub fn calculate_required_screen_size(configs: &[MonitorConfig]) -> (u32, u32) {
 +        let mut max_x = 0u32;
 +        let mut max_y = 0u32;
++
 +        for config in configs {
 +            if !config.enabled {
 +                continue;
 +            }
++
 +            let (eff_width, eff_height) =
 +                Self::rotated_dimensions(config.width, config.height, config.rotation);
++
 +            let right = config.x.max(0) as u32 + eff_width;
 +            let bottom = config.y.max(0) as u32 + eff_height;
++
 +            max_x = max_x.max(right);
 +            max_y = max_y.max(bottom);
 +        }
++
 +        // Ensure minimum screen size
 +        (max_x.max(320), max_y.max(200))
 +    }
++
 +    /// Prepare the screen for a set of monitor configurations.
 +    /// This should be called before applying any configurations to ensure the screen is large enough.
 +    pub fn prepare_screen_for_configs(&self, configs: &[MonitorConfig]) -> Result<()> {
 +        let (required_width, required_height) = Self::calculate_required_screen_size(configs);
 +        tracing::info!(
 +            "preparing screen for {} monitors: required size {}x{}",
 +            configs.iter().filter(|c| c.enabled).count(),
 +            required_width,
 +            required_height
 +        );
 +        self.ensure_screen_size(required_width, required_height)
 +    }
++
 +    /// Shrink the screen to the minimum size required for the current outputs.
 +    /// Call this after applying all configurations to clean up excess virtual screen space.
 +    pub fn shrink_screen_to_fit(&self) -> Result<()> {
 +        let outputs = self.get_outputs()?;
++
 +        let mut max_x = 0u32;
 +        let mut max_y = 0u32;
++
 +        for output in &outputs {
 +            if !output.connected {
 +                continue;
 +            }
 +            if let (Some(mode), Some(pos)) = (&output.current_mode, output.position) {
 +                let right = pos.0.max(0) as u32 + mode.width as u32;
 +                let bottom = pos.1.max(0) as u32 + mode.height as u32;
 +                max_x = max_x.max(right);
 +                max_y = max_y.max(bottom);
 +            }
 +        }
++
 +        // Ensure minimum size
 +        let target_width = max_x.max(320);
 +        let target_height = max_y.max(200);
++
 +        let mm_width = (target_width as f64 * 25.4 / 96.0) as u32;
 +        let mm_height = (target_height as f64 * 25.4 / 96.0) as u32;
++
 +        tracing::info!(
 +            "shrinking virtual screen to {}x{} ({}x{}mm)",
 +            target_width,
 +            target_height,
 +            mm_width,
 +            mm_height
 +        );
++
 +        self.conn.inner().randr_set_screen_size(
 +            self.root,
 +            target_width as u16,
 +            target_height as u16,
 +            mm_width,
 +            mm_height,
 +        )?;
++
 +        self.conn.inner().flush()?;
 +        Ok(())
 +    }
++
      /// Apply a monitor configuration.
      pub fn apply_monitor(&self, config: &MonitorConfig) -> Result<()> {
          if !config.enabled {
          // Find available CRTC
          let crtc = self.find_available_crtc(&resources, &output_info, output)?;
 +        // Calculate effective dimensions after rotation
 +        let (eff_width, eff_height) =
 +            Self::rotated_dimensions(config.width, config.height, config.rotation);
++
 +        // Calculate required screen size to contain this monitor
 +        let required_width = config.x as u32 + eff_width;
 +        let required_height = config.y as u32 + eff_height;
++
 +        tracing::debug!(
 +            "apply_monitor {}: {}x{} rot={} -> effective {}x{} at ({}, {})",
 +            config.name,
 +            config.width,
 +            config.height,
 +            config.rotation,
 +            eff_width,
 +            eff_height,
 +            config.x,
 +            config.y
 +        );
++
 +        // Ensure screen is large enough BEFORE applying the CRTC config
 +        self.ensure_screen_size(required_width, required_height)?;
++
 +        // Re-fetch resources after screen resize (timestamps may have changed)
 +        let resources = self.get_resources()?;
++
          // Convert rotation
          let rotation = match config.rotation {
 => randr::Rotation::ROTATE90,
+         }
          tracing::info!(
 -            "applied config for {}: {}x{} at ({}, {})",
 +            "applied config for {}: {}x{} rot={} at ({}, {})",
              config.name,
              config.width,
              config.height,
 +            config.rotation,
              config.x,
              config.y
          );

gardisplay/src/ui/confirm_overlay.rsmodified

      /// Handle an input event.
      pub fn handle_event(&mut self, event: &InputEvent) -> ConfirmResult {
 -        // Check for timeout on any event
 +        // ALWAYS check for timeout first, on EVERY event
 +        // This is critical for auto-revert to work reliably
          if self.is_expired() {
 +            tracing::info!(
 +                "confirm overlay expired after {:?} (timeout was {:?})",
 +                self.start_time.elapsed(),
 +                self.timeout
 +            );
              return ConfirmResult::Reverted;
+         }
              InputEvent::Key(e) if e.pressed => {
                  match e.key {
                      // Enter confirms
 -                    Key::Return => ConfirmResult::Confirmed,
 +                    Key::Return => {
 +                        tracing::info!("user confirmed display changes via Enter key");
 +                        ConfirmResult::Confirmed
 +                    }
                      // Escape reverts
 -                    Key::Escape => ConfirmResult::Reverted,
 +                    Key::Escape => {
 +                        tracing::info!("user reverted display changes via Escape key");
 +                        ConfirmResult::Reverted
 +                    }
                      _ => ConfirmResult::None,
+                 }
+             }
+             }
              InputEvent::MousePress(e) if e.button == Some(MouseButton::Left) => {
                  if self.keep_btn.contains_point(e.position) {
 +                    tracing::info!("user confirmed display changes via Keep button");
                      ConfirmResult::Confirmed
                  } else if self.revert_btn.contains_point(e.position) {
 +                    tracing::info!("user reverted display changes via Revert button");
                      ConfirmResult::Reverted
                  } else {
                      ConfirmResult::None
+                 }
+             }
              InputEvent::Idle => {
 -                // Check timeout on idle
 -                if self.is_expired() {
 -                    ConfirmResult::Reverted
 -                } else {
 -                    ConfirmResult::Redraw // Update countdown display
 +                // Check timeout on idle (already checked above, but log for debugging)
 +                let remaining = self.remaining_secs();
 +                if remaining <= 3 {
 +                    tracing::debug!("confirm overlay: {}s remaining", remaining);
+                 }
 +                ConfirmResult::Redraw // Update countdown display
+             }
              _ => ConfirmResult::None,
+         }