feat: enhance PlainSlider to eliminate visual artifacts and improve boundary handling

.kilo/plans/1784908213566-iced-plain-slider-fix.md

@@ -0,0 +1,149 @@
+# Fix Iced PlainSlider Visual Artifacts and Boundary Handling
+
+## Goal
+Clean up the horizontal rendering artifacts on the Iced `PlainSlider` track and ensure the slider can be dragged to the absolute minimum (0 %) and maximum (100 %) values even when the mouse cursor moves slightly outside the widget bounds.
+
+## Scope
+Only the Iced GUI slider implementation is in scope:
+- `hcie-iced-app/crates/hcie-iced-gui/src/widgets/plain_slider.rs`
+- Re-exported by `hcie-iced-app/crates/iced-panel-adapter/src/lib.rs`
+
+The egui `PlainSlider` is explicitly out of scope (user chose option A).
+
+## Root Cause Analysis
+
+### 1. Horizontal lines / visual artifacts
+The value fill is currently drawn as 4 horizontal bands:
+```rust
+let bands = 4;
+for band in 0..bands {
+    let t = band as f32 / (bands - 1) as f32;
+    let mut fill = mix_color(...);
+    frame.fill_rectangle(
+        Point::new(0.0, bounds.height * band as f32 / bands as f32),
+        Size::new(fill_width, bounds.height / bands as f32 + 0.5),
+        fill,
+    );
+}
+```
+The `+ 0.5` height overlap and sub-pixel band edges create visible horizontal seams/aliasing, especially on high-DPI displays or when the widget height is small (22 px). The vertical handle line drawn on top also adds to the perceived noise.
+
+### 2. Cannot reach 0 % / 100 % with the mouse
+The current drag handler only processes `CursorMoved` while `local` (cursor position in bounds) is `Some`:
+```rust
+canvas::Event::Mouse(mouse::Event::CursorMoved { .. }) if state.dragging => {
+    if let Some(position) = local {
+        let value = self.value_at(position.x, bounds.width);
+        ...
+    }
+    (canvas::event::Status::Captured, None)
+}
+```
+When the user drags past the left or right edge of the slider, Iced stops reporting a local position, so the value is never clamped to the endpoint. The slider gets stuck just short of 0 % or 100 %.
+
+In addition, `value_at` normalizes against `track_width = width - SPINNER_WIDTH`, so the effective draggable area is narrower than the full widget. The rightmost spinner region is reserved for step buttons, but the boundary between track and spinner is sensitive to small mouse movements.
+
+## Implementation Plan
+
+### Change A: Remove band-based fill artifact
+Replace the 4-band gradient loop with a single rounded rectangle fill.
+
+Suggested draw code:
+```rust
+let track_width = (bounds.width - SPINNER_WIDTH).max(0.0);
+let fill_width = track_width * self.fraction();
+if fill_width > 0.0 {
+    let fill = self.colors.accent;
+    let fill_rect = Path::rounded_rectangle(
+        Point::new(0.0, 0.0),
+        Size::new(fill_width, bounds.height),
+        radius,
+    );
+    frame.fill(&fill_rect, fill);
+}
+```
+This eliminates:
+- The `+ 0.5` band overlap
+- Sub-pixel horizontal edges
+- The separate vertical handle line (the filled rounded rectangle itself acts as the value indicator)
+
+If a gradient is still desired, use `canvas::Gradient` or a single linear gradient path instead of multiple overlapping rectangles.
+
+### Change B: Clamp to endpoints when dragging outside bounds
+Use the global cursor position during an active drag so the slider value can continue updating even when the cursor leaves the widget.
+
+In `update`:
+1. Store `dragging` state as already done.
+2. In the `CursorMoved` branch, if `state.dragging`:
+   - If `local` is `Some`, use `position.x`.
+   - If `local` is `None`, fall back to `cursor.position()` (global) and convert it to widget-local X by subtracting `bounds.x`.
+   - Clamp the resulting X to `[0.0, track_width]`.
+   - Compute the value with `value_at` and emit `on_change`.
+
+Suggested helper:
+```rust
+fn drag_value(&self, cursor: mouse::Cursor, bounds: Rectangle) -> f32 {
+    let track_width = (bounds.width - SPINNER_WIDTH).max(1.0);
+    let x = match cursor.position_in(bounds) {
+        Some(local) => local.x,
+        None => cursor.position().map_or(0.0, |p| p.x - bounds.x),
+    }
+    .clamp(0.0, track_width);
+    self.value_at(x, bounds.width)
+}
+```
+
+This ensures:
+- Dragging left of the widget clamps to `range.start()` (0 %).
+- Dragging right of the widget clamps to `range.end()` (100 %).
+- Normal in-bounds dragging still works exactly as before.
+
+### Change C: Preserve spinner step behavior while improving boundary feel
+Do **not** remove the spinner buttons. Keep the existing step-button logic in `ButtonPressed`:
+```rust
+if position.x >= bounds.width - SPINNER_WIDTH {
+    // up/down step
+}
+```
+The boundary fix in Change B applies only during active drags. A click in the spinner area still performs a single step; a drag that started in the track area can now overshoot the track/spinner boundary and clamp correctly.
+
+### Change D: Update or add unit tests
+The existing test `pointer_mapping_respects_endpoints_and_step` already covers in-bounds endpoint mapping. Extend it with out-of-bounds cases:
+
+```rust
+#[test]
+fn pointer_mapping_clamps_outside_track() {
+    let slider = test_slider("");
+    assert_eq!(slider.value_at(-10.0, 124.0), 0.0);
+    assert_eq!(slider.value_at(120.0, 124.0), 1.0);
+}
+```
+
+Add a test for the global-to-local conversion logic if it is extracted into a testable helper.
+
+## Validation Steps
+1. Run the existing widget tests:
+   ```bash
+   cargo test -p hcie-iced-gui widgets::plain_slider
+   ```
+2. Build the iced GUI crate:
+   ```bash
+   cargo check -p hcie-iced-gui
+   cargo check -p iced-panel-adapter
+   ```
+3. Manual visual check (if running the app):
+   - Open a panel with a slider such as Opacity / Alpha.
+   - Verify no horizontal streaks across the slider fill.
+   - Drag the mouse left past the widget edge: value should snap to 0 %.
+   - Drag the mouse right past the widget edge: value should snap to 100 %.
+   - Verify spinner arrows still increment/decrement the value by one step.
+
+## Risks and Mitigations
+| Risk | Mitigation |
+|------|------------|
+| Removing the gradient makes the slider look too flat | Use a single `canvas::Gradient` linear fill if the design requires a gradient; do not reintroduce overlapping bands. |
+| Global cursor fallback reports incorrect position on multi-window / scaled displays | Iced's `cursor.position()` is in logical window coordinates; subtracting `bounds.x` (also logical) is safe. Test on the target platform. |
+| Spinner clicks accidentally interpreted as drags | The `ButtonPressed` branch already returns `Captured` immediately for spinner clicks. Drags are only detected via `CursorMoved` after a non-spinner press, so the distinction remains. |
+
+## Open Questions
+None — the user confirmed Iced-only scope (option A) and the design direction is straightforward.

hcie-iced-app/crates/hcie-iced-gui/src/widgets/plain_slider.rs

@@ -110,6 +110,25 @@ impl<Message> PlainSlider<Message> {
         self.snap(*self.range.start() + (*self.range.end() - *self.range.start()) * fraction)
     }
 
+    /// Computes the slider value for the current cursor position while dragging.
+    ///
+    /// **Purpose:** Allow an active drag to continue updating even when the cursor moves
+    /// slightly outside the widget bounds, so 0 % and 100 % can always be reached.
+    /// **Logic & Workflow:** If the cursor is inside the widget bounds, use its local X.
+    /// Otherwise fall back to the global cursor position and convert it to widget-local
+    /// X by subtracting `bounds.x`. Clamp the resulting X to the draggable track width.
+    /// **Arguments:** `cursor` is the current mouse cursor; `bounds` is the widget rectangle.
+    /// **Returns:** The stepped value at the clamped cursor position.
+    fn drag_value(&self, cursor: mouse::Cursor, bounds: Rectangle) -> f32 {
+        let track_width = (bounds.width - SPINNER_WIDTH).max(1.0);
+        let x = match cursor.position_in(bounds) {
+            Some(local) => local.x,
+            None => cursor.position().map_or(0.0, |p| p.x - bounds.x),
+        }
+        .clamp(0.0, track_width);
+        self.value_at(x, bounds.width)
+    }
+
     /// Returns the normalized fill fraction for the current value.
     fn fraction(&self) -> f32 {
         let span = *self.range.end() - *self.range.start();
@@ -170,22 +189,10 @@ impl<Message> canvas::Program<Message> for PlainSlider<Message> {
         let track_width = (bounds.width - SPINNER_WIDTH).max(0.0);
         let fill_width = track_width * self.fraction();
         if fill_width > 0.0 {
-            let bands = 4;
+            let mut fill = self.colors.accent;
-            for band in 0..bands {
+            fill.a = if self.colors.is_light { 0.30 } else { 0.25 };
-                let t = band as f32 / (bands - 1) as f32;
+            let fill_rect = Path::rounded_rectangle(Point::new(0.0, 0.0), Size::new(fill_width, bounds.height), radius);
-                let mut fill = mix_color(self.colors.accent, self.colors.accent_hover, t * 0.55);
+            frame.fill(&fill_rect, fill);
-                fill.a = if self.colors.is_light { 0.30 } else { 0.25 };
-                frame.fill_rectangle(
-                    Point::new(0.0, bounds.height * band as f32 / bands as f32),
-                    Size::new(fill_width, bounds.height / bands as f32 + 0.5),
-                    fill,
-                );
-            }
-            frame.fill_rectangle(
-                Point::new((fill_width - 1.5).max(0.0), 1.0),
-                Size::new(3.0_f32.min(fill_width), (bounds.height - 2.0).max(0.0)),
-                self.colors.accent,
-            );
         }
 
         frame.stroke(
@@ -298,14 +305,11 @@ impl<Message> canvas::Program<Message> for PlainSlider<Message> {
                 (canvas::event::Status::Ignored, None)
             }
             canvas::Event::Mouse(mouse::Event::CursorMoved { .. }) if state.dragging => {
-                if let Some(position) = local {
+                let value = self.drag_value(cursor, bounds);
-                    let value = self.value_at(position.x, bounds.width);
+                return (
-                    return (
+                    canvas::event::Status::Captured,
-                        canvas::event::Status::Captured,
+                    Some((self.on_change)(value)),
-                        Some((self.on_change)(value)),
+                );
-                    );
-                }
-                (canvas::event::Status::Captured, None)
             }
             canvas::Event::Mouse(mouse::Event::ButtonReleased(mouse::Button::Left)) => {
                 if state.dragging {
@@ -409,17 +413,6 @@ fn is_numeric_fragment(text: &str) -> bool {
             .all(|character| character.is_ascii_digit() || matches!(character, '.' | ',' | '-'))
 }
 
-/// Blends two theme colors while preserving alpha interpolation.
-fn mix_color(from: iced::Color, to: iced::Color, amount: f32) -> iced::Color {
-    let amount = amount.clamp(0.0, 1.0);
-    iced::Color::new(
-        from.r + (to.r - from.r) * amount,
-        from.g + (to.g - from.g) * amount,
-        from.b + (to.b - from.b) * amount,
-        from.a + (to.a - from.a) * amount,
-    )
-}
-
 #[cfg(test)]
 mod tests {
     use super::{is_numeric_fragment, PlainSlider, State};
@@ -463,4 +456,11 @@ mod tests {
         assert_eq!(slider.value_at(100.0, 124.0), 1.0);
         assert!((slider.value_at(33.0, 124.0) - 0.33).abs() < 1e-6);
     }
+
+    #[test]
+    fn pointer_mapping_clamps_outside_track() {
+        let slider = test_slider("");
+        assert_eq!(slider.value_at(-10.0, 124.0), 0.0);
+        assert_eq!(slider.value_at(120.0, 124.0), 1.0);
+    }
 }