`9eca12e`

fix fullscreen rendering and dialog stacking

- Implement chunked put_image for images exceeding X11 max request size
- Remove initial render before event loop to avoid stale dimensions
- Add surface/window size verification before blitting
- Pass parent_window to picker for transient-for hint

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

SHA: 9eca12e8af0b9a8c60cd0a4bdbc671297e41aa0e
Parents: 7762f04
Tree: 69c65dd

3 changed files

Status	File	+	-
M	`garfield-portal/src/file_chooser.rs`	34	7
M	`garfield/src/app.rs`	127	24
M	`garfield/src/main.rs`	7	0

garfield-portal/src/file_chooser.rsmodified

          }
      }
++    /// Parse X11 window ID from portal parent_window string.
++    /// Format is "x11:<xid>" where xid is the window ID in hex.
++    fn parse_parent_window(parent_window: &str) -> Option<u32> {
++        if parent_window.starts_with("x11:") {
++            let hex_str = &parent_window[4..];
++            u32::from_str_radix(hex_str, 16).ok()
++        } else {
++            None
++        }
++    }
++
      /// Spawn garfield in picker mode and collect results.
      async fn spawn_picker(
          &self,
          multiple: bool,
          filters: Vec<String>,
          current_folder: Option<String>,
++        parent_window: Option<u32>,
      ) -> (u32, HashMap<String, Value<'static>>) {
          // Find garfield binary - prefer ~/.cargo/bin (development), then /usr/local/bin, then PATH
          let home = std::env::var("HOME").unwrap_or_default();
              cmd.arg("--filter").arg(filters.join(";"));
          }
++        if let Some(parent) = parent_window {
++            cmd.arg("--parent-window").arg(parent.to_string());
++        }
++
          if let Some(folder) = current_folder {
              cmd.arg(&folder);
          }
          title: &str,
          suggested_filename: Option<String>,
          current_folder: Option<String>,
++        parent_window: Option<u32>,
      ) -> (u32, HashMap<String, Value<'static>>) {
          // Find garfield binary - prefer ~/.cargo/bin (development), then /usr/local/bin, then PATH
          let home = std::env::var("HOME").unwrap_or_default();
              cmd.arg("--title").arg(title);
          }
++        if let Some(parent) = parent_window {
++            cmd.arg("--parent-window").arg(parent.to_string());
++        }
++
          if let Some(folder) = current_folder {
              cmd.arg(&folder);
          }
          #[zbus(object_server)] server: &zbus::ObjectServer,
          handle: ObjectPath<'_>,
          _app_id: &str,
--        _parent_window: &str,
++        parent_window: &str,
          title: &str,
          options: HashMap<&str, Value<'_>>,
      ) -> fdo::Result<(u32, HashMap<String, Value<'static>>)> {
--        tracing::info!("OpenFile request: handle={}, title={}", handle, title);
++        tracing::info!("OpenFile request: handle={}, title={}, parent_window={}", handle, title, parent_window);
          let handle_owned: OwnedObjectPath = handle.into();
++        let parent_window_id = Self::parse_parent_window(parent_window);
          // Parse options
          let multiple = options.get("multiple")
              multiple,
              filters,
              current_folder,
++            parent_window_id,
          ).await;
          tracing::debug!("Picker returned: {:?}", result.0);
          #[zbus(object_server)] server: &zbus::ObjectServer,
          handle: ObjectPath<'_>,
          _app_id: &str,
--        _parent_window: &str,
++        parent_window: &str,
          title: &str,
          options: HashMap<&str, Value<'_>>,
      ) -> fdo::Result<(u32, HashMap<String, Value<'static>>)> {
--        tracing::info!("SaveFile request: handle={}, title={}", handle, title);
++        tracing::info!("SaveFile request: handle={}, title={}, parent_window={}", handle, title, parent_window);
          tracing::debug!("SaveFile options: {:?}", options);
          let handle_owned: OwnedObjectPath = handle.into();
++        let parent_window_id = Self::parse_parent_window(parent_window);
          let current_folder = Self::parse_current_folder(&options);
          let suggested_filename = Self::parse_current_name(&options);
--        tracing::info!("SaveFile: folder={:?}, filename={:?}", current_folder, suggested_filename);
++        tracing::info!("SaveFile: folder={:?}, filename={:?}, parent={:?}", current_folder, suggested_filename, parent_window_id);
          let request = Request::new(handle_owned.clone(), self.request_manager.clone());
          server.at(handle_owned.as_ref(), request).await
              title,
              suggested_filename,
              current_folder,
++            parent_window_id,
          ).await;
          let _ = server.remove::<Request, _>(&handle_owned).await;
          #[zbus(object_server)] server: &zbus::ObjectServer,
          handle: ObjectPath<'_>,
          _app_id: &str,
--        _parent_window: &str,
++        parent_window: &str,
          title: &str,
          options: HashMap<&str, Value<'_>>,
      ) -> fdo::Result<(u32, HashMap<String, Value<'static>>)> {
--        tracing::info!("SaveFiles request: handle={}, title={}", handle, title);
++        tracing::info!("SaveFiles request: handle={}, title={}, parent_window={}", handle, title, parent_window);
          // SaveFiles picks a directory for saving multiple files
          let handle_owned: OwnedObjectPath = handle.into();
++        let parent_window_id = Self::parse_parent_window(parent_window);
          let current_folder = Self::parse_current_folder(&options);
          let request = Request::new(handle_owned.clone(), self.request_manager.clone());
              false,
              Vec::new(),
              current_folder,
++            parent_window_id,
          ).await;
          let _ = server.remove::<Request, _>(&handle_owned).await;

garfield/src/app.rsmodified

          let x = monitor.rect.x + (monitor.rect.width as i32 - width as i32) / 2;
          let y = monitor.rect.y + (monitor.rect.height as i32 - height as i32) / 2;
++        tracing::debug!(
++            "Window size: monitor={}x{}, requested={}x{} at ({}, {})",
++            monitor.rect.width, monitor.rect.height, width, height, x, y
++        );
++
          // Window title depends on mode
          let title = if picker_config.is_picker() {
              picker_config.title.clone().unwrap_or_else(|| {
          // Use Dialog window type for picker mode (better focus handling)
          if picker_config.is_picker() {
              window_config = window_config.window_type(gartk_x11::WindowType::Dialog);
++
++            // Set transient-for if parent window specified (dialog belongs to parent)
++            if let Some(parent) = picker_config.parent_window {
++                window_config = window_config.parent_window(parent).modal(true);
++            }
+         }
          let window = Window::create(conn.clone(), window_config)?;
      pub fn run(&mut self) -> Result<()> {
          let mut event_loop = EventLoop::new(&self.window, EventLoopConfig::default())?;
--        // Initial render
++        // Don't render before event loop - wait for ConfigureNotify/Expose
--        self.render()?;
++        // to get correct window dimensions from WM before first render
          event_loop.run(|ev, event| {
              match event {
                      ev.request_redraw();
+                 }
                  InputEvent::Resize { width, height } => {
--                    let _ = self.renderer.resize(width, height);
++                    tracing::debug!("ConfigureNotify resize: {}x{}", width, height);
++                    if let Err(e) = self.renderer.resize(width, height) {
++                        tracing::error!("Failed to resize renderer: {}", e);
++                    }
++                    // Update internal rect tracking (don't send X11 request - WM already sized us)
++                    self.window.set_size(width, height);
                      self.update_layout(width, height);
                      ev.request_redraw();
+                 }
+                 }
                  PickerToolbarClick::None => {}
+             }
--        } else {
++        }
--            // Check normal toolbar clicks
++
--            if let Some(action) = self.toolbar.on_click(pos) {
++        // Check normal toolbar clicks (always, not just when picker_toolbar is None)
--                self.handle_toolbar_action(action);
++        if let Some(action) = self.toolbar.on_click(pos) {
--                return;
++            self.handle_toolbar_action(action);
--            }
++            return;
+         }
          // Check pane toolbar clicks (view mode buttons)
          let sidebar_w = self.sidebar.width();
          let header_height = TAB_BAR_HEIGHT + TOOLBAR_HEIGHT + BREADCRUMB_HEIGHT;
++        tracing::trace!("render: size={}x{}, sidebar_w={}, root_pane_bounds={:?}",
++            size.width, size.height, sidebar_w, self.root_pane.bounds());
++
          // Clear background
          self.renderer.clear()?;
      /// Blit the rendered surface to the window.
      fn blit_surface(&mut self) -> Result<()> {
          let size = self.renderer.size();
++        let window_size = self.window.size();
++        let stride = self.renderer.surface().stride() as usize;
++        let row_bytes = size.width as usize * 4; // 4 bytes per pixel (ARGB)
++
++        // Verify surface matches window size
++        if size.width != window_size.width || size.height != window_size.height {
++            tracing::warn!("Surface size {}x{} doesn't match window size {}x{}, skipping blit",
++                size.width, size.height, window_size.width, window_size.height);
++            return Ok(());
++        }
++
          let window_id = self.window.id();
          let depth = self.window.depth();
          let gc = self.gc;
          let conn = self.window.connection().clone();
--        // Access surface data directly without copying, blit to X11
++        // Get maximum request size (leave some headroom for request headers)
--        self.renderer.surface_mut().with_data(|data| {
++        let max_request_bytes = conn.maximum_request_bytes().saturating_sub(1024);
--            let _ = conn.inner().put_image(
++        let total_bytes = row_bytes * size.height as usize;
--                ImageFormat::Z_PIXMAP,
++
--                window_id,
++        // Check if we need to chunk the image
--                gc,
++        if total_bytes <= max_request_bytes {
--                size.width as u16,
++            // Image fits in a single request
--                size.height as u16,
++            tracing::trace!("blit_surface: {}x{} ({} bytes) in single request",
--                0,
++                size.width, size.height, total_bytes);
--                0,
++
--                0,
++            self.renderer.surface_mut().with_data(|data| {
--                depth,
++                if stride == row_bytes {
--                data,
++                    let _ = conn.inner().put_image(
--            );
++                        ImageFormat::Z_PIXMAP,
--        })?;
++                        window_id,
++                        gc,
++                        size.width as u16,
++                        size.height as u16,
++                        0,
++                        0,
++                        0,
++                        depth,
++                        data,
++                    );
++                } else {
++                    let mut packed = Vec::with_capacity(total_bytes);
++                    for y in 0..size.height as usize {
++                        let row_start = y * stride;
++                        let row_end = row_start + row_bytes;
++                        if row_end <= data.len() {
++                            packed.extend_from_slice(&data[row_start..row_end]);
++                        }
++                    }
++                    let _ = conn.inner().put_image(
++                        ImageFormat::Z_PIXMAP,
++                        window_id,
++                        gc,
++                        size.width as u16,
++                        size.height as u16,
++                        0,
++                        0,
++                        0,
++                        depth,
++                        &packed,
++                    );
++                }
++            })?;
++        } else {
++            // Image too large - split into horizontal bands
++            let rows_per_chunk = max_request_bytes / row_bytes;
++            let rows_per_chunk = rows_per_chunk.max(1); // At least 1 row per chunk
++
++            tracing::debug!("blit_surface: {}x{} ({} bytes) exceeds max {} bytes, using {} rows per chunk",
++                size.width, size.height, total_bytes, max_request_bytes, rows_per_chunk);
++
++            self.renderer.surface_mut().with_data(|data| {
++                let mut y_offset = 0u32;
++                while y_offset < size.height {
++                    let chunk_height = (size.height - y_offset).min(rows_per_chunk as u32);
++                    let chunk_bytes = row_bytes * chunk_height as usize;
++
++                    // Extract this chunk's data
++                    let mut chunk_data = Vec::with_capacity(chunk_bytes);
++                    for row in 0..chunk_height as usize {
++                        let src_y = y_offset as usize + row;
++                        let row_start = src_y * stride;
++                        let row_end = row_start + row_bytes;
++                        if row_end <= data.len() {
++                            chunk_data.extend_from_slice(&data[row_start..row_end]);
++                        }
++                    }
++
++                    let _ = conn.inner().put_image(
++                        ImageFormat::Z_PIXMAP,
++                        window_id,
++                        gc,
++                        size.width as u16,
++                        chunk_height as u16,
++                        0,
++                        y_offset as i16,
++                        0,
++                        depth,
++                        &chunk_data,
++                    );
++
++                    y_offset += chunk_height;
++                }
++            })?;
++        }
          self.window.connection().flush()?;

garfield/src/main.rsmodified

      /// Suggested filename for save mode
      #[arg(long, requires = "save")]
      pub save_filename: Option<String>,
++
++    /// Parent window ID for transient-for hint (X11 window ID)
++    #[arg(long, requires = "picker")]
++    pub parent_window: Option<u32>,
  }
  /// Picker mode configuration parsed from CLI args.
      pub title: Option<String>,
      /// Accept button label.
      pub accept_label: String,
++    /// Parent window ID for transient-for hint.
++    pub parent_window: Option<u32>,
  }
  impl PickerConfig {
              mode,
              title: args.title.clone(),
              accept_label: args.accept_label.clone().unwrap_or_else(|| default_label.to_string()),
++            parent_window: args.parent_window,
          }
      }