From d05c459e92d4830281e96b5d082ae266bfcf1cbb Mon Sep 17 00:00:00 2001 From: wfx Date: Wed, 4 Feb 2026 18:25:39 +0100 Subject: [PATCH] fix: Simplify crop_overlay - NO coordinate transformation KISS principle: Widget is in stack over canvas, uses SAME coordinates! - Mouse position = exact position to draw - selection.region = canvas pixel coordinates - NO relative/absolute conversion needed - NO bounds offset calculations Removed all the over-engineered coordinate math. --- src/ui/widgets/crop_overlay.rs | 187 ++++++++++++++------------------- 1 file changed, 79 insertions(+), 108 deletions(-) diff --git a/src/ui/widgets/crop_overlay.rs b/src/ui/widgets/crop_overlay.rs index aef274f..1821550 100644 --- a/src/ui/widgets/crop_overlay.rs +++ b/src/ui/widgets/crop_overlay.rs @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-3.0-or-later // src/ui/widgets/crop_overlay.rs // -// Simple crop overlay widget. +// Simple crop overlay - draws directly on canvas coordinates. use cosmic::{ Element, Renderer, @@ -29,9 +29,10 @@ const HANDLE_COLOR: Color = Color::WHITE; const BORDER_COLOR: Color = Color::WHITE; const BORDER_WIDTH: f32 = 2.0; -/// Simple crop overlay widget. +/// Crop overlay widget. /// -/// Works with RELATIVE coordinates - selection.region is relative to bounds (0,0). +/// Simple: Draws at exact mouse coordinates. No coordinate transformation! +/// selection.region is in canvas pixel coordinates, we draw at those exact pixels. pub struct CropOverlay { selection: CropSelection, show_grid: bool, @@ -42,28 +43,18 @@ impl CropOverlay { pub fn new(selection: &CropSelection, show_grid: bool) -> Self { Self { selection: selection.clone(), - last_click: None, show_grid, + last_click: None, } } - /// Convert relative coords to absolute screen coords. - fn to_screen(&self, x: f32, y: f32, bounds: &Rectangle) -> Point { - Point::new(bounds.x + x, bounds.y + y) - } - - /// Convert absolute screen coords to relative coords. - fn to_relative(&self, point: Point, bounds: &Rectangle) -> Point { - Point::new(point.x - bounds.x, point.y - bounds.y) - } - - /// Hit test for handles (in relative coordinates). - fn hit_test_handle(&self, rel_point: Point) -> DragHandle { + /// Hit test for handles - direct coordinate comparison. + fn hit_test_handle(&self, point: Point) -> DragHandle { let Some((x, y, w, h)) = self.selection.region else { return DragHandle::None; }; - // 8 handle positions (relative coordinates) + // 8 handle positions let handles = [ (Point::new(x, y), DragHandle::TopLeft), (Point::new(x + w, y), DragHandle::TopRight), @@ -75,55 +66,46 @@ impl CropOverlay { (Point::new(x + w, y + h / 2.0), DragHandle::Right), ]; - // Test handles + // Check handles for (pos, handle) in handles { - if point_in_handle(rel_point, pos) { + if point_in_handle(point, pos) { return handle; } } - // Test if inside selection (move) - if rel_point.x >= x && rel_point.x <= x + w && rel_point.y >= y && rel_point.y <= y + h { + // Check if inside selection (move) + if point.x >= x && point.x <= x + w && point.y >= y && point.y <= y + h { return DragHandle::Move; } DragHandle::None } - /// Draw darkened overlay (4 rectangles around selection). + /// Draw darkened overlay outside selection. fn draw_overlay(&self, renderer: &mut Renderer, bounds: Rectangle) { let Some((x, y, w, h)) = self.selection.region else { - // No selection - darken entire canvas + // No selection - darken everything draw_quad(renderer, bounds, OVERLAY_COLOR); return; }; - // Convert to absolute screen coordinates - let sel_x = bounds.x + x; - let sel_y = bounds.y + y; - let sel_right = sel_x + w; - let sel_bottom = sel_y + h; + // Selection rectangle + let sel_right = x + w; + let sel_bottom = y + h; - // Clamp to bounds - let sel_x = sel_x.max(bounds.x); - let sel_y = sel_y.max(bounds.y); - let sel_right = sel_right.min(bounds.x + bounds.width); - let sel_bottom = sel_bottom.min(bounds.y + bounds.height); - - // Draw 4 overlay rectangles - // Top - if sel_y > bounds.y { + // Top overlay + if y > bounds.y { draw_quad( renderer, Rectangle::new( Point::new(bounds.x, bounds.y), - Size::new(bounds.width, sel_y - bounds.y), + Size::new(bounds.width, y - bounds.y), ), OVERLAY_COLOR, ); } - // Bottom + // Bottom overlay if sel_bottom < bounds.y + bounds.height { draw_quad( renderer, @@ -135,45 +117,41 @@ impl CropOverlay { ); } - // Left - if sel_x > bounds.x { + // Left overlay + if x > bounds.x { draw_quad( renderer, Rectangle::new( - Point::new(bounds.x, sel_y), - Size::new(sel_x - bounds.x, sel_bottom - sel_y), + Point::new(bounds.x, y), + Size::new(x - bounds.x, h), ), OVERLAY_COLOR, ); } - // Right + // Right overlay if sel_right < bounds.x + bounds.width { draw_quad( renderer, Rectangle::new( - Point::new(sel_right, sel_y), - Size::new(bounds.x + bounds.width - sel_right, sel_bottom - sel_y), + Point::new(sel_right, y), + Size::new(bounds.x + bounds.width - sel_right, h), ), OVERLAY_COLOR, ); } } - /// Draw border (4 lines). - fn draw_border(&self, renderer: &mut Renderer, bounds: Rectangle) { + /// Draw border. + fn draw_border(&self, renderer: &mut Renderer) { let Some((x, y, w, h)) = self.selection.region else { return; }; - // Convert to absolute screen coordinates - let sx = bounds.x + x; - let sy = bounds.y + y; - // Top draw_quad( renderer, - Rectangle::new(Point::new(sx, sy), Size::new(w, BORDER_WIDTH)), + Rectangle::new(Point::new(x, y), Size::new(w, BORDER_WIDTH)), BORDER_COLOR, ); @@ -181,7 +159,7 @@ impl CropOverlay { draw_quad( renderer, Rectangle::new( - Point::new(sx, sy + h - BORDER_WIDTH), + Point::new(x, y + h - BORDER_WIDTH), Size::new(w, BORDER_WIDTH), ), BORDER_COLOR, @@ -190,7 +168,7 @@ impl CropOverlay { // Left draw_quad( renderer, - Rectangle::new(Point::new(sx, sy), Size::new(BORDER_WIDTH, h)), + Rectangle::new(Point::new(x, y), Size::new(BORDER_WIDTH, h)), BORDER_COLOR, ); @@ -198,31 +176,31 @@ impl CropOverlay { draw_quad( renderer, Rectangle::new( - Point::new(sx + w - BORDER_WIDTH, sy), + Point::new(x + w - BORDER_WIDTH, y), Size::new(BORDER_WIDTH, h), ), BORDER_COLOR, ); } - /// Draw handles (8 squares). - fn draw_handles(&self, renderer: &mut Renderer, bounds: Rectangle) { + /// Draw handles. + fn draw_handles(&self, renderer: &mut Renderer) { let Some((x, y, w, h)) = self.selection.region else { return; }; let half = HANDLE_SIZE / 2.0; - // 8 handle positions (relative, then convert to screen) + // 8 handle positions let handles = [ - self.to_screen(x, y, &bounds), - self.to_screen(x + w, y, &bounds), - self.to_screen(x, y + h, &bounds), - self.to_screen(x + w, y + h, &bounds), - self.to_screen(x + w / 2.0, y, &bounds), - self.to_screen(x + w / 2.0, y + h, &bounds), - self.to_screen(x, y + h / 2.0, &bounds), - self.to_screen(x + w, y + h / 2.0, &bounds), + Point::new(x, y), + Point::new(x + w, y), + Point::new(x, y + h), + Point::new(x + w, y + h), + Point::new(x + w / 2.0, y), + Point::new(x + w / 2.0, y + h), + Point::new(x, y + h / 2.0), + Point::new(x + w, y + h / 2.0), ]; for pos in handles { @@ -237,8 +215,8 @@ impl CropOverlay { } } - /// Draw rule-of-thirds grid. - fn draw_grid(&self, renderer: &mut Renderer, bounds: Rectangle) { + /// Draw grid. + fn draw_grid(&self, renderer: &mut Renderer) { if !self.show_grid { return; } @@ -251,30 +229,26 @@ impl CropOverlay { return; } - // Convert to absolute screen coordinates - let sx = bounds.x + x; - let sy = bounds.y + y; - let grid_color = Color::from_rgba(1.0, 1.0, 1.0, 0.3); let third_w = w / 3.0; let third_h = h / 3.0; // 2 vertical lines for i in 1..3 { - let line_x = sx + third_w * i as f32; + let line_x = x + third_w * i as f32; draw_quad( renderer, - Rectangle::new(Point::new(line_x, sy), Size::new(1.0, h)), + Rectangle::new(Point::new(line_x, y), Size::new(1.0, h)), grid_color, ); } // 2 horizontal lines for i in 1..3 { - let line_y = sy + third_h * i as f32; + let line_y = y + third_h * i as f32; draw_quad( renderer, - Rectangle::new(Point::new(sx, line_y), Size::new(w, 1.0)), + Rectangle::new(Point::new(x, line_y), Size::new(w, 1.0)), grid_color, ); } @@ -303,9 +277,9 @@ impl Widget for CropOverlay { let bounds = layout.bounds(); self.draw_overlay(renderer, bounds); - self.draw_border(renderer, bounds); - self.draw_handles(renderer, bounds); - self.draw_grid(renderer, bounds); + self.draw_border(renderer); + self.draw_handles(renderer); + self.draw_grid(renderer); } fn on_event( @@ -323,40 +297,38 @@ impl Widget for CropOverlay { match event { Event::Mouse(mouse::Event::ButtonPressed(Button::Left)) => { - if let Some(screen_pos) = cursor.position_in(bounds) { - let rel_pos = self.to_relative(screen_pos, &bounds); - let handle = self.hit_test_handle(rel_pos); + if let Some(pos) = cursor.position_in(bounds) { + let handle = self.hit_test_handle(pos); + + // Check for double-click on Move handle + if handle == DragHandle::Move { + use std::time::{Duration, Instant}; + let now = Instant::now(); + if let Some(last) = self.last_click { + if now.duration_since(last) < Duration::from_millis(400) { + // Double-click - apply crop + shell.publish(AppMessage::ApplyCrop); + self.last_click = None; + return Status::Captured; + } + } + self.last_click = Some(now); + } shell.publish(AppMessage::CropDragStart { - x: rel_pos.x, - y: rel_pos.y, + x: pos.x, + y: pos.y, handle, }); return Status::Captured; - - // Check for double-click on Move handle - if handle == DragHandle::Move { - use std::time::{Duration, Instant}; - let now = Instant::now(); - if let Some(last) = self.last_click { - if now.duration_since(last) < Duration::from_millis(400) { - // Double-click detected - apply crop - shell.publish(AppMessage::ApplyCrop); - self.last_click = None; - return Status::Captured; - } - } - self.last_click = Some(now); - } } } Event::Mouse(mouse::Event::CursorMoved { .. }) => { if self.selection.is_dragging { - if let Some(screen_pos) = cursor.position_in(bounds) { - let rel_pos = self.to_relative(screen_pos, &bounds); + if let Some(pos) = cursor.position_in(bounds) { shell.publish(AppMessage::CropDragMove { - x: rel_pos.x, - y: rel_pos.y, + x: pos.x, + y: pos.y, max_x: bounds.width, max_y: bounds.height, }); @@ -386,9 +358,8 @@ impl Widget for CropOverlay { ) -> mouse::Interaction { let bounds = layout.bounds(); - if let Some(screen_pos) = cursor.position_in(bounds) { - let rel_pos = self.to_relative(screen_pos, &bounds); - let handle = self.hit_test_handle(rel_pos); + if let Some(pos) = cursor.position_in(bounds) { + let handle = self.hit_test_handle(pos); return match handle { DragHandle::TopLeft | DragHandle::BottomRight => { mouse::Interaction::ResizingDiagonallyDown @@ -422,7 +393,7 @@ fn point_in_handle(point: Point, handle_center: Point) -> bool { && point.y <= handle_center.y + half } -/// Helper: Draw a filled quad. +/// Helper: Draw quad. fn draw_quad(renderer: &mut Renderer, bounds: Rectangle, color: Color) { renderer.fill_quad( Quad {