Files
hcie-rust-v3.05/.kilo/plans/1784908213566-iced-plain-slider-fix.md
T

150 lines
6.4 KiB
Markdown
Raw Normal View History

# 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.