feat: implement file opening logic in a new tab and streamline document loading process
mandatory-regression-gate / deterministic-tests (push) Has been cancelled
mandatory-regression-gate / protected-performance-path (push) Has been cancelled

This commit is contained in:
Your Name
2026-07-24 00:51:09 +03:00
parent a2264b130d
commit f2cca7e6a1
8 changed files with 230 additions and 260 deletions
+129 -139
View File
@@ -12,8 +12,10 @@
//! These sections are optimized for 4K performance. Modifying them //! These sections are optimized for 4K performance. Modifying them
//! may cause rendering regressions or performance degradation. //! may cause rendering regressions or performance degradation.
mod document_open;
mod transform_dirty; mod transform_dirty;
use self::document_open::load_path_into_engine;
use self::transform_dirty::preview_bounds; use self::transform_dirty::preview_bounds;
use crate::ai_chat::{self, AiChatState, ChatMessage, ComfyUiState, SYSTEM_PROMPT}; use crate::ai_chat::{self, AiChatState, ChatMessage, ComfyUiState, SYSTEM_PROMPT};
use crate::dialogs; use crate::dialogs;
@@ -2498,6 +2500,57 @@ impl HcieIcedApp {
self.refresh_panel_caches(); self.refresh_panel_caches();
} }
/// Opens a file in a newly allocated editor tab.
///
/// **Purpose:** Prevents file-open entry points from replacing the active
/// document engine. **Logic & Workflow:** Creates a disposable 1x1 document,
/// loads the selected path into its engine, activates and initializes it on
/// success, or removes it and restores the previous tab on failure.
/// **Arguments:** `path` is the file to load and `add_to_recent` controls
/// recent-file persistence. **Returns:** The new document index or an engine
/// error string. **Side Effects / Dependencies:** Reads the file, appends or
/// removes one document, updates GPU upload caches, and may persist recents.
fn open_path_in_new_tab(
&mut self,
path: std::path::PathBuf,
add_to_recent: bool,
) -> Result<usize, String> {
let previous_active = self.active_doc;
let previous_count = self.documents.len();
let _ = self.update(Message::NewDocument(1, 1));
let document_index = self.documents.len() - 1;
if let Err(error) = load_path_into_engine(&mut self.documents[document_index].engine, &path)
{
self.documents.remove(document_index);
self.active_doc = previous_active.min(self.documents.len().saturating_sub(1));
debug_assert_eq!(self.documents.len(), previous_count);
return Err(error);
}
self.show_welcome = false;
self.active_doc = document_index;
let document = &mut self.documents[document_index];
document.engine.pre_tile_all_layers();
document.selection_model_dirty = true;
document.name = path
.file_name()
.map(|name| name.to_string_lossy().to_string())
.unwrap_or_else(|| "Untitled".to_string());
document.source_path = Some(path.clone());
document.modified = false;
document.composite_raw = document.engine.get_composite_pixels();
document.composite_pixels.replace(&document.composite_raw);
document.render_generation = document.render_generation.wrapping_add(1);
document.full_upload.replace(true);
if add_to_recent {
self.add_recent_file(&path);
}
self.refresh_composite_if_needed();
Ok(document_index)
}
/// Apply brush parameters from tool state to the engine. /// Apply brush parameters from tool state to the engine.
/// ///
/// Called before starting a stroke to ensure the engine uses /// Called before starting a stroke to ensure the engine uses
@@ -7895,49 +7948,15 @@ impl HcieIcedApp {
} }
Message::FileDialogClosed(Some(path)) => { Message::FileDialogClosed(Some(path)) => {
let path_str = path.to_string_lossy().to_string(); let path_str = path.to_string_lossy().to_string();
let ext = path
.extension()
.and_then(|e| e.to_str())
.unwrap_or("png")
.to_lowercase();
match self.pending_file_operation.take() { match self.pending_file_operation.take() {
Some(PendingFileOperation::Open) => { Some(PendingFileOperation::Open) => {
let _ = self.update(Message::NewDocument(1, 1)); match self.open_path_in_new_tab(path, true) {
let doc_idx = self.documents.len() - 1; Ok(_) => {
let engine = &mut self.documents[doc_idx].engine;
let result = match ext.as_str() {
"psd" => engine.import_psd(&path_str),
"kra" => engine.import_kra(&path_str),
"hcie" => engine.load_native(&path_str),
_ => engine.open_image(&path_str),
};
match result {
Ok(()) => {
self.show_welcome = false;
self.active_doc = doc_idx;
self.documents[doc_idx].engine.pre_tile_all_layers();
self.documents[doc_idx].selection_model_dirty = true;
self.documents[doc_idx].name = path
.file_name()
.map(|n| n.to_string_lossy().to_string())
.unwrap_or_else(|| "Untitled".to_string());
self.documents[doc_idx].source_path = Some(path.clone());
self.documents[doc_idx].modified = false;
self.add_recent_file(&path);
self.documents[doc_idx].full_upload.replace(true);
self.documents[doc_idx].cached_layers =
self.documents[doc_idx].engine.layer_infos();
self.refresh_composite_if_needed();
return Task::perform(async {}, |_| Message::CompositeRefresh); return Task::perform(async {}, |_| Message::CompositeRefresh);
} }
Err(e) => { Err(e) => {
log::error!("Failed to open: {}", e); log::error!("Failed to open: {}", e);
// Remove empty dummy document if load failed
if self.documents.len() > 1 {
self.documents.pop();
self.active_doc = self.documents.len() - 1;
}
} }
} }
} }
@@ -8358,6 +8377,7 @@ impl HcieIcedApp {
pending_thumbnail_layers: std::collections::HashSet::new(), pending_thumbnail_layers: std::collections::HashSet::new(),
thumb_gen: 0, thumb_gen: 0,
}); });
self.active_doc = self.documents.len() - 1;
} }
Message::DocSwitch(idx) => { Message::DocSwitch(idx) => {
@@ -8406,7 +8426,10 @@ impl HcieIcedApp {
} }
Message::FileOpened(Ok(path)) => { Message::FileOpened(Ok(path)) => {
log::info!("File opened: {:?}", path); match self.open_path_in_new_tab(path, true) {
Ok(_) => return Task::perform(async {}, |_| Message::CompositeRefresh),
Err(error) => log::error!("Failed to open file: {error}"),
}
} }
Message::FileOpened(Err(e)) => { Message::FileOpened(Err(e)) => {
@@ -8463,53 +8486,9 @@ impl HcieIcedApp {
self.active_menu = None; self.active_menu = None;
let path = std::path::PathBuf::from(&path_str); let path = std::path::PathBuf::from(&path_str);
if path.exists() { if path.exists() {
let ext = path match self.open_path_in_new_tab(path, true) {
.extension() Ok(_) => return Task::perform(async {}, |_| Message::CompositeRefresh),
.and_then(|e| e.to_str()) Err(error) => log::error!("Failed to open recent file: {error}"),
.unwrap_or("")
.to_lowercase();
let result = match ext.as_str() {
"psd" => self.documents[self.active_doc].engine.import_psd(&path_str),
"kra" => self.documents[self.active_doc].engine.import_kra(&path_str),
"hcie" => self.documents[self.active_doc]
.engine
.load_native(&path_str),
_ => self.documents[self.active_doc].engine.open_image(&path_str),
};
match result {
Ok(()) => {
self.documents[self.active_doc].engine.pre_tile_all_layers();
self.documents[self.active_doc].name = path
.file_name()
.map(|n| n.to_string_lossy().to_string())
.unwrap_or_else(|| "Untitled".to_string());
self.documents[self.active_doc].source_path = Some(path.clone());
self.add_recent_file(&path);
let composite = self.documents[self.active_doc]
.engine
.get_composite_pixels();
self.documents[self.active_doc].composite_raw = composite;
self.documents[self.active_doc]
.composite_pixels
.replace(&self.documents[self.active_doc].composite_raw);
let selection = self.documents[self.active_doc]
.engine
.get_selection_mask()
.map(ToOwned::to_owned);
set_document_selection_mask(
&mut self.documents[self.active_doc],
selection,
);
self.documents[self.active_doc].full_upload.replace(true);
self.documents[self.active_doc].render_generation = self.documents
[self.active_doc]
.render_generation
.wrapping_add(1);
self.documents[self.active_doc].cached_layers =
self.documents[self.active_doc].engine.layer_infos();
return Task::perform(async {}, |_| Message::CompositeRefresh);
}
Err(e) => log::error!("Failed to open recent file: {}", e),
} }
} else { } else {
log::error!("Recent file no longer exists: {}", path_str); log::error!("Recent file no longer exists: {}", path_str);
@@ -9060,64 +9039,12 @@ impl HcieIcedApp {
} }
Message::ViewerOpenFile(path) => { Message::ViewerOpenFile(path) => {
// Open into a new editor document so browsing never overwrites a dirty canvas. // Open into a new editor document so browsing never overwrites a dirty canvas.
let _ = self.update(Message::NewDocument(1, 1)); match self.open_path_in_new_tab(path.clone(), true) {
let document_index = self.documents.len() - 1; Ok(_) => {
let path_str = path.to_string_lossy().to_string();
let ext = path
.extension()
.and_then(|e| e.to_str())
.unwrap_or("")
.to_lowercase();
let result = match ext.as_str() {
"psd" => self.documents[document_index].engine.import_psd(&path_str),
"kra" => self.documents[document_index].engine.import_kra(&path_str),
"hcie" => self.documents[document_index].engine.load_native(&path_str),
_ => self.documents[document_index].engine.open_image(&path_str),
};
match result {
Ok(()) => {
self.viewer_active = false; self.viewer_active = false;
self.active_doc = document_index;
self.documents[document_index].engine.pre_tile_all_layers();
self.documents[document_index].name = path
.file_name()
.map(|n| n.to_string_lossy().to_string())
.unwrap_or_else(|| "Untitled".to_string());
self.documents[document_index].source_path = Some(path.clone());
self.add_recent_file(&path);
let composite =
self.documents[document_index].engine.get_composite_pixels();
self.documents[document_index].composite_raw = composite;
self.documents[document_index]
.composite_pixels
.replace(&self.documents[document_index].composite_raw);
let selection = self.documents[document_index]
.engine
.get_selection_mask()
.map(ToOwned::to_owned);
set_document_selection_mask(&mut self.documents[document_index], selection);
self.documents[document_index].full_upload.replace(true);
self.documents[document_index].render_generation = self.documents
[document_index]
.render_generation
.wrapping_add(1);
self.documents[document_index].cached_layers =
self.documents[document_index].engine.layer_infos();
let history_len = self.documents[document_index].engine.history_len();
self.documents[document_index].cached_history = (0..history_len)
.filter_map(|i| {
self.documents[document_index]
.engine
.history_description(i)
.map(|d| (i, d))
})
.collect();
self.documents[document_index].history_current =
self.documents[document_index].engine.history_current();
return Task::perform(async {}, |_| Message::CompositeRefresh); return Task::perform(async {}, |_| Message::CompositeRefresh);
} }
Err(e) => { Err(e) => {
self.documents.remove(document_index);
log::error!("Failed to open file from viewer: {}", e); log::error!("Failed to open file from viewer: {}", e);
self.viewer_state.load_error = self.viewer_state.load_error =
Some(format!("Could not edit {}: {e}", path.display())); Some(format!("Could not edit {}: {e}", path.display()));
@@ -10174,7 +10101,7 @@ mod cycle_one_ux_tests {
use super::{ use super::{
active_index_after_document_close, consume_selection_sync_request, active_index_after_document_close, consume_selection_sync_request,
document_close_disposition, overlay_escape_target, should_schedule_canvas_frames, document_close_disposition, overlay_escape_target, should_schedule_canvas_frames,
union_regions, DocumentCloseDisposition, OverlayEscapeTarget, union_regions, DocumentCloseDisposition, HcieIcedApp, Message, OverlayEscapeTarget,
}; };
use hcie_engine_api::Tool; use hcie_engine_api::Tool;
@@ -10254,6 +10181,69 @@ mod cycle_one_ux_tests {
assert!(!should_schedule_canvas_frames(true, Tool::VectorSelect)); assert!(!should_schedule_canvas_frames(true, Tool::VectorSelect));
} }
/// Confirms a newly created document becomes the selected tab.
///
/// **Purpose:** Guards against a tab-management regression where the new
/// document existed but later operations still targeted the previous tab.
/// **Logic & Workflow:** Creates application state, dispatches the normal
/// `NewDocument` message, and checks both count and active index.
/// **Arguments & Returns:** None. **Side Effects / Dependencies:** Allocates
/// two in-memory engine documents and reads ordinary application settings.
#[test]
fn new_document_becomes_the_active_tab() {
let (mut app, _) = HcieIcedApp::new(None, None);
let previous_count = app.documents.len();
let _ = app.update(Message::NewDocument(32, 24));
assert_eq!(app.documents.len(), previous_count + 1);
assert_eq!(app.active_doc, app.documents.len() - 1);
assert_eq!(app.documents[app.active_doc].engine.canvas_width(), 32);
assert_eq!(app.documents[app.active_doc].engine.canvas_height(), 24);
}
/// Confirms opening another file appends a tab without mutating the first.
///
/// **Purpose:** Reproduces the reported second-document overwrite regression.
/// **Logic & Workflow:** Writes a tiny PNG, opens it through the shared tab
/// helper, and verifies the initial document identity and new active tab.
/// **Arguments & Returns:** None. **Side Effects / Dependencies:** Creates and
/// removes one temporary image file; recent-file persistence is disabled.
#[test]
fn opening_a_second_file_preserves_the_existing_tab() {
let (mut app, _) = HcieIcedApp::new(None, None);
let original_name = app.documents[0].name.clone();
let unique = std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.unwrap()
.as_nanos();
let path = std::env::temp_dir().join(format!(
"hcie-open-tab-{}-{unique}.png",
std::process::id()
));
image::save_buffer(
&path,
&[255, 0, 0, 255, 0, 255, 0, 255],
2,
1,
image::ColorType::Rgba8,
)
.unwrap();
let opened_index = app.open_path_in_new_tab(path.clone(), false).unwrap();
assert_eq!(app.documents.len(), 2);
assert_eq!(opened_index, 1);
assert_eq!(app.active_doc, 1);
assert_eq!(app.documents[0].name, original_name);
assert!(app.documents[0].source_path.is_none());
assert_eq!(app.documents[1].source_path.as_deref(), Some(path.as_path()));
assert_eq!(app.documents[1].engine.canvas_width(), 2);
assert_eq!(app.documents[1].engine.canvas_height(), 1);
let _ = std::fs::remove_file(path);
}
// ── Recent color proximity dedup ───────────────────── // ── Recent color proximity dedup ─────────────────────
/// Confirms an identical color IS proximate (diff=0 ≤ threshold). /// Confirms an identical color IS proximate (diff=0 ≤ threshold).
@@ -0,0 +1,60 @@
//! File-format routing for documents opened in editor tabs.
//!
//! **Purpose:** Keeps all tab-opening entry points on one engine-loading path.
//! **Logic & Workflow:** Classifies the extension case-insensitively, delegates
//! layered formats to their dedicated engine API importers, and uses the generic
//! image loader for raster formats. **Side Effects / Dependencies:** Mutates only
//! the supplied `hcie_engine_api::Engine` and reads the requested file.
use hcie_engine_api::Engine;
use std::path::Path;
/// Loads a path into an otherwise disposable document engine.
///
/// **Purpose:** Ensures Open, Open Recent, Viewer, and imported file messages use
/// identical format routing. **Logic & Workflow:** PSD, KRA, and HCIE extensions
/// select dedicated APIs; every other extension uses `Engine::open_image`.
/// **Arguments:** `engine` receives the file and `path` identifies it.
/// **Returns:** `Ok(())` on success or the engine's error text on failure.
/// **Side Effects / Dependencies:** Replaces document state inside `engine` and
/// performs file I/O through `hcie-engine-api`.
pub(crate) fn load_path_into_engine(engine: &mut Engine, path: &Path) -> Result<(), String> {
let path_text = path.to_string_lossy();
match normalized_extension(path).as_str() {
"psd" => engine.import_psd(&path_text),
"kra" => engine.import_kra(&path_text),
"hcie" => engine.load_native(&path_text),
_ => engine.open_image(&path_text),
}
}
/// Returns a lowercase extension for format routing.
///
/// **Purpose:** Makes format selection deterministic across differently cased
/// file names. **Arguments:** `path` is the source file. **Returns:** A lowercase
/// extension or an empty string. **Side Effects / Dependencies:** None.
fn normalized_extension(path: &Path) -> String {
path.extension()
.and_then(|extension| extension.to_str())
.unwrap_or_default()
.to_ascii_lowercase()
}
#[cfg(test)]
mod tests {
use super::normalized_extension;
use std::path::Path;
/// Confirms extension routing remains case-insensitive.
///
/// **Purpose:** Prevents layered formats from accidentally taking the raster
/// loader after path normalization changes. **Logic & Workflow:** Exercises
/// uppercase and extensionless names. **Arguments & Returns:** None.
/// **Side Effects / Dependencies:** None.
#[test]
fn extension_normalization_is_case_insensitive() {
assert_eq!(normalized_extension(Path::new("image.PSD")), "psd");
assert_eq!(normalized_extension(Path::new("project.KrA")), "kra");
assert_eq!(normalized_extension(Path::new("untitled")), "");
}
}
@@ -1,126 +1,35 @@
//! Runtime build-ID management. //! Runtime access to immutable HCIE build identity.
//! //!
//! **Purpose:** Reads, atomically increments, and writes `build.id` on every application startup //! **Purpose:** Exposes the version generated by `hcie-build-info` without
//! so each launch receives a unique, monotonically increasing build number without requiring //! depending on Cargo-only environment variables or a writable source tree.
//! a wrapper script or a separate build step. //! **Logic & Workflow:** Cargo's build script embeds the canonical build ID in
//! **Logic & Workflow:** Locates the repository root by walking upward from `CARGO_MANIFEST_DIR` //! `hcie_build_info::VERSION`; application startup clones that static value for
//! until a `Cargo.toml` containing `[workspace]` is found, then reads/increments/writes `build.id` //! title-bar state. **Side Effects / Dependencies:** None at runtime.
//! under a file lock. Exposes the resulting version string for title-bar display.
//! **Side Effects / Dependencies:** Performs one file-system write per startup; concurrent launches
//! are serialized via an advisory directory lock.
use std::fs; /// Returns the build version embedded by Cargo during compilation.
use std::path::{Path, PathBuf};
use std::time::{SystemTime, UNIX_EPOCH};
/// Maximum age (seconds) before a lock is considered stale and forcefully removed.
const LOCK_STALE_SECS: u64 = 30;
/// File-system lock directory used to serialize concurrent increments.
const LOCK_DIR: &str = ".build-id.lock";
/// Finds the repository root by walking upward from the compile-time manifest directory.
/// ///
/// **Returns:** The repository root `PathBuf`. /// **Purpose:** Supplies a stable title-bar version in development, packaged,
/// **Side Effects / Dependencies:** Panics if the workspace root cannot be located. /// and directly executed binaries. **Logic & Workflow:** Copies the compile-time
fn find_repo_root() -> PathBuf { /// constant instead of searching for and mutating `build.id` at startup.
let manifest_dir = PathBuf::from( /// **Arguments:** None. **Returns:** A version such as `0.1.0+build.1019`.
std::env::var_os("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR is required"), /// **Side Effects / Dependencies:** Depends only on the `hcie-build-info` crate;
); /// it performs no environment lookup or file-system access.
let mut candidate = manifest_dir.clone();
loop {
let tombstone = candidate.join("Cargo.toml");
if tombstone.exists() {
if let Ok(content) = fs::read_to_string(&tombstone) {
if content.contains("[workspace]") {
return candidate;
}
}
}
if !candidate.pop() {
panic!("could not locate repository root above {:?}", manifest_dir);
}
}
}
/// Returns current Unix timestamp in seconds, or 0 on clock error.
fn unix_now() -> u64 {
SystemTime::now()
.duration_since(UNIX_EPOCH)
.map(|d| d.as_secs())
.unwrap_or(0)
}
/// Checks whether a lock directory is stale (older than `LOCK_STALE_SECS`).
fn is_lock_stale(lock_dir: &Path) -> bool {
let ts_file = lock_dir.join("ts");
if let Ok(ts_str) = fs::read_to_string(&ts_file) {
if let Ok(ts) = ts_str.trim().parse::<u64>() {
return unix_now().saturating_sub(ts) > LOCK_STALE_SECS;
}
}
// No timestamp file or unparseable → treat as stale.
true
}
/// Acquires an advisory directory lock, increments the counter, and releases the lock.
///
/// **Returns:** The new (post-increment) build ID.
/// **Side Effects / Dependencies:** Creates and removes a temporary lock directory.
fn increment_build_id(root: &Path) -> u64 {
let lock_dir = root.join(LOCK_DIR);
let counter_file = root.join("build.id");
// Busy-wait with stale-lock detection.
loop {
match fs::create_dir(&lock_dir) {
Ok(()) => {
// Write timestamp for stale-lock detection by other instances.
let _ = fs::write(
lock_dir.join("ts"),
format!("{}\n", unix_now()),
);
break;
}
Err(_) => {
if is_lock_stale(&lock_dir) {
let _ = fs::remove_dir_all(&lock_dir);
continue;
}
std::thread::sleep(std::time::Duration::from_millis(5));
}
}
}
// Read current value.
let current: u64 = fs::read_to_string(&counter_file)
.ok()
.and_then(|s| s.trim().parse().ok())
.unwrap_or(0);
let next = current + 1;
// Atomic write: write to temp file, then rename.
let tmp = counter_file.with_extension("id.tmp");
fs::write(&tmp, format!("{}\n", next)).expect("failed to write build.id tmp");
fs::rename(&tmp, &counter_file).expect("failed to rename build.id");
// Release lock.
let _ = fs::remove_dir_all(&lock_dir);
next
}
/// Reads, increments, and returns the build version string at runtime.
///
/// **Returns:** A version string like `"0.1.0+build.992"`.
/// **Side Effects / Dependencies:** Writes to `build.id` on every call (intended for startup only).
pub fn runtime_build_version() -> String { pub fn runtime_build_version() -> String {
let root = find_repo_root(); hcie_build_info::VERSION.to_owned()
let build_id = increment_build_id(&root); }
let base_version = hcie_build_info::VERSION
.split("+build.") #[cfg(test)]
.next() mod tests {
.unwrap_or("0.1.0"); use super::runtime_build_version;
format!("{base_version}+build.{build_id}")
/// Confirms startup uses the immutable build-script output.
///
/// **Purpose:** Prevents regression to runtime `CARGO_MANIFEST_DIR` access.
/// **Logic & Workflow:** Compares the public wrapper with the embedded value.
/// **Arguments & Returns:** Takes no arguments and returns nothing.
/// **Side Effects / Dependencies:** None.
#[test]
fn runtime_version_is_the_compile_time_version() {
assert_eq!(runtime_build_version(), hcie_build_info::VERSION);
}
} }
BIN
View File
Binary file not shown.
BIN
View File
Binary file not shown.
BIN
View File
Binary file not shown.
BIN
View File
Binary file not shown.
+11
View File
@@ -0,0 +1,11 @@
Build:1022
commit : a2264b130d47fbbd565226040bc28fd5b9626177
4K MULTI LAYEPERFORMANCE OK : SOL 5.6 HIGH feat(perf): add real-time canvas performance measurements
- Introduced a new performance module for the iced canvas to measure input queueing, engine drawing, CPU compositing/staging, GPU upload, and renderer preparation costs.
- Implemented a performance probe in the egui reference renderer for comparative diagnostics.
- Added methods to record durations, byte transfers, and input timestamps, enabling detailed performance analysis.
- Enhanced the CanvasShaderPipeline with a new method to upload full textures without rebuilding GPU resources.
- Updated the main application to conditionally log performance diagnostics based on an environment variable.
- Created a performance plan document outlining steps to measure and optimize drawing performance in the iced application.