diff --git a/hcie-document/src/lib.rs b/hcie-document/src/lib.rs index b2f365c..59e1b1b 100644 --- a/hcie-document/src/lib.rs +++ b/hcie-document/src/lib.rs @@ -88,7 +88,7 @@ impl Document { } pub fn delete_layer(&mut self, idx: usize) { - if self.layers.len() <= 1 || idx >= self.layers.len() { return; } + if idx >= self.layers.len() { return; } let layer = &self.layers[idx]; let action = LayerDeleteAction { diff --git a/hcie-engine-api/src/ffi.rs b/hcie-engine-api/src/ffi.rs index 68a0c1b..f80e6f5 100644 --- a/hcie-engine-api/src/ffi.rs +++ b/hcie-engine-api/src/ffi.rs @@ -225,7 +225,7 @@ pub unsafe extern "C" fn hcie_engine_flatten_image(handle: *mut EngineHandle) { #[no_mangle] pub unsafe extern "C" fn hcie_engine_clear_active_layer(handle: *mut EngineHandle) { with_engine(handle, |e| { - e.clear_active_layer_pixels(); + e.clear_active_layer_pixels_undoable(); }); } diff --git a/hcie-engine-api/src/layer_property_ops.rs b/hcie-engine-api/src/layer_property_ops.rs index 986fb72..4104665 100644 --- a/hcie-engine-api/src/layer_property_ops.rs +++ b/hcie-engine-api/src/layer_property_ops.rs @@ -97,6 +97,11 @@ impl Engine { self.below_cache_dirty = true; } + /// Return the index of the currently active layer. + pub fn active_layer_index(&self) -> usize { + self.document.active_layer + } + /// Switch the active (selected) layer by ID. pub fn set_active_layer(&mut self, id: u64) { if let Some(idx) = self.document.layer_index_by_id(id) { @@ -270,6 +275,32 @@ impl Engine { self.below_cache_dirty = true; } + /// Push a full-layer pixel snapshot for the currently active layer. + /// + /// **Purpose:** Lets callers that already modified the active layer pixels + /// through `set_active_layer_pixels` still record an undo step without + /// needing access to the internal `Document` type. + /// + /// **Arguments:** + /// * `before_pixels` — the layer RGBA buffer captured before the change. + /// * `layer_idx` — index of the layer to snapshot (typically the active layer). + /// * `description` — human-readable history entry label. + /// + /// **Side Effects:** Appends a `DrawAction` to the document history. + pub fn push_active_layer_snapshot( + &mut self, + before_pixels: Vec, + layer_idx: usize, + description: &str, + ) { + self.document.push_draw_snapshot( + layer_idx, + before_pixels, + None, + description.to_string(), + ); + } + pub fn undo(&mut self) -> bool { if self.document.can_undo() { self.document.undo(); diff --git a/hcie-engine-api/src/lib.rs b/hcie-engine-api/src/lib.rs index 15a9fb2..86df77e 100644 --- a/hcie-engine-api/src/lib.rs +++ b/hcie-engine-api/src/lib.rs @@ -3400,11 +3400,32 @@ impl Engine { } /// Clear pixels only within the active selection mask. If no selection, clears all. + /// Records a full-layer draw snapshot so the operation is undoable. pub fn clear_selection_pixels(&mut self) { - let mask = self.cached_selection_mask.clone(); + let mask = self.document.selection_mask.clone(); + let active_idx = self.document.active_layer; + log::debug!( + "[Engine::clear_selection_pixels] active_layer={}, selection_active={}, document_mask_len={:?}, stroke_cache_mask_len={:?}", + active_idx, + self.document.selection_active, + mask.as_ref().map(Vec::len), + self.cached_selection_mask.as_ref().map(Vec::len), + ); if let Some(ref mask_data) = mask { let w = self.document.canvas_width as usize; let h = self.document.canvas_height as usize; + let selected_pixels = mask_data.iter().filter(|coverage| **coverage > 0).count(); + log::debug!( + "[Engine::clear_selection_pixels] clearing selected region: canvas={}x{}, selected_pixels={}", + w, + h, + selected_pixels, + ); + let before_pixels = self + .document + .active_layer() + .map(|layer| layer.pixels.clone()) + .unwrap_or_default(); if let Some(layer) = self.document.active_layer_mut() { for y in 0..h { for x in 0..w { @@ -3418,14 +3439,45 @@ impl Engine { } } layer.dirty = true; - self.document.composite_dirty = true; - self.document.modified = true; } + self.document.push_draw_snapshot( + active_idx, + before_pixels, + None, + "Clear Selection".to_string(), + ); + self.document.composite_dirty = true; + self.document.modified = true; } else { - self.clear_active_layer_pixels(); + log::debug!( + "[Engine::clear_selection_pixels] no active document mask; clearing the entire active layer" + ); + self.clear_active_layer_pixels_undoable(); } } + /// Clears the entire active layer and records an undo snapshot. + pub fn clear_active_layer_pixels_undoable(&mut self) { + let active_idx = self.document.active_layer; + let before_pixels = self + .document + .active_layer() + .map(|layer| layer.pixels.clone()) + .unwrap_or_default(); + if let Some(layer) = self.document.active_layer_mut() { + layer.pixels.fill(0); + layer.dirty = true; + } + self.document.push_draw_snapshot( + active_idx, + before_pixels, + None, + "Clear Layer".to_string(), + ); + self.document.composite_dirty = true; + self.document.modified = true; + } + pub fn set_active_layer_pixels(&mut self, pixels: Vec) { if let Some(layer) = self.document.active_layer_mut() { layer.pixels = pixels; diff --git a/hcie-engine-api/tests/selection_clear.rs b/hcie-engine-api/tests/selection_clear.rs new file mode 100644 index 0000000..2b53c7b --- /dev/null +++ b/hcie-engine-api/tests/selection_clear.rs @@ -0,0 +1,54 @@ +//! Selection pixel-clearing regression tests. +//! +//! **Purpose:** Verifies that destructive selection operations use the active +//! document selection mask rather than the brush stroke's temporary mask cache. +//! +//! **Logic & Workflow:** Each test creates an opaque raster layer through the +//! public engine API, installs a deterministic selection mask, clears selected +//! pixels, and compares alpha values inside and outside that mask. +//! +//! **Side Effects / Dependencies:** Uses only in-memory `hcie-engine-api` state; +//! no files, network services, GUI runtime, or engine internals are accessed. + +use hcie_engine_api::Engine; + +/// Confirms that selection clearing removes internal pixels and preserves all +/// pixels external to the selection. +/// +/// **Purpose:** Prevents a regression where `clear_selection_pixels` consulted +/// an empty stroke cache and consequently cleared the complete active layer. +/// +/// **Logic & Workflow:** Creates a 3×3 opaque layer, selects only its center +/// pixel, invokes selection clearing, then checks every resulting alpha value. +/// +/// **Arguments & Returns:** Takes no arguments and returns nothing; assertion +/// failures report whether an internal or external pixel was handled wrongly. +/// +/// **Side Effects / Dependencies:** Mutates only the local in-memory engine and +/// records the normal undo snapshot generated by the clear operation. +#[test] +fn clear_selection_pixels_clears_inside_and_preserves_outside() { + let mut engine = Engine::new(3, 3); + engine.set_active_layer_pixels(vec![255; 3 * 3 * 4]); + + let mut mask = vec![0; 3 * 3]; + mask[4] = 255; + engine.set_selection_mask(mask); + + engine.clear_selection_pixels(); + + let pixels = engine + .get_active_layer_pixels() + .expect("a new engine must have an active raster layer"); + for pixel_index in 0..9 { + let alpha = pixels[pixel_index * 4 + 3]; + if pixel_index == 4 { + assert_eq!(alpha, 0, "selected center pixel must be cleared"); + } else { + assert_eq!( + alpha, 255, + "unselected pixel {pixel_index} must be preserved" + ); + } + } +} diff --git a/hcie-iced-app/crates/hcie-iced-gui/src/app.rs b/hcie-iced-app/crates/hcie-iced-gui/src/app.rs index 187254a..a123c44 100644 --- a/hcie-iced-app/crates/hcie-iced-gui/src/app.rs +++ b/hcie-iced-app/crates/hcie-iced-gui/src/app.rs @@ -12,6 +12,8 @@ //! These sections are optimized for 4K performance. Modifying them //! may cause rendering regressions or performance degradation. +mod transform_dirty; + use crate::ai_chat::{self, AiChatState, ChatMessage, SYSTEM_PROMPT}; use crate::dialogs; use crate::dock::manager::DockDragState; @@ -22,6 +24,7 @@ use crate::panels; use crate::selection; use crate::selection::{CropState, SelectionTransform, TransformHandle}; use crate::theme::ThemeState; +use self::transform_dirty::preview_bounds; use hcie_engine_api::{ BlendMode, BrushStyle, BrushTip, Engine, FilterType, LayerStyle, Tool, ZOOM_MAX, ZOOM_MIN, }; @@ -382,6 +385,12 @@ pub struct IcedDocument { pub transform_original: Option, /// Source pixels and placement removed when a move transform began, for exact Cancel restore. pub transform_source: Option, + /// Original active-layer pixels for the transform's single history transaction. + pub transform_layer_baseline: Option>, + /// Original engine selection mask restored when a transform is cancelled. + pub transform_selection_baseline: Option>, + /// Composite snapshot without the floating transform, used as the base for each preview frame. + pub transform_preview_base: Option>, /// Internal clipboard for copy/paste operations pub internal_clipboard: Option, /// Selection mask (alpha channel) for the current selection @@ -1053,91 +1062,173 @@ fn is_vector_shape_tool(tool: Tool) -> bool { ) } -/// Composite a floating selection transform's pixels back onto the active layer. +/// Clears mask-covered alpha values without creating an engine history entry. /// -/// The transform supports translation, uniform scaling (via `size / pixel-dims`), -/// and rotation (in radians). Each destination pixel is sampled from the -/// source transform buffer by inverting the affine transform. Source pixels -/// with zero alpha are skipped so transparent gaps do not overwrite the layer. +/// **Purpose:** Starts a move transaction while deferring history creation until Apply, avoiding +/// the engine's standalone `Clear Selection` snapshot used by the Delete command. /// -/// This mirrors egui's `apply_transform_to_layer` but adds rotation support -/// (egui only handled scale + translation). +/// **Logic & Workflow:** Visits one mask byte per canvas pixel and clears only the corresponding +/// layer alpha byte when coverage is non-zero, matching `Engine::clear_selection_pixels`. +/// +/// **Arguments:** `pixels` is active-layer RGBA data and `mask` is one-byte selection coverage. +/// **Returns:** Nothing. **Side Effects / Dependencies:** Mutates `pixels`; performs no allocation, +/// engine call, or history operation. +fn clear_masked_pixels_without_history(pixels: &mut [u8], mask: &[u8]) { + for (index, coverage) in mask.iter().copied().enumerate() { + let alpha_index = index * 4 + 3; + if coverage > 0 && alpha_index < pixels.len() { + pixels[alpha_index] = 0; + } + } +} + +/// Restores a rectangular region from a stable transform preview baseline. +/// +/// **Purpose:** Removes the prior floating preview without restoring the full canvas per pointer +/// event. **Logic & Workflow:** Copies each row in the clipped exclusive rectangle from `source` +/// to `destination`. **Arguments:** Buffers are full-canvas RGBA data, `canvas_width` is their row +/// stride, and `bounds` is `[x0, y0, x1, y1]`. **Returns:** Nothing. +/// **Side Effects / Dependencies:** Mutates only `destination` rows inside `bounds`; no allocation. +fn restore_preview_region( + destination: &mut [u8], + source: &[u8], + canvas_width: u32, + bounds: [u32; 4], +) { + let row_bytes = (bounds[2] - bounds[0]) as usize * 4; + for y in bounds[1]..bounds[3] { + let start = (y as usize * canvas_width as usize + bounds[0] as usize) * 4; + let end = start + row_bytes; + if end <= destination.len() && end <= source.len() { + destination[start..end].copy_from_slice(&source[start..end]); + } + } +} + +/// Applies a floating transform and records exactly one caller-labelled layer snapshot. +/// +/// **Purpose:** Commits move and paste transforms atomically using the same inverse-affine sampler +/// as preview. **Logic & Workflow:** Reads the current non-preview layer, composites the transform, +/// installs final pixels, and pushes the supplied original baseline only when pixels changed. +/// +/// **Arguments:** `engine` is the API boundary, `tr` is floating geometry, canvas dimensions define +/// raster clipping, `before_pixels` is the transaction baseline, and `history_name` labels undo. +/// **Returns:** Nothing. **Side Effects / Dependencies:** Mutates the active layer and may append one +/// engine history snapshot; emits DEBUG timing and transaction-boundary logs. fn apply_transform_to_layer( engine: &mut hcie_engine_api::Engine, tr: &selection::SelectionTransform, canvas_w: u32, canvas_h: u32, + before_pixels: Vec, + history_name: &str, ) { + let started = std::time::Instant::now(); if tr.is_empty() || canvas_w == 0 || canvas_h == 0 { return; } let Some(mut dest) = engine.get_active_layer_pixels() else { return; }; - - let src_w = tr.width as f32; - let src_h = tr.height as f32; - let scale_x = if src_w > 0.0 { tr.size.x / src_w } else { 1.0 }; - let scale_y = if src_h > 0.0 { tr.size.y / src_h } else { 1.0 }; - let px = tr.pos.x.round() as f32; - let py = tr.pos.y.round() as f32; - - // Destination footprint in canvas pixels. - let dst_w = tr.size.x.round().max(1.0) as i32; - let dst_h = tr.size.y.round().max(1.0) as i32; - - let cos = tr.rotation.cos(); - let sin = tr.rotation.sin(); - let has_rotation = tr.rotation.abs() > 1e-4; - - // Center of the destination box in canvas space. - let dst_cx = px + dst_w as f32 / 2.0; - let dst_cy = py + dst_h as f32 / 2.0; - - for dy in 0..dst_h { - for dx in 0..dst_w { - let dst_x = px + dx as f32; - let dst_y = py + dy as f32; - if dst_x < 0.0 || dst_x >= canvas_w as f32 || dst_y < 0.0 || dst_y >= canvas_h as f32 { - continue; - } - - // Map destination pixel back into source texture space. - let (sx, sy) = if has_rotation { - // Vector from dst center, inverse-rotate, then map into src space. - let rel_x = dst_x - dst_cx; - let rel_y = dst_y - dst_cy; - let inv_x = rel_x * cos + rel_y * sin; - let inv_y = -rel_x * sin + rel_y * cos; - let center_src_x = src_w / 2.0; - let center_src_y = src_h / 2.0; - ( - (inv_x / scale_x + center_src_x), - (inv_y / scale_y + center_src_y), - ) - } else { - let nx = dx as f32 / scale_x.max(1e-6); - let ny = dy as f32 / scale_y.max(1e-6); - (nx, ny) - }; - - let sxi = sx.round() as i32; - let syi = sy.round() as i32; - if sxi < 0 || sxi >= tr.width as i32 || syi < 0 || syi >= tr.height as i32 { - continue; - } - let src_idx = (syi as u32 * tr.width + sxi as u32) as usize * 4; - if src_idx + 3 >= tr.pixels.len() || tr.pixels[src_idx + 3] == 0 { - continue; - } - let dst_idx = (dst_y as u32 * canvas_w + dst_x as u32) as usize * 4; - if dst_idx + 3 < dest.len() { - dest[dst_idx..dst_idx + 4].copy_from_slice(&tr.pixels[src_idx..src_idx + 4]); - } - } + let active_idx = engine.active_layer_index(); + let bounds = selection::transform::composite_transform(&mut dest, canvas_w, canvas_h, tr); + log::debug!( + "[TransformApply] begin history_name={history_name:?}, active_layer={active_idx}, bounds={bounds:?}" + ); + if before_pixels != dest { + engine.set_active_layer_pixels(dest); + engine.push_active_layer_snapshot(before_pixels, active_idx, history_name); } + log::debug!( + "[TransformApply] end history_name={history_name:?}, elapsed_us={}", + started.elapsed().as_micros() + ); +} - engine.set_active_layer_pixels(dest); +#[cfg(test)] +mod transform_transaction_tests { + use super::apply_transform_to_layer; + use crate::selection::SelectionTransform; + + /// Verifies that move commit creates one correctly named history boundary. + /// + /// **Purpose:** Prevents reintroduction of a preliminary `Clear Selection` snapshot before the + /// final move. **Logic & Workflow:** Starts from a transparent layer, applies one floating pixel + /// with an explicit original baseline, and checks history growth and description. + /// **Arguments & Returns:** No arguments or return value; assertions report transaction errors. + /// **Side Effects / Dependencies:** Uses an in-memory engine only and performs no I/O. + #[test] + fn move_commit_pushes_one_named_snapshot() { + let mut engine = hcie_engine_api::Engine::new(4, 4); + let baseline = engine.get_active_layer_pixels().unwrap(); + let history_before = engine.history_len(); + let transform = SelectionTransform { + pixels: vec![255, 0, 0, 255], + width: 1, + height: 1, + pos: iced::Vector::new(2.0, 1.0), + size: iced::Vector::new(1.0, 1.0), + rotation: 0.0, + }; + + apply_transform_to_layer(&mut engine, &transform, 4, 4, baseline, "Move Selection"); + + assert_eq!(engine.history_len(), history_before + 1); + assert_eq!( + engine.history_description(history_before).as_deref(), + Some("Move Selection") + ); + } +} + +/// Renders the current floating selection and publishes only its old/new union as dirty. +/// +/// **Purpose:** Keeps interactive transforms responsive while retaining the protected RefCell +/// dirty-region accumulator and shader partial-upload path. +/// +/// **Logic & Workflow:** Restores only the previous transformed AABB from the stable base, invokes +/// the shared inverse-affine compositor for the current AABB, unions both with any pending dirty +/// region, and updates the shader Arc in place when uniquely owned. A full-buffer Arc clone is used +/// only when Iced still owns the prior frame's immutable Arc. +/// +/// **Arguments:** `doc` contains transform/composite state and `previous_bounds` identifies stale +/// preview pixels. **Returns:** Nothing. **Side Effects / Dependencies:** Mutates preview buffers, +/// the dirty-region RefCell, and render generation; logs bounds and elapsed time at DEBUG level. +fn render_transform_preview(doc: &mut IcedDocument, previous_bounds: Option<[u32; 4]>) { + let started = std::time::Instant::now(); + let canvas_width = doc.engine.canvas_width(); + let canvas_height = doc.engine.canvas_height(); + if let (Some(base), Some(bounds)) = (doc.transform_preview_base.as_ref(), previous_bounds) { + restore_preview_region(&mut doc.composite_raw, base, canvas_width, bounds); + } + let current_bounds = doc.selection_transform.as_ref().and_then(|transform| { + selection::transform::composite_transform( + &mut doc.composite_raw, + canvas_width, + canvas_height, + transform, + ) + }); + let changed_bounds = match (previous_bounds, current_bounds) { + (Some(previous), Some(current)) => Some(union_regions(Some(previous), current)), + (Some(bounds), None) | (None, Some(bounds)) => Some(bounds), + (None, None) => None, + }; + if let Some(bounds) = changed_bounds { + let pending = doc.dirty_region.replace(None); + let upload_bounds = union_regions(pending, bounds); + doc.dirty_region.replace(Some(upload_bounds)); + if let Some(shader_pixels) = Arc::get_mut(&mut doc.composite_pixels) { + restore_preview_region(shader_pixels, &doc.composite_raw, canvas_width, upload_bounds); + } else { + doc.composite_pixels = Arc::new(doc.composite_raw.clone()); + } + doc.render_generation = doc.render_generation.wrapping_add(1); + log::debug!( + "[TransformPreview] previous={previous_bounds:?}, current={current_bounds:?}, upload={upload_bounds:?}, elapsed_us={}", + started.elapsed().as_micros() + ); + } } impl HcieIcedApp { @@ -1206,6 +1297,9 @@ impl HcieIcedApp { transform_drag_start: None, transform_original: None, transform_source: None, + transform_layer_baseline: None, + transform_selection_baseline: None, + transform_preview_base: None, internal_clipboard: None, selection_mask: None, selection_mask_dirty: std::cell::Cell::new(false), @@ -1242,8 +1336,9 @@ impl HcieIcedApp { last_cursor_pos: None, canvas_cursor_pos: None, active_dialog: ActiveDialog::None, - active_tool_slot: 0, + active_tool_slot: crate::sidebar::slot_for_tool(hcie_engine_api::Tool::Brush).unwrap_or(0), sub_tools_open: false, + sidebar_expanded: settings.panel_layout.sidebar_expanded, slot_last_used: vec![0; 20], dialog_input: String::new(), dialog_input2: String::new(), @@ -1331,7 +1426,6 @@ impl HcieIcedApp { vector_drag_session: None, bool_shape_a: None, bool_shape_b: None, - sidebar_expanded: settings.panel_layout.sidebar_expanded, }; if let Some(panel) = app @@ -2267,30 +2361,34 @@ impl HcieIcedApp { h, ); if !tr.is_empty() { - // Clear pixels within the selection area (not the - // whole layer), then drop the selection mask. - if let Some(mut layer_pixels) = - doc.engine.get_active_layer_pixels() - { - let cw = w as usize; - for y in 0..h as usize { - for x in 0..cw { - let midx = y * cw + x; - if midx < mask_data.len() && mask_data[midx] > 0 { - let pidx = midx * 4; - if pidx + 3 < layer_pixels.len() { - layer_pixels[pidx..pidx + 4].fill(0); - } - } - } - } - doc.engine.set_active_layer_pixels(layer_pixels); - } + let Some(original_layer) = doc.engine.get_active_layer_pixels() + else { + return Task::none(); + }; + let mut cut_layer = original_layer.clone(); + clear_masked_pixels_without_history(&mut cut_layer, &mask_data); + log::debug!( + "[TransformMove] begin atomic transaction bounds={:?}, selected_pixels={}", + preview_bounds(&tr, w, h), + mask_data.iter().filter(|coverage| **coverage > 0).count() + ); + doc.engine.set_active_layer_pixels(cut_layer); doc.engine.selection_clear(); - doc.selection_mask = Some(mask_data); + doc.selection_mask = Some(mask_data.clone()); doc.selection_mask_dirty.set(true); doc.transform_source = Some(tr.clone()); + doc.transform_layer_baseline = Some(original_layer); + doc.transform_selection_baseline = Some(mask_data); doc.selection_transform = Some(tr); + doc.transform_drag_handle = TransformHandle::Move; + doc.transform_drag_start = Some((canvas_x, canvas_y)); + // Force a composite refresh so the cleared source + // area is visible immediately. + doc.engine.mark_composite_dirty(); + self.refresh_composite_if_needed(); + let doc = &mut self.documents[self.active_doc]; + doc.transform_preview_base = Some(doc.composite_raw.clone()); + render_transform_preview(doc, None); } } } @@ -2964,10 +3062,6 @@ impl HcieIcedApp { // - Style exists → clone and toggle enabled flag let new_style = match existing { None => { - println!( - "LayerStyleToggle: Style {} not found, creating new", - disc - ); match disc { "DropShadow" => LayerStyle::DropShadow { enabled: true, @@ -3078,19 +3172,11 @@ impl HcieIcedApp { | LayerStyle::PatternOverlay { enabled, .. } | LayerStyle::Stroke { enabled, .. } => { *enabled = !*enabled; - println!( - "LayerStyleToggle: Style {} found, toggling to {}", - disc, *enabled - ); } } cloned } }; - println!( - "LayerStyleToggle: Calling update_layer_style with new_style: {:?}", - new_style - ); self.documents[self.active_doc] .engine .update_layer_style(layer_id, new_style); @@ -4361,7 +4447,7 @@ impl HcieIcedApp { .filter_map(|i| engine.history_description(i).map(|d| (i, d))) .collect(); let history_current = engine.history_current(); - self.documents.push(IcedDocument { + let doc = IcedDocument { engine, composite_pixels: Arc::new(composite_raw.clone()), composite_raw, @@ -4384,12 +4470,15 @@ impl HcieIcedApp { transform_drag_start: None, transform_original: None, transform_source: None, + transform_layer_baseline: None, + transform_selection_baseline: None, + transform_preview_base: None, internal_clipboard: None, selection_mask: None, selection_mask_dirty: std::cell::Cell::new(false), selection_bounds: None, - selection_history: Vec::new(), - selection_history_marker: 0, + selection_history: Vec::new(), + selection_history_marker: 0, crop_state: CropState::default(), lasso_points: Vec::new(), polygon_points: Vec::new(), @@ -4405,8 +4494,17 @@ impl HcieIcedApp { selected_vector_shape: None, vector_cycle_index: 0, vector_edit_handle: hcie_engine_api::VectorEditHandle::None, - }); - self.active_doc = self.documents.len() - 1; + }; + if self.show_welcome && self.documents.len() == 1 { + // Replace the placeholder empty document created for the + // welcome screen instead of appending a second tab. + self.documents = vec![doc]; + self.active_doc = 0; + } else { + self.documents.push(doc); + self.active_doc = self.documents.len() - 1; + } + self.show_welcome = false; self.active_dialog = ActiveDialog::None; } @@ -4673,29 +4771,9 @@ impl HcieIcedApp { doc.quick_mask = !doc.quick_mask; } Message::ClearPixels => { - let mask = self.documents[self.active_doc] + self.documents[self.active_doc] .engine - .get_selection_mask() - .map(ToOwned::to_owned); - if let (Some(mask), Some(mut pixels)) = ( - mask, - self.documents[self.active_doc] - .engine - .get_active_layer_pixels(), - ) { - for (index, alpha) in mask.into_iter().enumerate() { - if alpha != 0 && index * 4 + 3 < pixels.len() { - pixels[index * 4..index * 4 + 4].fill(0); - } - } - self.documents[self.active_doc] - .engine - .set_active_layer_pixels(pixels); - } else { - self.documents[self.active_doc] - .engine - .clear_active_layer_pixels(); - } + .clear_selection_pixels(); self.documents[self.active_doc].modified = true; self.refresh_composite_if_needed(); } @@ -4711,6 +4789,12 @@ impl HcieIcedApp { } Message::TransformDragMove(x, y) => { let doc = &mut self.documents[self.active_doc]; + let canvas_w = doc.engine.canvas_width(); + let canvas_h = doc.engine.canvas_height(); + let previous_preview_bounds = doc + .selection_transform + .as_ref() + .and_then(|tr| preview_bounds(tr, canvas_w, canvas_h)); if let (Some(start), Some(ref mut tr)) = (doc.transform_drag_start, &mut doc.selection_transform) { @@ -4722,54 +4806,33 @@ impl HcieIcedApp { tr.pos = iced::Vector::new(tr.pos.x + dx, tr.pos.y + dy); doc.transform_drag_start = Some((x, y)); } - TransformHandle::TopLeft => { - tr.pos = iced::Vector::new(tr.pos.x + dx, tr.pos.y + dy); - tr.size = iced::Vector::new(tr.size.x - dx, tr.size.y - dy); - doc.transform_drag_start = Some((x, y)); - } - TransformHandle::TopRight => { - tr.size = iced::Vector::new(tr.size.x + dx, tr.size.y - dy); - tr.pos = iced::Vector::new(tr.pos.x, tr.pos.y + dy); - doc.transform_drag_start = Some((x, y)); - } - TransformHandle::BottomLeft => { - tr.size = iced::Vector::new(tr.size.x - dx, tr.size.y + dy); - tr.pos = iced::Vector::new(tr.pos.x + dx, tr.pos.y); - doc.transform_drag_start = Some((x, y)); - } - TransformHandle::BottomRight => { - tr.size = iced::Vector::new(tr.size.x + dx, tr.size.y + dy); - doc.transform_drag_start = Some((x, y)); - } - TransformHandle::Top => { - tr.pos = iced::Vector::new(tr.pos.x, tr.pos.y + dy); - tr.size = iced::Vector::new(tr.size.x, tr.size.y - dy); - doc.transform_drag_start = Some((x, y)); - } - TransformHandle::Bottom => { - tr.size = iced::Vector::new(tr.size.x, tr.size.y + dy); - doc.transform_drag_start = Some((x, y)); - } - TransformHandle::Left => { - tr.pos = iced::Vector::new(tr.pos.x + dx, tr.pos.y); - tr.size = iced::Vector::new(tr.size.x - dx, tr.size.y); - doc.transform_drag_start = Some((x, y)); - } - TransformHandle::Right => { - tr.size = iced::Vector::new(tr.size.x + dx, tr.size.y); + TransformHandle::TopLeft + | TransformHandle::TopRight + | TransformHandle::BottomLeft + | TransformHandle::BottomRight + | TransformHandle::Top + | TransformHandle::Bottom + | TransformHandle::Left + | TransformHandle::Right => { + tr.resize_from_delta(doc.transform_drag_handle, dx, dy); doc.transform_drag_start = Some((x, y)); } TransformHandle::Rotate => { - // Calculate rotation from center - let center_x = tr.pos.x + tr.size.x / 2.0; - let center_y = tr.pos.y + tr.size.y / 2.0; - let angle = (y - center_y).atan2(x - center_x); - tr.rotation = angle; + if let Some(original) = doc.transform_original.as_ref() { + tr.rotation = selection::transform::rotation_from_drag( + original, + start, + (x, y), + ); + } } TransformHandle::None => {} } tr.sanitize_geometry(self.modifiers.shift()); } + let doc = &mut self.documents[self.active_doc]; + render_transform_preview(doc, previous_preview_bounds); + return Task::perform(async {}, |_| Message::CompositeRefresh); } Message::TransformDragEnd => { let doc = &mut self.documents[self.active_doc]; @@ -4779,39 +4842,70 @@ impl HcieIcedApp { Message::TransformApply => { let tr_taken = self.documents[self.active_doc].selection_transform.take(); if let Some(tr) = tr_taken { - let canvas_w = self.documents[self.active_doc].engine.canvas_width(); - let canvas_h = self.documents[self.active_doc].engine.canvas_height(); + let doc = &mut self.documents[self.active_doc]; + let canvas_w = doc.engine.canvas_width(); + let canvas_h = doc.engine.canvas_height(); + let was_move = doc.transform_source.is_some(); + let before_pixels = doc + .transform_layer_baseline + .take() + .or_else(|| doc.engine.get_active_layer_pixels()); + let Some(before_pixels) = before_pixels else { + return Task::none(); + }; apply_transform_to_layer( - &mut self.documents[self.active_doc].engine, + &mut doc.engine, &tr, canvas_w, canvas_h, + before_pixels, + if was_move { + "Move Selection" + } else { + "Transform Apply" + }, ); - self.documents[self.active_doc].engine.selection_clear(); - self.documents[self.active_doc].selection_bounds = None; - self.documents[self.active_doc].selection_mask = None; - self.documents[self.active_doc].selection_mask_dirty.set(true); - self.documents[self.active_doc].transform_drag_handle = TransformHandle::None; - self.documents[self.active_doc].transform_drag_start = None; - self.documents[self.active_doc].transform_original = None; - self.documents[self.active_doc].transform_source = None; - self.documents[self.active_doc].modified = true; + doc.engine.selection_clear(); + doc.selection_bounds = None; + doc.selection_mask = None; + doc.selection_mask_dirty.set(true); + doc.transform_drag_handle = TransformHandle::None; + doc.transform_drag_start = None; + doc.transform_original = None; + doc.transform_source = None; + doc.transform_selection_baseline = None; + doc.transform_preview_base = None; + doc.modified = true; self.refresh_composite_if_needed(); return Task::perform(async {}, |_| Message::CompositeRefresh); } } Message::TransformCancel => { let doc = &mut self.documents[self.active_doc]; - if let Some(source) = doc.transform_source.take() { - let width = doc.engine.canvas_width(); - let height = doc.engine.canvas_height(); - apply_transform_to_layer(&mut doc.engine, &source, width, height); + log::debug!( + "[TransformCancel] begin restore_layer={}, restore_selection={}", + doc.transform_source.is_some() && doc.transform_layer_baseline.is_some(), + doc.transform_selection_baseline.is_some() + ); + let original_layer = doc.transform_layer_baseline.take(); + if doc.transform_source.is_some() { + if let Some(original_layer) = original_layer { + doc.engine.set_active_layer_pixels(original_layer); + } + } + if let Some(mask) = doc.transform_selection_baseline.take() { + doc.engine.set_selection_mask(mask.clone()); + doc.selection_mask = Some(mask); + doc.selection_mask_dirty.set(true); } doc.selection_transform = None; + doc.transform_source = None; doc.transform_original = None; doc.transform_drag_start = None; doc.transform_drag_handle = TransformHandle::None; + doc.transform_preview_base = None; self.refresh_composite_if_needed(); + log::debug!("[TransformCancel] end without history snapshot"); } // ── Vector tools ────────────────────────────── @@ -6204,30 +6298,31 @@ impl HcieIcedApp { transform.height, ); doc.internal_clipboard = Some(transform); - if let Some(mut pixels) = doc.engine.get_active_layer_pixels() { - for (index, alpha) in mask.into_iter().enumerate() { - if alpha != 0 && index * 4 + 3 < pixels.len() { - pixels[index * 4..index * 4 + 4].fill(0); - } - } - doc.engine.set_active_layer_pixels(pixels); - doc.modified = true; - } + // Record an undoable snapshot of the source pixels before + // clearing them, then clear the selected region via the engine. + doc.engine.clear_selection_pixels(); + doc.modified = true; } } self.refresh_composite_if_needed(); } Message::PasteImage => { - let doc = &self.documents[self.active_doc]; - // Check internal clipboard first - if let Some(ref transform) = doc.internal_clipboard { + if let Some(transform) = self.documents[self.active_doc].internal_clipboard.clone() + { // Enter transform mode with the clipboard content - let mut placed = transform.clone(); + let mut placed = transform; placed.pos.x += 10.0; placed.pos.y += 10.0; - self.documents[self.active_doc].selection_transform = Some(placed); - self.documents[self.active_doc].transform_drag_handle = TransformHandle::None; + let doc = &mut self.documents[self.active_doc]; + doc.transform_layer_baseline = doc.engine.get_active_layer_pixels(); + doc.transform_selection_baseline = + doc.engine.get_selection_mask().map(ToOwned::to_owned); + doc.transform_source = None; + doc.transform_preview_base = Some(doc.composite_raw.clone()); + doc.selection_transform = Some(placed); + doc.transform_drag_handle = TransformHandle::None; + render_transform_preview(doc, None); return Task::perform(async {}, |_| Message::CompositeRefresh); } @@ -6270,10 +6365,17 @@ impl HcieIcedApp { } Message::ImagePasted(Ok(Some((pixels, w, h)))) => { log::info!("Pasted image: {}x{}", w, h); - let canvas_w = self.documents[self.active_doc].engine.canvas_width(); - let canvas_h = self.documents[self.active_doc].engine.canvas_height(); - self.documents[self.active_doc].selection_transform = + let doc = &mut self.documents[self.active_doc]; + let canvas_w = doc.engine.canvas_width(); + let canvas_h = doc.engine.canvas_height(); + doc.transform_layer_baseline = doc.engine.get_active_layer_pixels(); + doc.transform_selection_baseline = + doc.engine.get_selection_mask().map(ToOwned::to_owned); + doc.transform_source = None; + doc.transform_preview_base = Some(doc.composite_raw.clone()); + doc.selection_transform = selection::clipboard::centered_transform(pixels, w, h, canvas_w, canvas_h); + render_transform_preview(doc, None); } Message::ImagePasted(Ok(None)) => { log::info!("No image in clipboard"); @@ -6761,6 +6863,9 @@ impl HcieIcedApp { transform_drag_start: None, transform_original: None, transform_source: None, + transform_layer_baseline: None, + transform_selection_baseline: None, + transform_preview_base: None, internal_clipboard: None, selection_mask: None, selection_mask_dirty: std::cell::Cell::new(false), @@ -7446,6 +7551,7 @@ impl HcieIcedApp { ); // Toolbox — 36px strip (single column) or 72px (2 columns) on the left edge + log::info!("[view] sidebar_expanded={}", self.sidebar_expanded); let toolbox = crate::sidebar::view( &self.tool_state, &self.fg_color, diff --git a/hcie-iced-app/crates/hcie-iced-gui/src/app/transform_dirty.rs b/hcie-iced-app/crates/hcie-iced-gui/src/app/transform_dirty.rs new file mode 100644 index 0000000..a0e61ce --- /dev/null +++ b/hcie-iced-app/crates/hcie-iced-gui/src/app/transform_dirty.rs @@ -0,0 +1,86 @@ +//! Selection-transform preview invalidation utilities. +//! +//! **Purpose:** Computes canvas-clipped dirty bounds for floating selection +//! previews so both their previous and current positions can be uploaded to the +//! persistent GPU texture during interactive movement. +//! +//! **Logic & Workflow:** Preview geometry is rounded exactly like the CPU +//! compositor, clamped to the canvas origin, and clipped at the right and +//! bottom canvas edges. Empty or fully out-of-canvas rectangles return `None`. +//! +//! **Side Effects / Dependencies:** The module is deterministic and has no I/O, +//! global state, allocation, engine dependency, or GPU dependency. + +/// Calculates the clipped pixel bounds occupied by a transform preview. +/// +/// **Purpose:** Produces the dirty rectangle that must be restored or redrawn +/// when a floating selection changes position or dimensions. +/// +/// **Logic & Workflow:** Delegates to the shared rotated-corner AABB calculation +/// used by both preview and apply, clips it to the canvas, and rejects empty rectangles. +/// +/// **Arguments:** +/// - `transform`: Floating selection source and affine geometry. +/// - `canvas_width`, `canvas_height`: Clipping dimensions of the document. +/// +/// **Returns:** `Some([x0, y0, x1, y1])` for a non-empty clipped rectangle, or +/// `None` when the preview cannot affect the canvas. +/// +/// **Side Effects / Dependencies:** None. +pub(crate) fn preview_bounds( + transform: &crate::selection::SelectionTransform, + canvas_width: u32, + canvas_height: u32, +) -> Option<[u32; 4]> { + crate::selection::transform::transformed_bounds(transform, canvas_width, canvas_height) +} + +#[cfg(test)] +mod tests { + use crate::selection::SelectionTransform; + use iced::Vector; + + use super::preview_bounds; + + fn transform(x: f32, y: f32) -> SelectionTransform { + SelectionTransform { + pixels: vec![255; 30 * 40 * 4], + width: 30, + height: 40, + pos: Vector::new(x, y), + size: Vector::new(30.0, 40.0), + rotation: 0.0, + } + } + + /// Verifies that consecutive preview positions expose both the old and new + /// rectangles needed by the caller's dirty-region union. + /// + /// **Purpose:** Guards against stale GPU pixels remaining at an old drag + /// position when only the current preview rectangle is uploaded. + /// + /// **Logic & Workflow:** Computes two partially overlapping positions and + /// confirms their bounds preserve the complete restoration/redraw extent. + /// + /// **Arguments & Returns:** Takes no arguments and returns nothing; + /// assertion failures identify incorrect transform bounds. + /// + /// **Side Effects / Dependencies:** None. + #[test] + fn consecutive_positions_retain_old_and_new_extents() { + let previous = preview_bounds(&transform(10.0, 20.0), 100, 100).unwrap(); + let current = preview_bounds(&transform(25.0, 30.0), 100, 100).unwrap(); + + assert_eq!(previous, [10, 20, 40, 60]); + assert_eq!(current, [25, 30, 55, 70]); + assert_eq!( + [ + previous[0].min(current[0]), + previous[1].min(current[1]), + previous[2].max(current[2]), + previous[3].max(current[3]), + ], + [10, 20, 55, 70], + ); + } +} diff --git a/hcie-iced-app/crates/hcie-iced-gui/src/canvas/mod.rs b/hcie-iced-app/crates/hcie-iced-gui/src/canvas/mod.rs index 0c1e9fa..e1cb666 100644 --- a/hcie-iced-app/crates/hcie-iced-gui/src/canvas/mod.rs +++ b/hcie-iced-app/crates/hcie-iced-gui/src/canvas/mod.rs @@ -167,7 +167,12 @@ impl OverlayProgram { } /// Test which transform handle is at the given viewport position. - pub fn hit_test_handle(&self, viewport_x: f32, viewport_y: f32) -> TransformHandle { + pub fn hit_test_handle( + &self, + viewport_x: f32, + viewport_y: f32, + pane_size: Size, + ) -> TransformHandle { let Some(ref tr) = self.selection_transform else { return TransformHandle::None; }; @@ -175,54 +180,13 @@ impl OverlayProgram { return TransformHandle::None; } - let (origin_x, origin_y, _, _) = self.canvas_origin(iced::Size::new(0.0, 0.0)); + let (origin_x, origin_y, _, _) = self.canvas_origin(pane_size); // Convert viewport to canvas coordinates let canvas_x = (viewport_x - origin_x) / self.zoom; let canvas_y = (viewport_y - origin_y) / self.zoom; - let handle_size = TransformHandle::HANDLE_SIZE / self.zoom; - let half = handle_size / 2.0; - - let x = tr.pos.x; - let y = tr.pos.y; - let w = tr.size.x; - let h = tr.size.y; - - // Check rotation handle first (it's outside the box) - let rotate_y = y - TransformHandle::ROTATE_OFFSET / self.zoom; - let rotate_x = x + w / 2.0; - let rotate_radius = 4.0 / self.zoom; - let dx = canvas_x - rotate_x; - let dy = canvas_y - rotate_y; - if dx * dx + dy * dy <= rotate_radius * rotate_radius { - return TransformHandle::Rotate; - } - - // Check corner and edge handles - let handles = [ - (TransformHandle::TopLeft, x, y), - (TransformHandle::TopRight, x + w, y), - (TransformHandle::BottomLeft, x, y + h), - (TransformHandle::BottomRight, x + w, y + h), - (TransformHandle::Top, x + w / 2.0, y), - (TransformHandle::Bottom, x + w / 2.0, y + h), - (TransformHandle::Left, x, y + h / 2.0), - (TransformHandle::Right, x + w, y + h / 2.0), - ]; - - for (handle, hx, hy) in &handles { - if (canvas_x - hx).abs() <= half && (canvas_y - hy).abs() <= half { - return *handle; - } - } - - // Check if inside the selection box (for move) - if canvas_x >= x && canvas_x <= x + w && canvas_y >= y && canvas_y <= y + h { - return TransformHandle::Move; - } - - TransformHandle::None + tr.hit_test(canvas_x, canvas_y, self.zoom) } /// Draw transform handles for selection. @@ -233,25 +197,17 @@ impl OverlayProgram { origin_x: f32, origin_y: f32, ) { - let handle_size = TransformHandle::HANDLE_SIZE / self.zoom.max(1.0); + let handle_size = TransformHandle::HANDLE_SIZE; let half = handle_size / 2.0; - // Calculate handle positions in viewport coordinates - let x = origin_x + tr.pos.x * self.zoom; - let y = origin_y + tr.pos.y * self.zoom; - let w = tr.size.x * self.zoom; - let h = tr.size.y * self.zoom; - - let handles = [ - (TransformHandle::TopLeft, x, y), - (TransformHandle::TopRight, x + w, y), - (TransformHandle::BottomLeft, x, y + h), - (TransformHandle::BottomRight, x + w, y + h), - (TransformHandle::Top, x + w / 2.0, y), - (TransformHandle::Bottom, x + w / 2.0, y + h), - (TransformHandle::Left, x, y + h / 2.0), - (TransformHandle::Right, x + w, y + h / 2.0), - ]; + let handles = TransformHandle::RESIZE_HANDLES.map(|handle| { + let (canvas_x, canvas_y) = tr.handle_position(handle, self.zoom); + ( + handle, + origin_x + canvas_x * self.zoom, + origin_y + canvas_y * self.zoom, + ) + }); let handle_color = iced::Color::from_rgb(1.0, 1.0, 1.0); let handle_stroke_color = iced::Color::from_rgb(0.0, 0.0, 0.0); @@ -268,29 +224,38 @@ impl OverlayProgram { &rect, Stroke { style: canvas::stroke::Style::Solid(handle_stroke_color), - width: 1.0 / self.zoom.max(1.0), + width: 1.0, ..Default::default() }, ); } // Draw rotation handle (circle above top center) - let rotate_y = y - TransformHandle::ROTATE_OFFSET / self.zoom.max(1.0); - let rotate_x = x + w / 2.0; - let rotate_radius = 4.0 / self.zoom.max(1.0); + let (rotate_canvas_x, rotate_canvas_y) = + tr.handle_position(TransformHandle::Rotate, self.zoom); + let rotate_y = origin_y + rotate_canvas_y * self.zoom; + let rotate_x = origin_x + rotate_canvas_x * self.zoom; + let rotate_radius = 4.0; let rotate_circle = Path::circle(Point::new(rotate_x, rotate_y), rotate_radius); frame.fill(&rotate_circle, handle_color); frame.stroke( &rotate_circle, Stroke { style: canvas::stroke::Style::Solid(handle_stroke_color), - width: 1.0 / self.zoom.max(1.0), + width: 1.0, ..Default::default() }, ); // Draw line from top center to rotation handle - let line = Path::line(Point::new(x + w / 2.0, y), Point::new(rotate_x, rotate_y)); + let (top_canvas_x, top_canvas_y) = tr.handle_position(TransformHandle::Top, self.zoom); + let line = Path::line( + Point::new( + origin_x + top_canvas_x * self.zoom, + origin_y + top_canvas_y * self.zoom, + ), + Point::new(rotate_x, rotate_y), + ); frame.stroke( &line, Stroke { @@ -1169,11 +1134,25 @@ impl canvas::Program for OverlayProgram { if let Some(ref tr) = self.selection_transform { if !tr.is_empty() { // Draw dotted bounding box - let x = origin_x + tr.pos.x * self.zoom; - let y = origin_y + tr.pos.y * self.zoom; - let w = tr.size.x * self.zoom; - let h = tr.size.y * self.zoom; - let bbox = Path::rectangle(Point::new(x, y), Size::new(w, h)); + let corners = [ + tr.handle_position(TransformHandle::TopLeft, self.zoom), + tr.handle_position(TransformHandle::TopRight, self.zoom), + tr.handle_position(TransformHandle::BottomRight, self.zoom), + tr.handle_position(TransformHandle::BottomLeft, self.zoom), + ]; + let bbox = Path::new(|builder| { + builder.move_to(Point::new( + origin_x + corners[0].0 * self.zoom, + origin_y + corners[0].1 * self.zoom, + )); + for corner in &corners[1..] { + builder.line_to(Point::new( + origin_x + corner.0 * self.zoom, + origin_y + corner.1 * self.zoom, + )); + } + builder.close(); + }); frame.stroke( &bbox, Stroke { @@ -1301,7 +1280,8 @@ impl canvas::Program for OverlayProgram { // Update hovered handle if self.selection_transform.is_some() { - state.hovered_handle = self.hit_test_handle(position.x, position.y); + state.hovered_handle = + self.hit_test_handle(local_pos.x, local_pos.y, bounds.size()); } else { state.hovered_handle = TransformHandle::None; } @@ -1349,6 +1329,23 @@ impl canvas::Program for OverlayProgram { if !cursor.is_over(bounds) { return mouse::Interaction::default(); } + match state.hovered_handle { + TransformHandle::Move => return mouse::Interaction::Grab, + TransformHandle::Top | TransformHandle::Bottom => { + return mouse::Interaction::ResizingVertically + } + TransformHandle::Left | TransformHandle::Right => { + return mouse::Interaction::ResizingHorizontally + } + TransformHandle::TopLeft | TransformHandle::BottomRight => { + return mouse::Interaction::ResizingDiagonallyDown + } + TransformHandle::TopRight | TransformHandle::BottomLeft => { + return mouse::Interaction::ResizingDiagonallyUp + } + TransformHandle::Rotate => return mouse::Interaction::Pointer, + TransformHandle::None => {} + } match state.hovered_vector_handle { H::Move => mouse::Interaction::Grab, H::Top | H::Bottom => mouse::Interaction::ResizingVertically, diff --git a/hcie-iced-app/crates/hcie-iced-gui/src/panels/menus.rs b/hcie-iced-app/crates/hcie-iced-gui/src/panels/menus.rs index ae7648c..cc6923e 100644 --- a/hcie-iced-app/crates/hcie-iced-gui/src/panels/menus.rs +++ b/hcie-iced-app/crates/hcie-iced-gui/src/panels/menus.rs @@ -10,12 +10,13 @@ //! - Hover: #b3d9ff (light blue) //! - Shortcuts: #666666 (gray, right-aligned) //! - Separators: 1px #cccccc lines -//! - Sub-menus: indicated by ">" arrow on the right +//! - Sub-menus: indicated by a readable `›` arrow in a stable right gutter use crate::app::Message; use crate::dock::state::{DockState, PaneType}; +use crate::panels::typography; use crate::theme::ThemeColors; -use iced::widget::{column, container, horizontal_rule, mouse_area, row, text}; +use iced::widget::{column, container, horizontal_rule, mouse_area, row, text, Space}; use iced::{Element, Length}; /// Identifies one implemented menu operation independently of menu position or label. @@ -157,6 +158,74 @@ impl MenuItem { } } +/// Builds one aligned dropdown or context-menu row with stable semantic gutters. +/// +/// **Purpose:** Gives every menu entry identical label alignment regardless of whether it is +/// checked, has a shortcut, or opens a submenu. +/// **Logic & Workflow:** The helper reserves fixed-width leading checkmark, trailing shortcut, +/// and trailing arrow columns around a fill-width label, then vertically centers all content in +/// the shared compact row height. Empty text and space widgets retain the unused column widths. +/// **Arguments:** `item` supplies owned display strings, `is_checked` selects the checkmark, and +/// `colors` supplies theme-aware foreground colors. +/// **Returns:** A message-compatible row element with no interaction of its own. +/// **Side Effects / Dependencies:** None; callers wrap the row in the existing command button. +fn menu_item_row( + item: &MenuItem, + is_checked: bool, + colors: ThemeColors, +) -> Element<'static, Message> { + let checkmark: Element<'static, Message> = if is_checked { + container( + text("✓") + .size(typography::MENU_LABEL) + .color(iced::Color::from_rgba(0.2, 0.6, 0.2, 1.0)), + ) + .width(typography::MENU_CHECK_GUTTER) + .align_x(iced::alignment::Horizontal::Center) + .into() + } else { + Space::with_width(typography::MENU_CHECK_GUTTER).into() + }; + + let shortcut = container( + text(item.shortcut.clone()) + .size(typography::MENU_SHORTCUT) + .color(colors.menu_shortcut), + ) + .width(typography::MENU_SHORTCUT_GUTTER) + .align_x(iced::alignment::Horizontal::Right); + + let submenu_arrow = container( + text(if item.has_submenu { + typography::MENU_SUBMENU_ARROW + } else { + "" + }) + .size(typography::MENU_SHORTCUT) + .color(colors.menu_shortcut), + ) + .width(typography::MENU_ARROW_GUTTER) + .align_x(iced::alignment::Horizontal::Right); + + row![ + checkmark, + text(item.label.clone()) + .size(typography::MENU_LABEL) + .color(if item.enabled { + colors.menu_text + } else { + colors.menu_shortcut + }) + .width(Length::Fill), + shortcut, + submenu_arrow, + ] + .spacing(0) + .align_y(iced::Alignment::Center) + .height(Length::Fill) + .into() +} + /// All menu definitions matching Photopea's menu structure exactly. #[derive(Clone)] struct MenuDef { @@ -539,19 +608,11 @@ pub fn dropdown_overlay( items.push( container(horizontal_rule(1)) .width(Length::Fill) - .padding([2, 8]) + .padding([2, typography::MENU_ROW_HORIZONTAL_PADDING]) .into(), ); } else { - let lbl = item.label.clone(); - let sht = item.shortcut.clone(); - let has_sub = item.has_submenu; let enabled = item.enabled; - let label_color = if enabled { - colors.menu_text - } else { - colors.menu_shortcut - }; // Check if this is a Window menu item that should show a checkmark let is_window_panel = menu_idx == 7 && item_idx <= 10; @@ -577,46 +638,7 @@ pub fn dropdown_overlay( false }; - // Photopea-style: label left, shortcut right in gray, submenu arrow ">" if needed - // Add checkmark prefix for open panels in Window menu - let label_row: Element<'static, Message> = { - let mut row_items = vec![]; - if is_checked { - row_items.push( - text("✓ ") - .size(13) - .color(iced::Color::from_rgba(0.2, 0.6, 0.2, 1.0)) - .into(), - ); - } - row_items.push( - text(lbl) - .size(13) - .font(iced::Font { - weight: iced::font::Weight::Semibold, - ..iced::Font::default() - }) - .color(label_color) - .width(Length::Fill) - .into(), - ); - if !sht.is_empty() { - row_items.push( - text(sht) - .size(12) - .font(iced::Font { - weight: iced::font::Weight::Medium, - ..iced::Font::default() - }) - .color(colors.menu_shortcut) - .into(), - ); - } - if has_sub { - row_items.push(text(">").size(12).color(colors.menu_shortcut).into()); - } - row(row_items).spacing(16).into() - }; + let label_row = menu_item_row(item, is_checked, colors); let c = colors; @@ -625,37 +647,46 @@ pub fn dropdown_overlay( .command .clone() .expect("enabled menu leaves must have commands"); - let btn = - iced::widget::button(container(label_row).width(Length::Fill).padding([6, 16])) - .on_press(Message::MenuCommand(command)) - .padding(0) - .style( - move |_theme: &iced::Theme, status: iced::widget::button::Status| { - match status { - // Photopea-style hover: light blue highlight - iced::widget::button::Status::Hovered => { - iced::widget::button::Style { - background: Some(iced::Background::Color(c.menu_hover)), - text_color: c.menu_text, - border: iced::Border::default(), - ..Default::default() - } - } - _ => iced::widget::button::Style { - background: Some(iced::Background::Color(c.menu_bg)), - text_color: c.menu_text, - border: iced::Border::default(), - ..Default::default() - }, - } + let btn = iced::widget::button( + container(label_row) + .width(Length::Fill) + .height(typography::MENU_ROW_HEIGHT) + .padding([ + typography::MENU_ROW_VERTICAL_PADDING, + typography::MENU_ROW_HORIZONTAL_PADDING, + ]), + ) + .on_press(Message::MenuCommand(command)) + .padding(0) + .style( + move |_theme: &iced::Theme, status: iced::widget::button::Status| { + match status { + // Photopea-style hover: light blue highlight + iced::widget::button::Status::Hovered => iced::widget::button::Style { + background: Some(iced::Background::Color(c.menu_hover)), + text_color: c.menu_text, + border: iced::Border::default(), + ..Default::default() }, - ); + _ => iced::widget::button::Style { + background: Some(iced::Background::Color(c.menu_bg)), + text_color: c.menu_text, + border: iced::Border::default(), + ..Default::default() + }, + } + }, + ); items.push(btn.into()); } else { // Photopea-style disabled: lighter text let disabled_item = container(label_row) .width(Length::Fill) - .padding([6, 16]) + .height(typography::MENU_ROW_HEIGHT) + .padding([ + typography::MENU_ROW_VERTICAL_PADDING, + typography::MENU_ROW_HORIZONTAL_PADDING, + ]) .style(move |_theme| iced::widget::container::Style { background: Some(iced::Background::Color(colors.menu_bg)), text_color: Some(colors.menu_shortcut), @@ -762,54 +793,39 @@ pub fn canvas_context_menu<'a>( fill_mode: iced::widget::rule::FillMode::Full, }), ) - .padding([4, 0]) + .padding([4, typography::MENU_ROW_HORIZONTAL_PADDING]) .into(), ); } else { - let label = text(item.label.clone()) - .size(13) - .font(iced::Font { - weight: iced::font::Weight::Semibold, - ..iced::Font::default() - }) - .style(move |_theme| iced::widget::text::Style { - color: Some(colors.menu_text), - }); - let mut label_row = row![label].align_y(iced::Alignment::Center); - - if !item.shortcut.is_empty() { - label_row = label_row.push(iced::widget::Space::with_width(Length::Fill)); - label_row = label_row.push(text(item.shortcut).size(12).style(move |_theme| { - iced::widget::text::Style { - color: Some(colors.menu_shortcut), - } - })); - } + let label_row = menu_item_row(&item, false, colors); let c = colors; - let menu_item = - iced::widget::button(container(label_row).width(Length::Fill).padding([7, 16])) - .padding(0) - .style( - move |_theme: &iced::Theme, status: iced::widget::button::Status| { - match status { - iced::widget::button::Status::Hovered => { - iced::widget::button::Style { - background: Some(iced::Background::Color(c.menu_hover)), - text_color: c.menu_text, - border: iced::Border::default(), - ..Default::default() - } - } - _ => iced::widget::button::Style { - background: Some(iced::Background::Color(c.menu_bg)), - text_color: c.menu_text, - border: iced::Border::default(), - ..Default::default() - }, - } - }, - ); + let menu_item = iced::widget::button( + container(label_row) + .width(Length::Fill) + .height(typography::MENU_ROW_HEIGHT) + .padding([ + typography::MENU_ROW_VERTICAL_PADDING, + typography::MENU_ROW_HORIZONTAL_PADDING, + ]), + ) + .padding(0) + .style( + move |_theme: &iced::Theme, status: iced::widget::button::Status| match status { + iced::widget::button::Status::Hovered => iced::widget::button::Style { + background: Some(iced::Background::Color(c.menu_hover)), + text_color: c.menu_text, + border: iced::Border::default(), + ..Default::default() + }, + _ => iced::widget::button::Style { + background: Some(iced::Background::Color(c.menu_bg)), + text_color: c.menu_text, + border: iced::Border::default(), + ..Default::default() + }, + }, + ); let menu_item = if let Some(command) = item.command { menu_item.on_press(Message::MenuCommand(command)) @@ -980,4 +996,16 @@ mod tests { assert!(cut.enabled); assert_eq!(cut.command, Some(MenuCommand::Cut)); } + + /// Guards the compact menu geometry and Unicode submenu marker used by both popup types. + #[test] + fn shared_menu_metrics_keep_rows_compact_and_gutters_stable() { + assert!((24.0..=26.0).contains(&typography::MENU_ROW_HEIGHT)); + assert!((10..=12).contains(&typography::MENU_ROW_HORIZONTAL_PADDING)); + assert!((4..=5).contains(&typography::MENU_ROW_VERTICAL_PADDING)); + assert!(typography::MENU_CHECK_GUTTER > 0.0); + assert!(typography::MENU_SHORTCUT_GUTTER > 0.0); + assert!(typography::MENU_ARROW_GUTTER > 0.0); + assert_eq!(typography::MENU_SUBMENU_ARROW, "›"); + } } diff --git a/hcie-iced-app/crates/hcie-iced-gui/src/panels/styles.rs b/hcie-iced-app/crates/hcie-iced-gui/src/panels/styles.rs index 904bd21..9a1c83d 100644 --- a/hcie-iced-app/crates/hcie-iced-gui/src/panels/styles.rs +++ b/hcie-iced-app/crates/hcie-iced-gui/src/panels/styles.rs @@ -11,6 +11,91 @@ use crate::theme::ThemeColors; +/// Distance between a hovered control and its balloon tooltip, in logical pixels. +/// +/// **Purpose:** Keeps the balloon visually detached from narrow toolbox and +/// toolbar controls instead of appearing attached to their outer edge. +/// +/// **Logic & Workflow:** Iced applies this value along the tooltip placement +/// axis after positioning the overlay beside the hovered control. +/// +/// **Side Effects / Dependencies:** Consumed only by Iced tooltip layout. +const TOOLTIP_CONTROL_GAP: u16 = 14; + +/// Builds the opaque elevated surface used by balloon-style tooltips. +/// +/// **Purpose:** Keeps tooltip text readable over the canvas and every panel by +/// drawing a fully opaque, theme-consistent surface with a rounded outline and +/// subtle elevation shadow. +/// +/// **Logic & Workflow:** Uses the active panel color as the tooltip fill, +/// selects the high-contrast theme border, rounds the outline, and adds a small +/// downward shadow to separate the balloon from the hovered control. +/// +/// **Arguments:** `colors` contains the active application theme tokens. +/// +/// **Returns:** An Iced 0.13 container style suitable for a tooltip overlay. +/// +/// **Side Effects / Dependencies:** None. The returned style is deterministic +/// and does not read or mutate application state. +pub fn tooltip_balloon_background(colors: ThemeColors) -> iced::widget::container::Style { + let mut background = colors.bg_panel; + background.a = 1.0; + + iced::widget::container::Style { + text_color: Some(colors.text_primary), + background: Some(iced::Background::Color(background)), + border: iced::Border::default() + .rounded(5) + .color(colors.border_high) + .width(1), + shadow: iced::Shadow { + color: iced::Color::from_rgba(0.0, 0.0, 0.0, 0.32), + offset: iced::Vector::new(0.0, 2.0), + blur_radius: 6.0, + }, + ..Default::default() + } +} + +/// Wraps an interactive control in a shared opaque balloon-style tooltip. +/// +/// **Purpose:** Applies identical tooltip typography, spacing, viewport +/// behavior, and visual treatment to toolbox and toolbar tool controls. +/// +/// **Logic & Workflow:** Converts the supplied control and owned label into +/// Iced elements, places the balloon at `position`, leaves a visible gap from +/// the hovered control, applies compact internal padding, and delegates the +/// overlay appearance to [`tooltip_balloon_background`]. +/// +/// **Arguments:** `content` is the hovered control; `label` is its tooltip +/// text; `position` selects the overlay side; and `colors` supplies active +/// theme tokens. +/// +/// **Returns:** An Iced 0.13 element that preserves the wrapped control's +/// messages and interaction behavior while displaying the styled tooltip. +/// +/// **Side Effects / Dependencies:** Rendering depends on Iced's tooltip +/// overlay system. Constructing the element performs no I/O or state mutation. +pub fn balloon_tooltip<'a, Message: 'a>( + content: impl Into>, + label: impl Into, + position: iced::widget::tooltip::Position, + colors: ThemeColors, +) -> iced::Element<'a, Message> { + iced::widget::tooltip( + content, + iced::widget::text(label.into()) + .size(11) + .color(colors.text_primary), + position, + ) + .gap(TOOLTIP_CONTROL_GAP) + .padding(7) + .style(move |_theme| tooltip_balloon_background(colors)) + .into() +} + /// Panel background — #2a2a2a (Photopea panel color). pub fn panel_background(colors: ThemeColors) -> iced::widget::container::Style { iced::widget::container::Style { @@ -175,3 +260,28 @@ pub fn modern_panel_header(colors: ThemeColors, focused: bool) -> iced::widget:: ..Default::default() } } + +#[cfg(test)] +mod tests { + use super::tooltip_balloon_background; + use crate::theme::{ThemeColors, ThemePreset}; + + /// Verifies tooltip surfaces remain opaque and readable in dark and light themes. + #[test] + fn tooltip_balloon_is_opaque_and_uses_theme_contrast_tokens() { + for preset in [ThemePreset::Photopea, ThemePreset::ProLight] { + let colors = ThemeColors::get(preset); + let style = tooltip_balloon_background(colors); + let background = match style.background.unwrap() { + iced::Background::Color(background) => background, + iced::Background::Gradient(_) => panic!("tooltip background must be a solid color"), + }; + + assert_eq!(background.a, 1.0); + assert_eq!(style.text_color, Some(colors.text_primary)); + assert_eq!(style.border.color, colors.border_high); + assert_eq!(style.border.width, 1.0); + assert!(style.shadow.blur_radius > 0.0); + } + } +} diff --git a/hcie-iced-app/crates/hcie-iced-gui/src/panels/title_bar.rs b/hcie-iced-app/crates/hcie-iced-gui/src/panels/title_bar.rs index d890523..1cbcb0f 100644 --- a/hcie-iced-app/crates/hcie-iced-gui/src/panels/title_bar.rs +++ b/hcie-iced-app/crates/hcie-iced-gui/src/panels/title_bar.rs @@ -9,6 +9,7 @@ //! - Window controls: standard minimize/maximize/close use crate::app::Message; +use crate::panels::typography; use crate::theme::ThemeColors; use iced::widget::{button, container, mouse_area, row, text}; use iced::{Element, Length}; @@ -21,11 +22,11 @@ pub(crate) const MENU_LABELS: &[&str] = &[ /// Horizontal title-menu metrics shared by button layout and dropdown placement. /// /// **Purpose:** Keeps menu buttons and dropdown anchors in one deterministic layout model. -/// **Logic & Workflow:** Widths include the measured 13px semibold label advance plus the -/// title button's 16px horizontal padding. [`menu_anchor_x`] sums these exact rendered widths. +/// **Logic & Workflow:** Widths include the measured 12px medium label advance plus the +/// title button's 20px horizontal padding. [`menu_anchor_x`] sums these exact rendered widths. /// **Side Effects / Dependencies:** Values depend on the title bar font and padding below. pub(crate) const MENU_BUTTON_WIDTHS: &[f32] = - &[42.0, 42.0, 52.0, 56.0, 60.0, 51.0, 47.0, 67.0, 50.0]; + &[46.0, 46.0, 56.0, 60.0, 64.0, 55.0, 51.0, 71.0, 54.0]; pub(crate) const MENU_LEFT_INSET: f32 = 4.0; /// Unified menu/title bar height shared with dropdown placement. pub(crate) const MENU_BAR_HEIGHT: f32 = 28.0; @@ -58,17 +59,21 @@ pub fn view<'a>( colors: ThemeColors, active_menu: Option, ) -> Element<'a, Message> { - // ── Menu items — Photopea style: 12px, tight padding ── + // ── Menu items — readable 12px medium labels with balanced padding ── let mut menu_items = row![].spacing(0); for (idx, &label) in MENU_LABELS.iter().enumerate() { let is_open = active_menu == Some(idx); let c = colors; - let btn = button(text(label).size(13).font(iced::Font { - weight: iced::font::Weight::Semibold, - ..iced::Font::default() - })) + let btn = button( + text(label) + .size(typography::MENU_BAR_LABEL) + .font(iced::Font { + weight: iced::font::Weight::Medium, + ..iced::Font::default() + }), + ) .on_press(Message::MenuOpen(idx)) - .padding([3, 8]) + .padding([3, 10]) .width(MENU_BUTTON_WIDTHS[idx]) .style( move |_theme: &iced::Theme, status: iced::widget::button::Status| { @@ -232,8 +237,8 @@ mod tests { #[test] fn menu_anchors_are_deterministic_across_viewport_widths() { assert_eq!(menu_anchor_x(0, 1280.0, 260.0), 4.0); - assert_eq!(menu_anchor_x(3, 1280.0, 260.0), 140.0); - assert_eq!(menu_anchor_x(8, 1280.0, 260.0), 421.0); + assert_eq!(menu_anchor_x(3, 1280.0, 260.0), 152.0); + assert_eq!(menu_anchor_x(8, 1280.0, 260.0), 453.0); assert_eq!(menu_anchor_x(8, 500.0, 260.0), 240.0); assert_eq!(menu_anchor_x(8, 180.0, 260.0), 0.0); } diff --git a/hcie-iced-app/crates/hcie-iced-gui/src/panels/toolbar.rs b/hcie-iced-app/crates/hcie-iced-gui/src/panels/toolbar.rs index 1d20fec..a8704c6 100644 --- a/hcie-iced-app/crates/hcie-iced-gui/src/panels/toolbar.rs +++ b/hcie-iced-app/crates/hcie-iced-gui/src/panels/toolbar.rs @@ -208,7 +208,7 @@ fn svg_btn( text("?").size(12).color(c.text_primary).into() }; - iced::widget::tooltip( + styles::balloon_tooltip( button(icon).on_press(msg).padding([2, 4]).style( move |_theme: &iced::Theme, status: iced::widget::button::Status| match status { iced::widget::button::Status::Hovered => iced::widget::button::Style { @@ -225,10 +225,10 @@ fn svg_btn( }, }, ), - text(tip_owned).size(11), + tip_owned, iced::widget::tooltip::Position::Bottom, + colors, ) - .into() } fn small_btn(label: &str, colors: ThemeColors) -> Element<'static, Message> { @@ -268,10 +268,10 @@ fn sep(colors: ThemeColors) -> Element<'static, Message> { /// Displays a compact toolbar value while moving its full meaning into a tooltip. fn compact_value(value: String, tip: &str, colors: ThemeColors) -> Element<'static, Message> { - iced::widget::tooltip( + styles::balloon_tooltip( container(text(value).size(10).color(colors.text_primary)).padding([1, 3]), - text(tip.to_string()).size(11), + tip, iced::widget::tooltip::Position::Bottom, + colors, ) - .into() } diff --git a/hcie-iced-app/crates/hcie-iced-gui/src/panels/typography.rs b/hcie-iced-app/crates/hcie-iced-gui/src/panels/typography.rs index a83d4c6..9278730 100644 --- a/hcie-iced-app/crates/hcie-iced-gui/src/panels/typography.rs +++ b/hcie-iced-app/crates/hcie-iced-gui/src/panels/typography.rs @@ -1,8 +1,9 @@ -//! Shared typography metrics for docked panels. +//! Shared typography and compact menu metrics for the Iced interface. //! -//! Purpose: Keep panel body, section, and dock-title text readable and consistent. -//! Logic & Workflow: Panel views use these constants instead of local pixel sizes so narrow -//! layouts can increase readability without diverging. Side Effects / Dependencies: None. +//! Purpose: Keep panel text and menu rows readable, aligned, and consistent. +//! Logic & Workflow: Panel and menu views use these constants instead of local pixel sizes; +//! fixed menu gutters keep labels stable when checkmarks, shortcuts, or submenus are absent. +//! Side Effects / Dependencies: None. /// Standard readable body text size used by panel labels and values. pub const BODY: u16 = 12; @@ -10,3 +11,24 @@ pub const BODY: u16 = 12; pub const SECTION: u16 = 13; /// Dock title size used by pane title bars. pub const TITLE: u16 = 13; + +/// Normal-sized text used by labels in the top application menu bar. +pub const MENU_BAR_LABEL: u16 = 12; +/// Normal-sized text used by dropdown and context-menu labels. +pub const MENU_LABEL: u16 = 13; +/// Slightly smaller normal text used by menu keyboard shortcuts. +pub const MENU_SHORTCUT: u16 = 12; +/// Compact row height that vertically centers menu text in a 24-26px target. +pub const MENU_ROW_HEIGHT: f32 = 26.0; +/// Horizontal inset applied to both sides of every menu row. +pub const MENU_ROW_HORIZONTAL_PADDING: u16 = 11; +/// Vertical inset applied above and below the contents of every menu row. +pub const MENU_ROW_VERTICAL_PADDING: u16 = 4; +/// Reserved leading column width for checked and unchecked menu entries. +pub const MENU_CHECK_GUTTER: f32 = 18.0; +/// Reserved trailing column width for present and absent keyboard shortcuts. +pub const MENU_SHORTCUT_GUTTER: f32 = 82.0; +/// Reserved trailing column width for present and absent submenu arrows. +pub const MENU_ARROW_GUTTER: f32 = 14.0; +/// Readable single-character marker used for submenu navigation. +pub const MENU_SUBMENU_ARROW: &str = "›"; diff --git a/hcie-iced-app/crates/hcie-iced-gui/src/selection/transform.rs b/hcie-iced-app/crates/hcie-iced-gui/src/selection/transform.rs index 876ca56..838bf3d 100644 --- a/hcie-iced-app/crates/hcie-iced-gui/src/selection/transform.rs +++ b/hcie-iced-app/crates/hcie-iced-gui/src/selection/transform.rs @@ -37,10 +37,11 @@ impl SelectionTransform { let mut max_x = 0u32; let mut max_y = 0u32; - // Find bounding box of non-zero mask pixels + // Find bounding box of non-zero mask pixels. + // The mask is one byte per pixel (w*h bytes), NOT RGBA (w*h*4 bytes). for y in 0..canvas_h { for x in 0..canvas_w { - let idx = ((y * canvas_w + x) * 4 + 3) as usize; + let idx = (y * canvas_w + x) as usize; if idx < mask.len() && mask[idx] > 0 { min_x = min_x.min(x); min_y = min_y.min(y); @@ -57,9 +58,9 @@ impl SelectionTransform { let width = max_x - min_x + 1; let height = max_y - min_y + 1; - // Extract RGBA pixels within the bounding box + // Extract RGBA pixels within the bounding box from the active layer. let mut pixels = vec![0u8; (width * height * 4) as usize]; - let comp = engine.get_composite_pixels(); + let layer_pixels = engine.get_active_layer_pixels().unwrap_or_default(); for y in 0..height { for x in 0..width { @@ -68,18 +69,20 @@ impl SelectionTransform { let src_idx = ((src_y * canvas_w + src_x) * 4) as usize; let dst_idx = ((y * width + x) * 4) as usize; - if src_idx + 3 < comp.len() && dst_idx + 3 < pixels.len() { - let mask_idx = ((src_y * canvas_w + src_x) * 4 + 3) as usize; + if src_idx + 3 < layer_pixels.len() && dst_idx + 3 < pixels.len() { + // Mask is one byte per pixel + let mask_idx = (src_y * canvas_w + src_x) as usize; let alpha = if mask_idx < mask.len() { mask[mask_idx] } else { 0 }; - pixels[dst_idx] = comp[src_idx]; - pixels[dst_idx + 1] = comp[src_idx + 1]; - pixels[dst_idx + 2] = comp[src_idx + 2]; - pixels[dst_idx + 3] = alpha; + pixels[dst_idx] = layer_pixels[src_idx]; + pixels[dst_idx + 1] = layer_pixels[src_idx + 1]; + pixels[dst_idx + 2] = layer_pixels[src_idx + 2]; + pixels[dst_idx + 3] = + ((layer_pixels[src_idx + 3] as u16 * alpha as u16) / 255) as u8; } } } @@ -117,46 +120,11 @@ impl SelectionTransform { /// **Returns:** The nearest resize/rotate/move handle or `None`. **Side Effects:** None. pub fn hit_test(&self, x: f32, y: f32, zoom: f32) -> TransformHandle { let half = TransformHandle::HANDLE_SIZE / zoom.max(0.01) * 0.5; - let points = [ - (TransformHandle::TopLeft, self.pos.x, self.pos.y), - ( - TransformHandle::TopRight, - self.pos.x + self.size.x, - self.pos.y, - ), - ( - TransformHandle::BottomLeft, - self.pos.x, - self.pos.y + self.size.y, - ), - ( - TransformHandle::BottomRight, - self.pos.x + self.size.x, - self.pos.y + self.size.y, - ), - ( - TransformHandle::Top, - self.pos.x + self.size.x * 0.5, - self.pos.y, - ), - ( - TransformHandle::Bottom, - self.pos.x + self.size.x * 0.5, - self.pos.y + self.size.y, - ), - ( - TransformHandle::Left, - self.pos.x, - self.pos.y + self.size.y * 0.5, - ), - ( - TransformHandle::Right, - self.pos.x + self.size.x, - self.pos.y + self.size.y * 0.5, - ), - ]; - let rotate_x = self.pos.x + self.size.x * 0.5; - let rotate_y = self.pos.y - TransformHandle::ROTATE_OFFSET / zoom.max(0.01); + let points = TransformHandle::RESIZE_HANDLES.map(|handle| { + let (hx, hy) = self.handle_position(handle, zoom); + (handle, hx, hy) + }); + let (rotate_x, rotate_y) = self.handle_position(TransformHandle::Rotate, zoom); if (x - rotate_x).hypot(y - rotate_y) <= half { return TransformHandle::Rotate; } @@ -165,10 +133,11 @@ impl SelectionTransform { return handle; } } - if x >= self.pos.x - && x <= self.pos.x + self.size.x - && y >= self.pos.y - && y <= self.pos.y + self.size.y + let (local_x, local_y) = self.inverse_rotate_point(x, y); + if local_x >= self.pos.x + && local_x <= self.pos.x + self.size.x + && local_y >= self.pos.y + && local_y <= self.pos.y + self.size.y { TransformHandle::Move } else { @@ -176,6 +145,131 @@ impl SelectionTransform { } } + /// Returns a transform handle's position after rotating it around the selection center. + /// + /// **Purpose:** Keeps painted handles, pointer hit-testing, and cursor routing on the same + /// geometry for translated, scaled, and rotated selections. + /// + /// **Logic & Workflow:** Resolves the handle in the unrotated box, applies the transform's + /// forward rotation around its center, and places the rotate handle at a screen-constant + /// offset above the top edge before rotation. + /// + /// **Arguments:** `handle` identifies the requested control and `zoom` converts the rotate + /// offset from screen pixels to canvas units. **Returns:** The canvas-space `(x, y)` point. + /// + /// **Side Effects / Dependencies:** None; the calculation uses only transform geometry. + pub fn handle_position(&self, handle: TransformHandle, zoom: f32) -> (f32, f32) { + let x = self.pos.x; + let y = self.pos.y; + let w = self.size.x; + let h = self.size.y; + let point = match handle { + TransformHandle::TopLeft => (x, y), + TransformHandle::TopRight => (x + w, y), + TransformHandle::BottomLeft => (x, y + h), + TransformHandle::BottomRight => (x + w, y + h), + TransformHandle::Top => (x + w * 0.5, y), + TransformHandle::Bottom => (x + w * 0.5, y + h), + TransformHandle::Left => (x, y + h * 0.5), + TransformHandle::Right => (x + w, y + h * 0.5), + TransformHandle::Rotate => ( + x + w * 0.5, + y - TransformHandle::ROTATE_OFFSET / zoom.max(0.01), + ), + TransformHandle::Move | TransformHandle::None => (x + w * 0.5, y + h * 0.5), + }; + self.rotate_point(point.0, point.1) + } + + /// Rotates a canvas point forward around this transform's center. + /// + /// **Purpose:** Supplies one forward-affine definition for handles and bounds. + /// **Logic & Workflow:** Translates to the center, applies the standard 2D rotation matrix, + /// then translates back. **Arguments:** `x` and `y` are canvas coordinates. **Returns:** The + /// rotated point. **Side Effects / Dependencies:** None. + pub fn rotate_point(&self, x: f32, y: f32) -> (f32, f32) { + let cx = self.pos.x + self.size.x * 0.5; + let cy = self.pos.y + self.size.y * 0.5; + let dx = x - cx; + let dy = y - cy; + let cos = self.rotation.cos(); + let sin = self.rotation.sin(); + (cx + dx * cos - dy * sin, cy + dx * sin + dy * cos) + } + + /// Inverse-rotates a canvas point into the transform's unrotated box space. + /// + /// **Purpose:** Allows move hit-testing and inverse-affine pixel sampling to agree. + /// **Logic & Workflow:** Applies the transpose of the forward rotation matrix around the + /// transform center. **Arguments:** `x` and `y` are canvas coordinates. **Returns:** The + /// corresponding unrotated point. **Side Effects / Dependencies:** None. + pub fn inverse_rotate_point(&self, x: f32, y: f32) -> (f32, f32) { + let cx = self.pos.x + self.size.x * 0.5; + let cy = self.pos.y + self.size.y * 0.5; + let dx = x - cx; + let dy = y - cy; + let cos = self.rotation.cos(); + let sin = self.rotation.sin(); + (cx + dx * cos + dy * sin, cy - dx * sin + dy * cos) + } + + /// Resizes a potentially rotated transform while anchoring the opposite edge or corner. + /// + /// **Purpose:** Makes drag deltas follow the visual handle axes after rotation instead of the + /// canvas axes. **Logic & Workflow:** Projects the canvas delta into local box space, moves the + /// selected local edges, clamps them to a one-pixel minimum, then rotates the local center shift + /// back into canvas space and rebuilds `pos`/`size` around the shifted center. + /// + /// **Arguments:** `handle` is a resize handle and `canvas_dx`/`canvas_dy` are incremental pointer + /// deltas. **Returns:** Nothing. **Side Effects / Dependencies:** Mutates geometry only; move, + /// rotate, and none handles are ignored. + pub fn resize_from_delta(&mut self, handle: TransformHandle, canvas_dx: f32, canvas_dy: f32) { + let cos = self.rotation.cos(); + let sin = self.rotation.sin(); + let local_dx = canvas_dx * cos + canvas_dy * sin; + let local_dy = -canvas_dx * sin + canvas_dy * cos; + let mut left = -self.size.x * 0.5; + let mut right = self.size.x * 0.5; + let mut top = -self.size.y * 0.5; + let mut bottom = self.size.y * 0.5; + if matches!( + handle, + TransformHandle::TopLeft | TransformHandle::BottomLeft | TransformHandle::Left + ) { + left = (left + local_dx).min(right - 1.0); + } + if matches!( + handle, + TransformHandle::TopRight | TransformHandle::BottomRight | TransformHandle::Right + ) { + right = (right + local_dx).max(left + 1.0); + } + if matches!( + handle, + TransformHandle::TopLeft | TransformHandle::TopRight | TransformHandle::Top + ) { + top = (top + local_dy).min(bottom - 1.0); + } + if matches!( + handle, + TransformHandle::BottomLeft | TransformHandle::BottomRight | TransformHandle::Bottom + ) { + bottom = (bottom + local_dy).max(top + 1.0); + } + if matches!( + handle, + TransformHandle::Move | TransformHandle::Rotate | TransformHandle::None + ) { + return; + } + let local_center_x = (left + right) * 0.5; + let local_center_y = (top + bottom) * 0.5; + let center_x = self.pos.x + self.size.x * 0.5 + local_center_x * cos - local_center_y * sin; + let center_y = self.pos.y + self.size.y * 0.5 + local_center_x * sin + local_center_y * cos; + self.size = Vector::new(right - left, bottom - top); + self.pos = Vector::new(center_x - self.size.x * 0.5, center_y - self.size.y * 0.5); + } + /// Enforces positive finite geometry and optionally preserves source aspect ratio. /// /// **Arguments:** `lock_aspect` applies the source pixel ratio when true. **Returns:** Nothing. @@ -232,6 +326,18 @@ impl TransformHandle { /// Size of rotation handle offset from top center. pub const ROTATE_OFFSET: f32 = 20.0; + /// Resize handles in hit-test priority order, with corners preceding edge handles. + pub const RESIZE_HANDLES: [Self; 8] = [ + Self::TopLeft, + Self::TopRight, + Self::BottomLeft, + Self::BottomRight, + Self::Top, + Self::Bottom, + Self::Left, + Self::Right, + ]; + /// Get cursor icon hint for this handle. pub fn cursor_hint(&self) -> &'static str { match self { @@ -245,3 +351,137 @@ impl TransformHandle { } } } + +/// Computes the canvas-clipped axis-aligned pixel footprint of a transformed selection. +/// +/// **Purpose:** Prevents rotated corners from being clipped and gives preview restoration and +/// partial GPU upload code the exact same destination extent as the rasterizer. +/// +/// **Logic & Workflow:** Rotates all four box corners, floors the minimum coordinates, ceils the +/// maximum coordinates, then clips the exclusive rectangle to the canvas. +/// +/// **Arguments:** `transform` supplies affine geometry; `canvas_width` and `canvas_height` define +/// clipping limits. **Returns:** `Some([x0, y0, x1, y1])` or `None` for an empty/off-canvas result. +/// +/// **Side Effects / Dependencies:** None and performs no allocation. +pub fn transformed_bounds( + transform: &SelectionTransform, + canvas_width: u32, + canvas_height: u32, +) -> Option<[u32; 4]> { + if transform.is_empty() || canvas_width == 0 || canvas_height == 0 { + return None; + } + let corners = [ + transform.rotate_point(transform.pos.x, transform.pos.y), + transform.rotate_point(transform.pos.x + transform.size.x, transform.pos.y), + transform.rotate_point(transform.pos.x, transform.pos.y + transform.size.y), + transform.rotate_point( + transform.pos.x + transform.size.x, + transform.pos.y + transform.size.y, + ), + ]; + let min_x = corners.iter().map(|p| p.0).fold(f32::INFINITY, f32::min); + let min_y = corners.iter().map(|p| p.1).fold(f32::INFINITY, f32::min); + let max_x = corners + .iter() + .map(|p| p.0) + .fold(f32::NEG_INFINITY, f32::max); + let max_y = corners + .iter() + .map(|p| p.1) + .fold(f32::NEG_INFINITY, f32::max); + let x0 = min_x.floor().max(0.0).min(canvas_width as f32) as u32; + let y0 = min_y.floor().max(0.0).min(canvas_height as f32) as u32; + let x1 = max_x.ceil().max(0.0).min(canvas_width as f32) as u32; + let y1 = max_y.ceil().max(0.0).min(canvas_height as f32) as u32; + (x0 < x1 && y0 < y1).then_some([x0, y0, x1, y1]) +} + +/// Calculates rotation from the pointer's angular delta relative to drag start. +/// +/// **Purpose:** Prevents the rotate handle from jumping to an absolute pointer angle on press. +/// **Logic & Workflow:** Measures start and current angles around the original transform center and +/// adds their difference to the original rotation. **Arguments:** `original` is drag-start geometry; +/// `start` and `current` are canvas pointer coordinates. **Returns:** The updated angle in radians. +/// **Side Effects / Dependencies:** None. +pub fn rotation_from_drag( + original: &SelectionTransform, + start: (f32, f32), + current: (f32, f32), +) -> f32 { + let center_x = original.pos.x + original.size.x * 0.5; + let center_y = original.pos.y + original.size.y * 0.5; + let start_angle = (start.1 - center_y).atan2(start.0 - center_x); + let current_angle = (current.1 - center_y).atan2(current.0 - center_x); + original.rotation + current_angle - start_angle +} + +/// Source-over composites a transformed selection into an existing canvas buffer. +/// +/// **Purpose:** Provides one inverse-affine nearest-neighbor sampler for both interactive preview +/// and final apply, including scaling, rotation, negative placement, and rotated AABB coverage. +/// +/// **Logic & Workflow:** Iterates only the clipped transformed AABB, maps each destination pixel +/// center through inverse rotation and scale into source pixel-center space, and alpha-composites +/// valid source samples onto `destination` without allocating another image. +/// +/// **Arguments:** `destination` is canvas RGBA data, `canvas_width`/`canvas_height` describe its +/// stride and clipping area, and `transform` supplies source pixels and affine geometry. +/// **Returns:** The affected clipped AABB, or `None` if no rasterization is possible. +/// +/// **Side Effects / Dependencies:** Mutates only pixels inside the returned bounds; no I/O or +/// allocation is performed. +pub fn composite_transform( + destination: &mut [u8], + canvas_width: u32, + canvas_height: u32, + transform: &SelectionTransform, +) -> Option<[u32; 4]> { + let bounds = transformed_bounds(transform, canvas_width, canvas_height)?; + if destination.len() < canvas_width as usize * canvas_height as usize * 4 { + return None; + } + let source_width = transform.width as f32; + let source_height = transform.height as f32; + let center_x = transform.pos.x + transform.size.x * 0.5; + let center_y = transform.pos.y + transform.size.y * 0.5; + let cos = transform.rotation.cos(); + let sin = transform.rotation.sin(); + for y in bounds[1]..bounds[3] { + for x in bounds[0]..bounds[2] { + let dx = x as f32 + 0.5 - center_x; + let dy = y as f32 + 0.5 - center_y; + let local_x = dx * cos + dy * sin + transform.size.x * 0.5; + let local_y = -dx * sin + dy * cos + transform.size.y * 0.5; + let source_x = local_x / transform.size.x.max(1.0e-6) * source_width - 0.5; + let source_y = local_y / transform.size.y.max(1.0e-6) * source_height - 0.5; + let sx = source_x.round() as i32; + let sy = source_y.round() as i32; + if sx < 0 || sy < 0 || sx >= transform.width as i32 || sy >= transform.height as i32 { + continue; + } + let source_index = (sy as usize * transform.width as usize + sx as usize) * 4; + let destination_index = (y as usize * canvas_width as usize + x as usize) * 4; + if source_index + 3 >= transform.pixels.len() { + continue; + } + let source_alpha = transform.pixels[source_index + 3] as f32 / 255.0; + if source_alpha <= 0.0 { + continue; + } + let destination_alpha = destination[destination_index + 3] as f32 / 255.0; + let output_alpha = source_alpha + destination_alpha * (1.0 - source_alpha); + for channel in 0..3 { + let source = transform.pixels[source_index + channel] as f32; + let destination_channel = destination[destination_index + channel] as f32; + destination[destination_index + channel] = ((source * source_alpha + + destination_channel * destination_alpha * (1.0 - source_alpha)) + / output_alpha) + .round() as u8; + } + destination[destination_index + 3] = (output_alpha * 255.0).round() as u8; + } + } + Some(bounds) +} diff --git a/hcie-iced-app/crates/hcie-iced-gui/src/sidebar/mod.rs b/hcie-iced-app/crates/hcie-iced-gui/src/sidebar/mod.rs index 59721c6..60560f6 100644 --- a/hcie-iced-app/crates/hcie-iced-gui/src/sidebar/mod.rs +++ b/hcie-iced-app/crates/hcie-iced-gui/src/sidebar/mod.rs @@ -163,6 +163,7 @@ pub fn view<'a>( expanded: bool, ) -> Element<'a, Message> { let sidebar_width = if expanded { 72 } else { 36 }; + log::info!("[sidebar] expanded={} active_slot={} sub_tools_open={}", expanded, active_slot, sub_tools_open); // Tool buttons — single column or 2-column grid depending on expanded state let mut tool_list = column![].spacing(1); @@ -536,7 +537,7 @@ fn tool_button<'a>( ); let popup_affordance: Element<'a, Message> = if has_sub_tools { - iced::widget::tooltip( + styles::balloon_tooltip( button(text("▸").size(12).color(colors.text_primary)) .on_press(Message::SubtoolsToggle(slot_idx)) .padding(0) @@ -553,10 +554,10 @@ fn tool_button<'a>( border: iced::Border::default(), ..Default::default() }), - text(format!("Open {} sub-tools", slot.label)).size(11), + format!("Open {} sub-tools", slot.label), iced::widget::tooltip::Position::Right, + colors, ) - .into() } else { iced::widget::Space::with_width(0).into() }; @@ -611,47 +612,57 @@ fn tool_button<'a>( .into() }; - let item = button(container(popup_icon).width(32).height(28).center(18)) - .on_press(Message::ToolSelected(tool)) - .padding(0) + // Wrap the icon in a fixed-size, centered container so the popup + // button always occupies 32×28 pixels even when the SVG is still + // loading or the text fallback is used. + let icon_container = container(popup_icon) + .width(20) + .height(20) + .center_x(Length::Fill) + .center_y(Length::Fill); + let item_content = container(icon_container) .width(32) - .height(28) - .style( - move |_theme: &iced::Theme, status: iced::widget::button::Status| match status { - iced::widget::button::Status::Hovered => iced::widget::button::Style { - background: Some(iced::Background::Color(if is_tool_active { - c.accent_green - } else { - c.bg_hover - })), - text_color: c.text_primary, - border: iced::Border::default() - .color(if is_tool_active { - c.accent - } else { - c.border_high - }) - .width(if is_tool_active { 1 } else { 0 }), - ..Default::default() - }, - _ => iced::widget::button::Style { - background: Some(iced::Background::Color(if is_tool_active { - c.accent_green - } else { - c.bg_active - })), - text_color: c.text_primary, - border: iced::Border::default() - .color(if is_tool_active { - c.accent - } else { - c.border_high - }) - .width(if is_tool_active { 1 } else { 0 }), - ..Default::default() - }, + .height(28); + + let item = button( + container(item_content) + .width(32) + .height(28) + .center_x(Length::Fill) + .center_y(Length::Fill), + ) + .on_press(Message::ToolSelected(tool)) + .padding(0) + .width(32) + .height(28) + .style( + move |_theme: &iced::Theme, status: iced::widget::button::Status| match status { + iced::widget::button::Status::Hovered => iced::widget::button::Style { + background: Some(iced::Background::Color(if is_tool_active { + c.accent_green + } else { + c.bg_hover + })), + text_color: c.text_primary, + border: iced::Border::default() + .color(if is_tool_active { c.accent } else { c.border_high }) + .width(if is_tool_active { 1 } else { 0 }), + ..Default::default() }, - ); + _ => iced::widget::button::Style { + background: Some(iced::Background::Color(if is_tool_active { + c.accent_green + } else { + c.bg_active + })), + text_color: c.text_primary, + border: iced::Border::default() + .color(if is_tool_active { c.accent } else { c.border_high }) + .width(if is_tool_active { 1 } else { 0 }), + ..Default::default() + }, + }, + ); let shortcut = tool_shortcut(tool); let tooltip_text = if shortcut.is_empty() { @@ -659,10 +670,11 @@ fn tool_button<'a>( } else { format!("{tool_name} ({shortcut})") }; - popup_row = popup_row.push(iced::widget::tooltip( + popup_row = popup_row.push(styles::balloon_tooltip( item, - text(tooltip_text).size(11), + tooltip_text, iced::widget::tooltip::Position::Right, + colors, )); if (tool_index + 1) % 2 == 0 || tool_index + 1 == slot.tools.len() { popup_grid = popup_grid.push(popup_row); @@ -684,10 +696,11 @@ fn tool_button<'a>( ..Default::default() }); - let parent = container(iced::widget::tooltip( + let parent = container(styles::balloon_tooltip( btn_element, - iced::widget::text(format!("{} (sub-tools open)", slot.label)).size(11), + format!("{} (sub-tools open)", slot.label), iced::widget::tooltip::Position::Right, + colors, )) .width(68); container(column![parent, popup].spacing(1)) @@ -696,12 +709,12 @@ fn tool_button<'a>( } else { // Tooltip on hover let tooltip_text = slot.label.to_string(); - iced::widget::tooltip( + styles::balloon_tooltip( btn_element, - iced::widget::text(tooltip_text).size(11), + tooltip_text, iced::widget::tooltip::Position::Right, + colors, ) - .into() } } diff --git a/hcie-iced-app/crates/hcie-iced-gui/tests/selection_test.rs b/hcie-iced-app/crates/hcie-iced-gui/tests/selection_test.rs index 071190e..096453b 100644 --- a/hcie-iced-app/crates/hcie-iced-gui/tests/selection_test.rs +++ b/hcie-iced-app/crates/hcie-iced-gui/tests/selection_test.rs @@ -2,6 +2,9 @@ use hcie_iced_gui::selection::clipboard::centered_transform; use hcie_iced_gui::selection::state::{combine_masks, mask_bounds, selected_spans}; +use hcie_iced_gui::selection::transform::{ + composite_transform, rotation_from_drag, transformed_bounds, +}; use hcie_iced_gui::selection::{CropState, SelectionMode, SelectionTransform, TransformHandle}; #[test] @@ -122,3 +125,91 @@ fn clipboard_transform_is_centered_and_transform_geometry_is_safe() { assert!(transform.size.x > 0.0 && transform.size.y > 0.0); assert!((transform.size.x / transform.size.y - 2.0).abs() < 0.001); } + +/// Verifies that the shared inverse-affine rasterizer preserves identity placement exactly. +/// +/// **Purpose:** Guards preview/apply parity for the common unscaled move path. +/// **Logic & Workflow:** Composites a two-pixel opaque source into an empty canvas and compares the +/// exact affected pixels and returned bounds. **Arguments & Returns:** No arguments or return value; +/// failed assertions identify sampling drift. **Side Effects / Dependencies:** None. +#[test] +fn inverse_affine_identity_sampling_is_exact() { + let transform = SelectionTransform { + pixels: vec![255, 0, 0, 255, 0, 255, 0, 255], + width: 2, + height: 1, + pos: iced::Vector::new(1.0, 2.0), + size: iced::Vector::new(2.0, 1.0), + rotation: 0.0, + }; + let mut canvas = vec![0; 4 * 4 * 4]; + assert_eq!( + composite_transform(&mut canvas, 4, 4, &transform), + Some([1, 2, 3, 3]) + ); + assert_eq!(&canvas[(2 * 4 + 1) * 4..(2 * 4 + 3) * 4], transform.pixels); +} + +/// Verifies that rotated bounds include every corner rather than clipping to the unrotated box. +/// +/// **Purpose:** Protects dirty restoration, partial upload, and final apply from rotated-corner +/// clipping. **Logic & Workflow:** Rotates a square by 45 degrees and checks its expanded integer +/// AABB. **Arguments & Returns:** No arguments or return value. **Side Effects / Dependencies:** None. +#[test] +fn rotated_bounds_expand_to_cover_corners() { + let transform = SelectionTransform { + pixels: vec![255; 2 * 2 * 4], + width: 2, + height: 2, + pos: iced::Vector::new(4.0, 4.0), + size: iced::Vector::new(2.0, 2.0), + rotation: std::f32::consts::FRAC_PI_4, + }; + assert_eq!(transformed_bounds(&transform, 10, 10), Some([3, 3, 7, 7])); +} + +/// Verifies rotated handle hit-testing and opposite-corner anchoring during resize. +/// +/// **Purpose:** Guards cursor routing and rotated scaling geometry together. +/// **Logic & Workflow:** Hit-tests the transformed top-left point, resizes from that handle, and +/// confirms the opposite corner remains fixed. **Arguments & Returns:** None. **Side Effects:** None. +#[test] +fn rotated_handle_hit_test_and_resize_share_geometry() { + let mut transform = SelectionTransform { + pixels: vec![255; 4 * 2 * 4], + width: 4, + height: 2, + pos: iced::Vector::new(4.0, 4.0), + size: iced::Vector::new(4.0, 2.0), + rotation: std::f32::consts::FRAC_PI_2, + }; + let top_left = transform.handle_position(TransformHandle::TopLeft, 1.0); + assert_eq!( + transform.hit_test(top_left.0, top_left.1, 1.0), + TransformHandle::TopLeft + ); + let anchored = transform.handle_position(TransformHandle::BottomRight, 1.0); + transform.resize_from_delta(TransformHandle::TopLeft, 0.0, 1.0); + let after = transform.handle_position(TransformHandle::BottomRight, 1.0); + assert!((anchored.0 - after.0).abs() < 0.001); + assert!((anchored.1 - after.1).abs() < 0.001); +} + +/// Verifies rotation uses drag-start delta and retains pre-existing rotation. +/// +/// **Purpose:** Prevents the first rotate move from jumping to an absolute angle. +/// **Logic & Workflow:** Starts from a non-zero rotation and applies a quarter-turn pointer delta. +/// **Arguments & Returns:** None. **Side Effects / Dependencies:** None. +#[test] +fn rotation_drag_adds_delta_to_original_angle() { + let transform = SelectionTransform { + pixels: vec![255; 4], + width: 1, + height: 1, + pos: iced::Vector::new(4.5, 4.5), + size: iced::Vector::new(1.0, 1.0), + rotation: 0.25, + }; + let angle = rotation_from_drag(&transform, (6.0, 5.0), (5.0, 6.0)); + assert!((angle - (0.25 + std::f32::consts::FRAC_PI_2)).abs() < 0.001); +}