Replaced overlay scrollbars with scrollarea calls.

Author: mkrueger

crates/icy_draw/src/ui/editor/ansi/dialog/tdf_font_selector.rs

@@ -18,7 +18,7 @@ use icy_engine_gui::{
         dialog_area, modal_container, primary_button, secondary_button, separator, ButtonType, Dialog, DialogAction, DIALOG_SPACING, DIALOG_WIDTH_XARGLE,
         TEXT_SIZE_NORMAL, TEXT_SIZE_SMALL,
     },
-    wrap_with_scrollbars, Viewport,
+    Viewport,
 };
 use icy_ui::{
     advanced::{
@@ -1046,9 +1046,8 @@ impl Dialog<Message> for TdfFontSelectorDialog {
         // Font list canvas (virtualized)
         let font_list: Element<'_, Message> = self.list_widget();
 
-        // Wrap canvas with scrollbar overlay
-        let needs_scrollbar = self.filtered_fonts.len() as f32 * FONT_ITEM_HEIGHT > LIST_HEIGHT;
-        let canvas_with_scrollbar = wrap_with_scrollbars(font_list, &self.viewport, needs_scrollbar, false);
+        // Font list canvas (scrollbars handled elsewhere if needed)
+        let canvas_with_scrollbar = font_list;
 
         let list_container = container(canvas_with_scrollbar)
             .width(Length::Fill)

crates/icy_draw/src/ui/editor/ansi/widget/canvas/mod.rs

@@ -12,13 +12,13 @@ use std::sync::Arc;
 
 use icy_engine::Screen;
 use icy_ui::{
-    widget::{container, stack},
-    Alignment, Element, Length, Task,
+    widget::{container, scroll_area, scrollable},
+    Element, Length, Task,
 };
 
 use icy_engine_gui::theme::main_area_background;
 use icy_engine_gui::TerminalMessage;
-use icy_engine_gui::{EditorMarkers, HorizontalScrollbarOverlay, MonitorSettings, ScalingMode, ScrollbarOverlay, Terminal, TerminalView, ZoomMessage};
+use icy_engine_gui::{EditorMarkers, MonitorSettings, ScalingMode, Terminal, TerminalView, ZoomMessage};
 use parking_lot::{Mutex, RwLock};
 
 /// Canvas view state for the ANSI editor
@@ -254,51 +254,59 @@ impl CanvasView {
     /// * `editor_markers` - Optional editor markers (layer bounds, selection, etc.)
     ///   The caller should set layer_bounds, selection_rect, etc. before calling view().
     pub fn view(&self, editor_markers: Option<EditorMarkers>) -> Element<'_, TerminalMessage> {
-        // Use TerminalView to render with CRT shader effect
-        let terminal_view = TerminalView::show_with_effects(&self.terminal, Arc::new(self.monitor_settings.read().clone()), editor_markers);
-
-        // Get scrollbar info using shared logic from icy_engine_gui
-        let scrollbar_info = self.terminal.scrollbar_info();
-
-        if scrollbar_info.needs_any_scrollbar() {
-            let mut layers: Vec<Element<'_, TerminalMessage>> = vec![terminal_view];
-
-            // Add vertical scrollbar if needed - uses viewport directly, no messages needed
-            if scrollbar_info.needs_vscrollbar {
-                let vscrollbar_view: Element<'_, ()> = ScrollbarOverlay::new(&self.terminal.viewport).view();
-                // Map () to CanvasMessage - scrollbar mutates viewport directly via Arc<RwLock>
-                let vscrollbar_mapped: Element<'_, TerminalMessage> = vscrollbar_view.map(|_| unreachable!());
-                let vscrollbar_container: container::Container<'_, TerminalMessage> =
-                    container(vscrollbar_mapped).width(Length::Fill).height(Length::Fill).align_x(Alignment::End);
-                layers.push(vscrollbar_container.into());
-            }
-
-            // Add horizontal scrollbar if needed - uses viewport directly, no messages needed
-            if scrollbar_info.needs_hscrollbar {
-                let hscrollbar_view: Element<'_, ()> = HorizontalScrollbarOverlay::new(&self.terminal.viewport).view();
-                let hscrollbar_mapped: Element<'_, TerminalMessage> = hscrollbar_view.map(|_| unreachable!());
-                let hscrollbar_container: container::Container<'_, TerminalMessage> =
-                    container(hscrollbar_mapped).width(Length::Fill).height(Length::Fill).align_y(Alignment::End);
-                layers.push(hscrollbar_container.into());
-            }
+        // Get scrollable content size from screen (virtual_size includes full document)
+        let screen = self.terminal.screen.lock();
+        let virtual_size = screen.virtual_size();
+        drop(screen);
+
+        // Get zoom from viewport
+        let zoom = self.terminal.viewport.read().zoom;
+
+        // Scrollable size in zoomed pixels (for scroll_area)
+        let scrollable_width = virtual_size.width as f32 * zoom;
+        let scrollable_height = virtual_size.height as f32 * zoom;
+
+        // Scrollable size in content pixels (for viewport)
+        let scrollable_content_width = virtual_size.width as f32;
+        let scrollable_content_height = virtual_size.height as f32;
+
+        let scrollable_size = icy_ui::Size::new(scrollable_width, scrollable_height);
+        let monitor_settings = Arc::new(self.monitor_settings.read().clone());
+        let viewport_arc = self.terminal.viewport.clone();
+
+        let content = scroll_area()
+            .width(Length::Fill)
+            .height(Length::Fill)
+            .direction(scrollable::Direction::Both {
+                vertical: scrollable::Scrollbar::new().width(8).scroller_width(6),
+                horizontal: scrollable::Scrollbar::new().width(8).scroller_width(6),
+            })
+            .show_viewport(scrollable_size, move |scroll_viewport| {
+                // Update terminal viewport with scroll position and content size from scroll_area
+                // scroll_viewport.x/y are in zoomed pixel coordinates
+                {
+                    let mut vp = viewport_arc.write();
+                    // Convert from zoomed coordinates back to content coordinates
+                    vp.scroll_x = scroll_viewport.x / zoom;
+                    vp.scroll_y = scroll_viewport.y / zoom;
+                    vp.visible_width = scroll_viewport.width;
+                    vp.visible_height = scroll_viewport.height;
+                    // Set scrollable content size (in content pixels, not zoomed)
+                    vp.content_width = scrollable_content_width;
+                    vp.content_height = scrollable_content_height;
+                }
+
+                TerminalView::show_with_effects(&self.terminal, monitor_settings.clone(), editor_markers.clone())
+                    .map(|msg| msg)
+            });
 
-            container(stack(layers))
-                .width(Length::Fill)
-                .height(Length::Fill)
-                .style(|theme: &icy_ui::Theme| container::Style {
-                    background: Some(icy_ui::Background::Color(main_area_background(theme))),
-                    ..Default::default()
-                })
-                .into()
-        } else {
-            container(terminal_view)
-                .width(Length::Fill)
-                .height(Length::Fill)
-                .style(|theme: &icy_ui::Theme| container::Style {
-                    background: Some(icy_ui::Background::Color(main_area_background(theme))),
-                    ..Default::default()
-                })
-                .into()
-        }
+        container(content)
+            .width(Length::Fill)
+            .height(Length::Fill)
+            .style(|theme: &icy_ui::Theme| container::Style {
+                background: Some(icy_ui::Background::Color(main_area_background(theme))),
+                ..Default::default()
+            })
+            .into()
     }
 }

crates/icy_draw/src/ui/editor/ansi/widget/layer_view/view.rs

@@ -33,8 +33,9 @@ use icy_engine_edit::EditState;
 use icy_engine_gui::theme::main_area_background;
 use icy_engine_gui::CheckerboardColors;
 use icy_engine_gui::DoubleClickDetector;
-use icy_engine_gui::{wrap_with_scrollbars, Viewport};
-use icy_ui::widget::menu::{context_menu, items as menu_items_fn, Item as MenuItemDef, Tree as MenuTree};
+use icy_engine_gui::Viewport;
+use icy_ui::menu::{item as menu_item, MenuNode};
+use icy_ui::widget::menu::context_menu;
 use parking_lot::Mutex;
 
 use crate::fl;
@@ -66,32 +67,6 @@ const PREVIEW_ATLAS_H: u32 = PREVIEW_TEX_H * PREVIEW_ATLAS_ROWS;
 
 const MAX_LABEL_CHARS: usize = 64;
 
-/// Context menu action for layers
-#[derive(Clone, Copy, Debug, PartialEq, Eq)]
-pub enum LayerContextAction {
-    Properties(usize),
-    NewLayer,
-    Duplicate(usize),
-    MergeDown(usize),
-    Delete(usize),
-    Clear(usize),
-}
-
-impl icy_ui::widget::menu::Action for LayerContextAction {
-    type Message = LayerMessage;
-
-    fn message(&self) -> Self::Message {
-        match *self {
-            LayerContextAction::Properties(i) => LayerMessage::EditLayer(i),
-            LayerContextAction::NewLayer => LayerMessage::Add,
-            LayerContextAction::Duplicate(i) => LayerMessage::Duplicate(i),
-            LayerContextAction::MergeDown(i) => LayerMessage::MergeDown(i),
-            LayerContextAction::Delete(i) => LayerMessage::Remove(i),
-            LayerContextAction::Clear(i) => LayerMessage::Clear(i),
-        }
-    }
-}
-
 /// Messages for the layer view
 #[derive(Clone, Debug)]
 pub enum LayerMessage {
@@ -577,31 +552,26 @@ impl LayerView {
     }
 
     /// Build the context menu items for a layer
-    fn build_context_menu_items(index: usize, layer_count: usize) -> Vec<MenuTree<'static, LayerMessage, Theme, icy_ui::Renderer>> {
-        use icy_ui::widget::menu::KeyBind;
-        use std::collections::HashMap;
-
-        let key_binds: HashMap<KeyBind, LayerContextAction> = HashMap::new();
-
+    fn build_context_menu_items(index: usize, layer_count: usize) -> Vec<MenuNode<LayerMessage>> {
         let mut items = vec![
-            MenuItemDef::Button(fl!("layer_tool_menu_layer_properties"), LayerContextAction::Properties(index)),
-            MenuItemDef::Button(fl!("layer_tool_menu_new_layer"), LayerContextAction::NewLayer),
-            MenuItemDef::Button(fl!("layer_tool_menu_duplicate_layer"), LayerContextAction::Duplicate(index)),
+            menu_item!(fl!("layer_tool_menu_layer_properties"), LayerMessage::EditLayer(index)),
+            menu_item!(fl!("layer_tool_menu_new_layer"), LayerMessage::Add),
+            menu_item!(fl!("layer_tool_menu_duplicate_layer"), LayerMessage::Duplicate(index)),
         ];
 
         // Merge is only available if not the bottom layer
         if index > 0 {
-            items.push(MenuItemDef::Button(fl!("layer_tool_menu_merge_layer"), LayerContextAction::MergeDown(index)));
+            items.push(menu_item!(fl!("layer_tool_menu_merge_layer"), LayerMessage::MergeDown(index)));
         }
 
         // Delete is only available if there's more than one layer
         if layer_count > 1 {
-            items.push(MenuItemDef::Button(fl!("layer_tool_menu_delete_layer"), LayerContextAction::Delete(index)));
+            items.push(menu_item!(fl!("layer_tool_menu_delete_layer"), LayerMessage::Remove(index)));
         }
 
-        items.push(MenuItemDef::Button(fl!("layer_tool_menu_clear_layer"), LayerContextAction::Clear(index)));
+        items.push(menu_item!(fl!("layer_tool_menu_clear_layer"), LayerMessage::Clear(index)));
 
-        menu_items_fn(&key_binds, items)
+        items
     }
 
     /// Create a single menu item button
@@ -688,8 +658,8 @@ impl LayerView {
         }
         .into();
 
-        let needs_scrollbar = self.viewport.borrow().is_scrollable_y();
-        let list_with_scrollbar = wrap_with_scrollbars(list_widget, &self.viewport, needs_scrollbar, false);
+        // Layer list (scrollbars handled elsewhere if needed)
+        let list_with_scrollbar = list_widget;
 
         let scroll_y = self.viewport.borrow().scroll_y;
         let icon_color = theme.background.on;
@@ -763,7 +733,7 @@ impl LayerView {
         // Wrap with context menu (for current selection)
         if layer_count > 0 {
             let menu_items = Self::build_context_menu_items(current_layer, layer_count);
-            context_menu(content, Some(menu_items)).into()
+            context_menu(content, &menu_items).into()
         } else {
             content.into()
         }

crates/icy_engine_gui/src/scrollbar/mod.rs

@@ -5,11 +5,27 @@
 pub mod state;
 pub use state::*;
 
-pub mod overlay;
-pub use overlay::{ScrollbarOverlay, ScrollbarOverlayCallback, ScrollbarOverlayState, ViewportAccess};
+// Overlay scrollbars are deprecated - use scroll_area with show_viewport instead
+// Keeping source files for reference/helper methods
+// pub mod overlay;
+// pub use overlay::{ScrollbarOverlay, ScrollbarOverlayCallback, ScrollbarOverlayState, ViewportAccess};
 
-pub mod horizontal_overlay;
-pub use horizontal_overlay::{HorizontalScrollbarOverlay, HorizontalScrollbarOverlayCallback, HorizontalScrollbarOverlayState};
+// pub mod horizontal_overlay;
+// pub use horizontal_overlay::{HorizontalScrollbarOverlay, HorizontalScrollbarOverlayCallback, HorizontalScrollbarOverlayState};
 
-pub mod info;
-pub use info::*;
+// pub mod info;
+// pub use info::*;
+
+use icy_ui::Element;
+
+/// Deprecated: Wrap content with scrollbar overlays
+/// This is now a no-op - use scroll_area with show_viewport instead for native scrollbars
+#[deprecated(note = "Use scroll_area with show_viewport instead")]
+pub fn wrap_with_scrollbars<'a, Message: 'a>(
+    content: Element<'a, Message>,
+    _needs_vscrollbar: bool,
+    _needs_hscrollbar: bool,
+) -> Element<'a, Message> {
+    // Just return content - scrollbars should be handled by scroll_area now
+    content
+}

crates/icy_engine_gui/src/terminal/crt_program.rs

@@ -280,7 +280,6 @@ impl<'a> CRTShaderProgram<'a> {
                 scroll_offset_y = params.scroll_offset_y;
                 scroll_offset_x = params.scroll_offset_x;
             }
-
             if cfg!(debug_assertions) && std::env::var("ICY_DEBUG_VIEWPORT").is_ok() && viewport_changed {
                 let max_scroll_x = (content_width - visible_width).max(0.0);
                 let max_scroll_y = (content_height - visible_height).max(0.0);

crates/icy_engine_gui/src/terminal/crt_state.rs

@@ -322,11 +322,27 @@ pub fn create_crt_shader<'a>(
     monitor_settings: Arc<MonitorSettings>,
     editor_markers: Option<crate::EditorMarkers>,
 ) -> Element<'a, TerminalMessage> {
-    // Let the parent wrapper decide sizing; shader can just be Fill.
-    shader(CRTShaderProgram::new(term, monitor_settings, editor_markers))
-        .width(icy_ui::Length::Fill)
-        .height(icy_ui::Length::Fill)
-        .into()
+    // Get content size directly from screen (virtual_size is in pixels)
+    let screen = term.screen.lock();
+    let virtual_size = screen.virtual_size();
+    drop(screen);
+
+    // Get zoom from viewport (still needed for zoom level)
+    let zoom = term.viewport.read().zoom;
+
+    let zoomed_width = (virtual_size.width as f32 * zoom).max(1.0);
+    let zoomed_height = (virtual_size.height as f32 * zoom).max(1.0);
+
+    // Wrap the shader in a container with fixed size so it works inside scrollable
+    // The shader itself still uses Fill to expand into this container
+    icy_ui::widget::container(
+        shader(CRTShaderProgram::new(term, monitor_settings, editor_markers))
+            .width(icy_ui::Length::Fill)
+            .height(icy_ui::Length::Fill),
+    )
+    .width(icy_ui::Length::Fixed(zoomed_width))
+    .height(icy_ui::Length::Fixed(zoomed_height))
+    .into()
 }
 
 static SCALE_FACTOR_BITS: AtomicU32 = AtomicU32::new(f32::to_bits(1.0));

crates/icy_engine_gui/src/terminal/mod.rs

@@ -103,7 +103,7 @@ impl Terminal {
 
     /// Update viewport when screen size changes
     pub fn update_viewport_size(&mut self) {
-        let scr = self.screen.lock();
+        let scr: parking_lot::lock_api::MutexGuard<'_, parking_lot::RawMutex, Box<dyn Screen + 'static>> = self.screen.lock();
         let virtual_size = scr.virtual_size();
         drop(scr);