From 83f492f4458807556b19b608e86195b2d0061478 Mon Sep 17 00:00:00 2001 From: Khawar Zamman Wani Date: Thu, 5 Nov 2026 22:07:47 +0530 Subject: [PATCH 1/5] Windows scratch room is measured; hard memory refusal names the build's limit fs::canonicalize returns verbatim paths on Windows, whose prefix never equals a disk's plain mount point, so free_space always returned None there. That removed the scratch allowance from import admission (unless a size limit was set) and turned off the low-disk check. The PsdHardBudget message said 'on this machine' even when the limit is the fixed cap of the 32-bit desktop or web build. It now says so and names the build to use. --- crates/engine/src/error.rs | 57 +++++++++++++++++++++++++++++++++++++- crates/raster/src/spill.rs | 54 +++++++++++++++++++++++++++++++++++- docs/development.md | 3 +- 3 files changed, 111 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/error.rs b/crates/engine/src/error.rs index 0abdb6302a..7fc7e4c808 100644 --- a/crates/engine/src/error.rs +++ b/crates/engine/src/error.rs @@ -23,7 +23,7 @@ pub enum EngineError { PsdApprovalRequired { name: String, required: u64, soft_limit: u64, hard_limit: u64, available_ram: Option }, /// The file's estimated memory cannot be admitted within the hard ceiling, which no approval /// can raise. The requirement may equal the ceiling when the estimate saturated. - #[error("cannot open “{name}”: its estimated memory requirement of {required} bytes cannot be admitted within the {limit} byte hard limit on this machine")] + #[error("cannot open “{name}”: its estimated memory requirement of {required} bytes cannot be admitted within the {limit} byte hard limit{scope}", scope = build_limit_scope(build_memory_limit()))] PsdHardBudget { name: String, required: u64, limit: u64 }, /// The file behind an opened handle is not the one whose memory estimate was approved. /// The document is discarded and is not opened. @@ -36,6 +36,38 @@ pub enum EngineError { Cancelled, } +/// Where this build's hard memory limit comes from. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum BuildMemoryLimit { + /// Measured from the computer's memory (64-bit desktop). + Measured, + /// Fixed by the 32-bit desktop build. + FixedDesktop32Bit, + /// Fixed by the browser build. + FixedWeb, +} + +/// The wasm32 check comes first: wasm32 is also 32-bit. +fn build_memory_limit() -> BuildMemoryLimit { + if cfg!(target_arch = "wasm32") { + BuildMemoryLimit::FixedWeb + } else if cfg!(target_pointer_width = "32") { + BuildMemoryLimit::FixedDesktop32Bit + } else { + BuildMemoryLimit::Measured + } +} + +fn build_limit_scope(limit: BuildMemoryLimit) -> &'static str { + match limit { + BuildMemoryLimit::Measured => " on this machine", + BuildMemoryLimit::FixedDesktop32Bit => { + " of this 32-bit build of PhotoCraft, however much memory this computer has; install the 64-bit build to open it" + } + BuildMemoryLimit::FixedWeb => " of the browser version of PhotoCraft; open it in the desktop app", + } +} + pub type Result = std::result::Result; impl EngineError { @@ -70,3 +102,26 @@ impl From for EngineError { Self::Other(format!("not enough memory for this operation ({error}); the document was not changed")) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn hard_budget_scope_text_per_build() { + assert_eq!(build_limit_scope(BuildMemoryLimit::Measured), " on this machine"); + assert_eq!( + build_limit_scope(BuildMemoryLimit::FixedDesktop32Bit), + " of this 32-bit build of PhotoCraft, however much memory this computer has; install the 64-bit build to open it" + ); + assert_eq!(build_limit_scope(BuildMemoryLimit::FixedWeb), " of the browser version of PhotoCraft; open it in the desktop app"); + } + + #[test] + fn hard_budget_message_names_file_numbers_and_scope() { + let text = EngineError::PsdHardBudget { name: "big.psd".into(), required: 9000, limit: 8000 }.to_string(); + for part in ["big.psd", "9000", "8000", build_limit_scope(build_memory_limit())] { + assert!(text.contains(part), "{text}"); + } + } +} diff --git a/crates/raster/src/spill.rs b/crates/raster/src/spill.rs index ac37c85686..9866729950 100644 --- a/crates/raster/src/spill.rs +++ b/crates/raster/src/spill.rs @@ -507,11 +507,32 @@ fn no_room(dir: &Path, cap: Option) -> String { /// prefix. `None` if unknown, which disables only the free-space check, not spilling. #[cfg(not(target_arch = "wasm32"))] fn free_space(dir: &Path) -> Option<(u64, u64)> { - let dir = std::fs::canonicalize(dir).ok()?; + let mut dir = std::fs::canonicalize(dir).ok()?; + // Only on Windows: `\\?\C:\x` is a legal file name on Unix. + if cfg!(windows) + && let Some(plain) = dir.to_str().and_then(without_verbatim_prefix) + { + dir = std::path::PathBuf::from(plain); + } let disks = sysinfo::Disks::new_with_refreshed_list_specifics(sysinfo::DiskRefreshKind::nothing().with_storage()).ok()?; disks.iter().filter(|d| dir.starts_with(d.mount_point())).max_by_key(|d| d.mount_point().as_os_str().len()).map(|d| (d.available_space(), d.total_space())) } +/// The plain form of a Windows verbatim path (`\\?\C:\x` to `C:\x`, `\\?\UNC\s\h\x` to `\\s\h\x`), +/// or `None` to keep the path as is. `canonicalize` returns verbatim paths, whose prefix never +/// compares equal to the plain prefix of a disk's mount point. +#[cfg(not(target_arch = "wasm32"))] +fn without_verbatim_prefix(path: &str) -> Option { + let rest = path.strip_prefix(r"\\?\")?; + if let Some(unc) = rest.strip_prefix(r"UNC\") { + return Some(format!(r"\\{unc}")); + } + match rest.as_bytes().get(..2) { + Some([letter, b':']) if letter.is_ascii_alphabetic() => Some(rest.to_string()), + _ => None, + } +} + #[cfg(target_arch = "wasm32")] fn free_space(_dir: &Path) -> Option<(u64, u64)> { None @@ -784,6 +805,37 @@ fn log_error(msg: &str) { mod tests { use super::*; + #[cfg(not(target_arch = "wasm32"))] + #[test] + fn verbatim_prefix_is_stripped_only_for_disks_and_unc() { + let cases: &[(&str, Option<&str>)] = &[ + (r"\\?\UNC\server\share\x", Some(r"\\server\share\x")), + (r"\\?\C:\x", Some(r"C:\x")), + (r"\\?\c:", Some("c:")), + (r"\\?\Volume{0000-1}\x", None), + (r"C:\x", None), + ("/tmp/x", None), + (r"\\?\", None), + (r"\\?\C", None), + (r"\\?\1:\x", None), + (r"\\?\é:\x", None), + ("", None), + ]; + for (input, want) in cases { + assert_eq!(without_verbatim_prefix(input).as_deref(), *want, "{input}"); + } + } + + // `free_space` returned `None` on Windows before the fix: `canonicalize` gives `\\?\C:\..`, + // which never started with the mount point `C:\`. + #[cfg(windows)] + #[test] + fn free_space_finds_the_temp_volume_on_windows() { + let (available, total) = free_space(&std::env::temp_dir()).unwrap(); + assert!(total > 0); + assert!(available <= total); + } + #[test] fn failed_read_retains_backing_and_blocks_commit_until_repaired() { let dir = std::env::temp_dir().join(format!("photocraft-read-failure-{}", std::process::id())); diff --git a/docs/development.md b/docs/development.md index 1fe3750533..6dfcfea8b6 100644 --- a/docs/development.md +++ b/docs/development.md @@ -819,7 +819,8 @@ physical RAM (excluding swap) and, where measurable, system commit headroom: modification time (a different file with both preserved is not distinguished there). The `file.approveMemory` permission is one-shot and is not journaled or recorded in an action. A refusal above the hard ceiling is the typed `PsdHardBudget` error (name, required, limit), not - a string, and it releases the queued source. + a string, and it releases the queued source. Its message says where the limit comes from: + the computer's memory on 64-bit desktop, or the fixed cap of the 32-bit desktop and web builds. - A layered TIFF (Photoshop layer data in tags 37724/34377) is admitted by the same policy before its composite is decoded. The page's tags are read into the PSD model first, without pixels; then that model's layer/workspace estimate, the decoded composite and its From af2914fd17563cd211eee116dd57ee4858166228 Mon Sep 17 00:00:00 2001 From: Khawar Zamman Wani Date: Thu, 5 Nov 2026 23:22:13 +0530 Subject: [PATCH 2/5] Refusal wording holds when memory sets the 32-bit limit; keep non-UTF-8 scratch paths On a 32-bit build, free RAM or commit headroom can set the limit instead of the cap, and then 'install the 64-bit build' was wrong. The message now mentions the cap and says the 64-bit build may open the file. The verbatim prefix is now removed through path components, so a scratch path with a non-UTF-8 name still finds its disk. The UNC rewrite is gone: sysinfo lists only fixed and removable volumes, so a network path can never match. That limitation is now documented. --- crates/engine/src/error.rs | 41 ++++++++++++++------ crates/raster/src/spill.rs | 78 ++++++++++++++++++++++---------------- docs/development.md | 3 +- docs/scratch-disks.md | 3 ++ 4 files changed, 81 insertions(+), 44 deletions(-) diff --git a/crates/engine/src/error.rs b/crates/engine/src/error.rs index 7fc7e4c808..30b9c495e0 100644 --- a/crates/engine/src/error.rs +++ b/crates/engine/src/error.rs @@ -41,28 +41,34 @@ pub enum EngineError { enum BuildMemoryLimit { /// Measured from the computer's memory (64-bit desktop). Measured, - /// Fixed by the 32-bit desktop build. - FixedDesktop32Bit, + /// The 32-bit desktop build: its cap, or the computer's memory if lower. + Desktop32Bit, /// Fixed by the browser build. FixedWeb, } -/// The wasm32 check comes first: wasm32 is also 32-bit. +/// This build's memory-limit source, from its target. fn build_memory_limit() -> BuildMemoryLimit { - if cfg!(target_arch = "wasm32") { + memory_limit_for(cfg!(target_arch = "wasm32"), cfg!(target_pointer_width = "32")) +} + +/// The limit source for a target. The wasm32 check comes first: wasm32 is also 32-bit. +fn memory_limit_for(is_wasm32: bool, is_32_bit: bool) -> BuildMemoryLimit { + if is_wasm32 { BuildMemoryLimit::FixedWeb - } else if cfg!(target_pointer_width = "32") { - BuildMemoryLimit::FixedDesktop32Bit + } else if is_32_bit { + BuildMemoryLimit::Desktop32Bit } else { BuildMemoryLimit::Measured } } +/// The text after "hard limit" that says where the limit comes from and what to do. fn build_limit_scope(limit: BuildMemoryLimit) -> &'static str { match limit { BuildMemoryLimit::Measured => " on this machine", - BuildMemoryLimit::FixedDesktop32Bit => { - " of this 32-bit build of PhotoCraft, however much memory this computer has; install the 64-bit build to open it" + BuildMemoryLimit::Desktop32Bit => { + " on this machine; 32-bit builds of PhotoCraft can use at most 2 GiB of memory for a file, so the 64-bit build may open it if this computer has more free" } BuildMemoryLimit::FixedWeb => " of the browser version of PhotoCraft; open it in the desktop app", } @@ -111,8 +117,8 @@ mod tests { fn hard_budget_scope_text_per_build() { assert_eq!(build_limit_scope(BuildMemoryLimit::Measured), " on this machine"); assert_eq!( - build_limit_scope(BuildMemoryLimit::FixedDesktop32Bit), - " of this 32-bit build of PhotoCraft, however much memory this computer has; install the 64-bit build to open it" + build_limit_scope(BuildMemoryLimit::Desktop32Bit), + " on this machine; 32-bit builds of PhotoCraft can use at most 2 GiB of memory for a file, so the 64-bit build may open it if this computer has more free" ); assert_eq!(build_limit_scope(BuildMemoryLimit::FixedWeb), " of the browser version of PhotoCraft; open it in the desktop app"); } @@ -120,8 +126,21 @@ mod tests { #[test] fn hard_budget_message_names_file_numbers_and_scope() { let text = EngineError::PsdHardBudget { name: "big.psd".into(), required: 9000, limit: 8000 }.to_string(); - for part in ["big.psd", "9000", "8000", build_limit_scope(build_memory_limit())] { + for part in ["big.psd", "9000", "8000", "hard limit", build_limit_scope(build_memory_limit())] { assert!(text.contains(part), "{text}"); } } + + #[test] + fn memory_limit_follows_the_target() { + assert_eq!(memory_limit_for(true, true), BuildMemoryLimit::FixedWeb); + assert_eq!(memory_limit_for(true, false), BuildMemoryLimit::FixedWeb); + assert_eq!(memory_limit_for(false, true), BuildMemoryLimit::Desktop32Bit); + assert_eq!(memory_limit_for(false, false), BuildMemoryLimit::Measured); + } + + #[test] + fn two_gib_in_the_text_matches_the_cap() { + assert_eq!(photocraft_io::memory::NATIVE_32BIT_PEAK_CAP, 2 << 30); + } } diff --git a/crates/raster/src/spill.rs b/crates/raster/src/spill.rs index 9866729950..c418919f2a 100644 --- a/crates/raster/src/spill.rs +++ b/crates/raster/src/spill.rs @@ -504,33 +504,33 @@ fn no_room(dir: &Path, cap: Option) -> String { } /// (available, total) bytes of the volume holding `dir`: the mount point that is its longest -/// prefix. `None` if unknown, which disables only the free-space check, not spilling. +/// prefix. `None` if unknown, which disables only the free-space check, not spilling. Network +/// scratch folders are not listed as disks, so they report `None`: they earn no scratch allowance +/// at import and the free-space reserve doesn't apply to them, though a Scratch Size Limit still does. #[cfg(not(target_arch = "wasm32"))] fn free_space(dir: &Path) -> Option<(u64, u64)> { - let mut dir = std::fs::canonicalize(dir).ok()?; - // Only on Windows: `\\?\C:\x` is a legal file name on Unix. - if cfg!(windows) - && let Some(plain) = dir.to_str().and_then(without_verbatim_prefix) - { - dir = std::path::PathBuf::from(plain); - } + let dir = std::fs::canonicalize(dir).ok()?; + let dir = without_verbatim_disk_prefix(&dir).unwrap_or(dir); let disks = sysinfo::Disks::new_with_refreshed_list_specifics(sysinfo::DiskRefreshKind::nothing().with_storage()).ok()?; disks.iter().filter(|d| dir.starts_with(d.mount_point())).max_by_key(|d| d.mount_point().as_os_str().len()).map(|d| (d.available_space(), d.total_space())) } -/// The plain form of a Windows verbatim path (`\\?\C:\x` to `C:\x`, `\\?\UNC\s\h\x` to `\\s\h\x`), -/// or `None` to keep the path as is. `canonicalize` returns verbatim paths, whose prefix never -/// compares equal to the plain prefix of a disk's mount point. +/// The plain form of a verbatim disk path (`\\?\C:\x` to `C:\x`), or `None` to keep the path as +/// is. `canonicalize` returns `\\?\C:\x` on Windows, whose prefix never equals a mount point's +/// plain `C:\`. Unix paths never have a prefix component, so this is `None` there. #[cfg(not(target_arch = "wasm32"))] -fn without_verbatim_prefix(path: &str) -> Option { - let rest = path.strip_prefix(r"\\?\")?; - if let Some(unc) = rest.strip_prefix(r"UNC\") { - return Some(format!(r"\\{unc}")); - } - match rest.as_bytes().get(..2) { - Some([letter, b':']) if letter.is_ascii_alphabetic() => Some(rest.to_string()), - _ => None, - } +fn without_verbatim_disk_prefix(path: &Path) -> Option { + use std::path::{Component, Prefix}; + let mut components = path.components(); + let Some(Component::Prefix(prefix)) = components.next() else { + return None; + }; + let Prefix::VerbatimDisk(letter) = prefix.kind() else { + return None; + }; + let mut out = PathBuf::from(format!("{}:", char::from(letter))); + out.extend(components); + Some(out) } #[cfg(target_arch = "wasm32")] @@ -805,27 +805,41 @@ fn log_error(msg: &str) { mod tests { use super::*; - #[cfg(not(target_arch = "wasm32"))] + #[cfg(windows)] #[test] - fn verbatim_prefix_is_stripped_only_for_disks_and_unc() { + fn verbatim_disk_prefix_is_stripped_only_for_disks() { let cases: &[(&str, Option<&str>)] = &[ - (r"\\?\UNC\server\share\x", Some(r"\\server\share\x")), - (r"\\?\C:\x", Some(r"C:\x")), - (r"\\?\c:", Some("c:")), + (r"\\?\C:\x\y", Some(r"C:\x\y")), + (r"\\?\C:\", Some(r"C:\")), + (r"\\?\UNC\server\share\x", None), (r"\\?\Volume{0000-1}\x", None), (r"C:\x", None), - ("/tmp/x", None), - (r"\\?\", None), - (r"\\?\C", None), - (r"\\?\1:\x", None), - (r"\\?\é:\x", None), - ("", None), + (r"\\server\share\x", None), ]; for (input, want) in cases { - assert_eq!(without_verbatim_prefix(input).as_deref(), *want, "{input}"); + assert_eq!(without_verbatim_disk_prefix(Path::new(input)).as_deref(), want.map(Path::new), "{input}"); } } + #[cfg(windows)] + #[test] + fn verbatim_disk_prefix_keeps_non_utf8_names() { + use std::os::windows::ffi::OsStringExt; + let mut wide: Vec = r"\\?\C:\a".encode_utf16().collect(); + wide.push(0xD800); + let path = PathBuf::from(std::ffi::OsString::from_wide(&wide)); + let plain = without_verbatim_disk_prefix(&path).unwrap(); + assert!(plain.starts_with(r"C:\")); + assert!(plain.to_str().is_none()); + } + + #[cfg(not(any(windows, target_arch = "wasm32")))] + #[test] + fn verbatim_disk_prefix_is_none_on_unix() { + assert_eq!(without_verbatim_disk_prefix(Path::new("/tmp/x")), None); + assert_eq!(without_verbatim_disk_prefix(Path::new(r"\\?\C:\x")), None); + } + // `free_space` returned `None` on Windows before the fix: `canonicalize` gives `\\?\C:\..`, // which never started with the mount point `C:\`. #[cfg(windows)] diff --git a/docs/development.md b/docs/development.md index 6dfcfea8b6..1771ab5b31 100644 --- a/docs/development.md +++ b/docs/development.md @@ -820,7 +820,8 @@ physical RAM (excluding swap) and, where measurable, system commit headroom: `file.approveMemory` permission is one-shot and is not journaled or recorded in an action. A refusal above the hard ceiling is the typed `PsdHardBudget` error (name, required, limit), not a string, and it releases the queued source. Its message says where the limit comes from: - the computer's memory on 64-bit desktop, or the fixed cap of the 32-bit desktop and web builds. + the computer's memory on 64-bit desktop; the web build names its fixed cap; on 32-bit desktop the + limit may be either, so the message mentions the 2 GiB cap and that the 64-bit build may open the file. - A layered TIFF (Photoshop layer data in tags 37724/34377) is admitted by the same policy before its composite is decoded. The page's tags are read into the PSD model first, without pixels; then that model's layer/workspace estimate, the decoded composite and its diff --git a/docs/scratch-disks.md b/docs/scratch-disks.md index acc784d242..5fc440b7d4 100644 --- a/docs/scratch-disks.md +++ b/docs/scratch-disks.md @@ -46,6 +46,9 @@ spilling resumes on its own once the directory is usable and has room, and chang preferences retries at once. Preferences → Scratch Disks → Scratch Size Limit (`scratchDisks.sizeLimitMb`, 0 = no limit; `spill::set_size_cap`) caps the scratch file: reaching it pauses spilling the same way, and the free-space reserve applies regardless. +Network scratch folders are not listed as disks, so their free space isn't measured: they earn +no scratch allowance when admitting large imports and the free-space reserve doesn't apply to +them (the Scratch Size Limit still does). A failed read retains its backing extent and blocks editing, saving and exporting of the document that holds the tile until it is readable again; other open documents keep working. Each commit checks the health of its own document (before and after an edit, the document a From eecc821ee824ede9ae510ffd7c162d0aece65db4 Mon Sep 17 00:00:00 2001 From: Khawar Zamman Wani Date: Fri, 6 Nov 2026 00:21:07 +0530 Subject: [PATCH 3/5] Scratch room uses what the user can write; a failed volume query is unknown sysinfo reports a Windows volume's free bytes, which a per-user disk quota can exceed, so admission could promise room the user can't write. free_space now asks GetDiskFreeSpaceEx for the bytes available to the caller. sysinfo also keeps a volume whose space query failed, with both numbers zero, which read as an empty disk and would pause spilling. A zero total now counts as unknown. Docs: describe network scratch per platform and the size limit's import allowance. --- crates/raster/src/spill.rs | 53 +++++++++++++++++++++++++++++++++++--- docs/scratch-disks.md | 9 ++++--- 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/crates/raster/src/spill.rs b/crates/raster/src/spill.rs index c418919f2a..422cd98357 100644 --- a/crates/raster/src/spill.rs +++ b/crates/raster/src/spill.rs @@ -504,15 +504,42 @@ fn no_room(dir: &Path, cap: Option) -> String { } /// (available, total) bytes of the volume holding `dir`: the mount point that is its longest -/// prefix. `None` if unknown, which disables only the free-space check, not spilling. Network -/// scratch folders are not listed as disks, so they report `None`: they earn no scratch allowance -/// at import and the free-space reserve doesn't apply to them, though a Scratch Size Limit still does. +/// prefix. `None` if unknown, which disables only the free-space check, not spilling. On Windows a +/// network scratch folder isn't listed, so it reports `None` (no free-space reserve; import +/// allowance only up to a Scratch Size Limit, if one is set). On Linux an unlisted network mount +/// matches its listed parent mount, such as `/`, and uses that volume's numbers. #[cfg(not(target_arch = "wasm32"))] fn free_space(dir: &Path) -> Option<(u64, u64)> { let dir = std::fs::canonicalize(dir).ok()?; let dir = without_verbatim_disk_prefix(&dir).unwrap_or(dir); let disks = sysinfo::Disks::new_with_refreshed_list_specifics(sysinfo::DiskRefreshKind::nothing().with_storage()).ok()?; - disks.iter().filter(|d| dir.starts_with(d.mount_point())).max_by_key(|d| d.mount_point().as_os_str().len()).map(|d| (d.available_space(), d.total_space())) + disks + .iter() + .filter(|d| dir.starts_with(d.mount_point())) + .max_by_key(|d| d.mount_point().as_os_str().len()) + .and_then(|d| known_space(available_to_caller(d.mount_point()).unwrap_or_else(|| d.available_space()), d.total_space())) +} + +/// sysinfo keeps a volume whose space query failed, with both numbers zero; treating that as an +/// empty disk would pause spilling, so it counts as unknown. +#[cfg(not(target_arch = "wasm32"))] +fn known_space(available: u64, total: u64) -> Option<(u64, u64)> { + (total != 0).then_some((available, total)) +} + +/// Bytes the current user can write to the volume at `mount`, which a per-user disk quota can make +/// less than the volume's free space that sysinfo reports. `None` if the query fails. +#[cfg(windows)] +fn available_to_caller(mount: &Path) -> Option { + let mut available = 0; + winsafe::GetDiskFreeSpaceEx(Some(mount.to_str()?), Some(&mut available), None, None).ok()?; + Some(available) +} + +/// Elsewhere sysinfo's figure already is what the user can write. +#[cfg(all(not(windows), not(target_arch = "wasm32")))] +fn available_to_caller(_mount: &Path) -> Option { + None } /// The plain form of a verbatim disk path (`\\?\C:\x` to `C:\x`), or `None` to keep the path as @@ -850,6 +877,24 @@ mod tests { assert!(available <= total); } + #[cfg(not(target_arch = "wasm32"))] + #[test] + fn a_volume_with_no_total_is_unknown() { + assert_eq!(known_space(0, 0), None); + assert_eq!(known_space(5, 0), None); + assert_eq!(known_space(0, 10), Some((0, 10))); + assert_eq!(known_space(5, 10), Some((5, 10))); + } + + #[cfg(windows)] + #[test] + fn available_to_caller_reads_the_temp_volume() { + let temp = std::fs::canonicalize(std::env::temp_dir()).unwrap(); + let temp = without_verbatim_disk_prefix(&temp).unwrap_or(temp); + let root = temp.ancestors().last().unwrap(); + assert!(available_to_caller(root).is_some()); + } + #[test] fn failed_read_retains_backing_and_blocks_commit_until_repaired() { let dir = std::env::temp_dir().join(format!("photocraft-read-failure-{}", std::process::id())); diff --git a/docs/scratch-disks.md b/docs/scratch-disks.md index 5fc440b7d4..0d65411fce 100644 --- a/docs/scratch-disks.md +++ b/docs/scratch-disks.md @@ -46,9 +46,12 @@ spilling resumes on its own once the directory is usable and has room, and chang preferences retries at once. Preferences → Scratch Disks → Scratch Size Limit (`scratchDisks.sizeLimitMb`, 0 = no limit; `spill::set_size_cap`) caps the scratch file: reaching it pauses spilling the same way, and the free-space reserve applies regardless. -Network scratch folders are not listed as disks, so their free space isn't measured: they earn -no scratch allowance when admitting large imports and the free-space reserve doesn't apply to -them (the Scratch Size Limit still does). +On Windows, network scratch folders aren't listed as disks, so their free space isn't +measured: the free-space reserve doesn't apply to them, and they earn a scratch allowance for +large imports only up to the Scratch Size Limit, if one is set. On Linux, an unlisted network +mount (NFS or CIFS) counts the free space of the listed mount above it, such as `/`, for both +the allowance and the reserve. With neither free space nor a size limit known, imports get no +scratch allowance. A failed read retains its backing extent and blocks editing, saving and exporting of the document that holds the tile until it is readable again; other open documents keep working. Each commit checks the health of its own document (before and after an edit, the document a From 349a03b6b18b0bcee64090f6faa9fc321960fc34 Mon Sep 17 00:00:00 2001 From: Khawar Zamman Wani Date: Fri, 6 Nov 2026 14:01:03 +0530 Subject: [PATCH 4/5] Refusal text states the lower limit, links the download and shows readable sizes The 32-bit message ended mid-phrase. It now says the cap applies or less if less memory is free, and separates the cause from the next step. Windows scratch tests check the returned numbers belong to the temp directory's volume. --- crates/engine/src/error.rs | 55 +++++++++++++++++++++++++++++--------- crates/raster/src/spill.rs | 11 ++++++-- docs/development.md | 1 + 3 files changed, 53 insertions(+), 14 deletions(-) diff --git a/crates/engine/src/error.rs b/crates/engine/src/error.rs index 30b9c495e0..e1361ad0aa 100644 --- a/crates/engine/src/error.rs +++ b/crates/engine/src/error.rs @@ -23,7 +23,7 @@ pub enum EngineError { PsdApprovalRequired { name: String, required: u64, soft_limit: u64, hard_limit: u64, available_ram: Option }, /// The file's estimated memory cannot be admitted within the hard ceiling, which no approval /// can raise. The requirement may equal the ceiling when the estimate saturated. - #[error("cannot open “{name}”: its estimated memory requirement of {required} bytes cannot be admitted within the {limit} byte hard limit{scope}", scope = build_limit_scope(build_memory_limit()))] + #[error("cannot open “{name}”: its estimated memory requirement of {required_text} ({required} bytes) cannot be admitted within the {limit_text} ({limit} byte) hard limit{scope}", required_text = bytes_text(.required), limit_text = bytes_text(.limit), scope = build_limit_scope(build_memory_limit()))] PsdHardBudget { name: String, required: u64, limit: u64 }, /// The file behind an opened handle is not the one whose memory estimate was approved. /// The document is discarded and is not opened. @@ -63,14 +63,29 @@ fn memory_limit_for(is_wasm32: bool, is_32_bit: bool) -> BuildMemoryLimit { } } -/// The text after "hard limit" that says where the limit comes from and what to do. -fn build_limit_scope(limit: BuildMemoryLimit) -> &'static str { +/// The text after "hard limit": where the limit comes from and, where another build could open +/// the file, where to get it. Its sentences stand apart so the action isn't lost in the cause. +fn build_limit_scope(limit: BuildMemoryLimit) -> String { + let releases = crate::updates_cmds::RELEASES; match limit { - BuildMemoryLimit::Measured => " on this machine", - BuildMemoryLimit::Desktop32Bit => { - " on this machine; 32-bit builds of PhotoCraft can use at most 2 GiB of memory for a file, so the 64-bit build may open it if this computer has more free" - } - BuildMemoryLimit::FixedWeb => " of the browser version of PhotoCraft; open it in the desktop app", + BuildMemoryLimit::Measured => " on this machine".to_owned(), + BuildMemoryLimit::Desktop32Bit => format!( + " on this machine. This 32-bit build of PhotoCraft can use at most 2 GiB of memory for a file, or less if the computer has less memory free. The 64-bit build may be able to open it: download it from {releases}" + ), + BuildMemoryLimit::FixedWeb => format!(" of the browser version of PhotoCraft. To open it, download the desktop app from {releases}"), + } +} + +/// A size for people, next to the exact byte count: "21.5 GiB", "900 MiB", "17 B". +fn bytes_text(bytes: &u64) -> String { + const GIB: u64 = 1 << 30; + const MIB: u64 = 1 << 20; + if *bytes >= GIB { + format!("{:.1} GiB", *bytes as f64 / GIB as f64) + } else if *bytes >= MIB { + format!("{} MiB", bytes / MIB) + } else { + format!("{bytes} B") } } @@ -116,19 +131,35 @@ mod tests { #[test] fn hard_budget_scope_text_per_build() { assert_eq!(build_limit_scope(BuildMemoryLimit::Measured), " on this machine"); + let releases = crate::updates_cmds::RELEASES; assert_eq!( build_limit_scope(BuildMemoryLimit::Desktop32Bit), - " on this machine; 32-bit builds of PhotoCraft can use at most 2 GiB of memory for a file, so the 64-bit build may open it if this computer has more free" + format!( + " on this machine. This 32-bit build of PhotoCraft can use at most 2 GiB of memory for a file, or less if the computer has less memory free. The 64-bit build may be able to open it: download it from {releases}" + ) + ); + assert_eq!( + build_limit_scope(BuildMemoryLimit::FixedWeb), + format!(" of the browser version of PhotoCraft. To open it, download the desktop app from {releases}") ); - assert_eq!(build_limit_scope(BuildMemoryLimit::FixedWeb), " of the browser version of PhotoCraft; open it in the desktop app"); } #[test] fn hard_budget_message_names_file_numbers_and_scope() { - let text = EngineError::PsdHardBudget { name: "big.psd".into(), required: 9000, limit: 8000 }.to_string(); - for part in ["big.psd", "9000", "8000", "hard limit", build_limit_scope(build_memory_limit())] { + let text = EngineError::PsdHardBudget { name: "big.psd".into(), required: 23_049_071_616, limit: 2_147_483_648 }.to_string(); + for part in ["big.psd", "21.5 GiB", "23049071616 bytes", "2.0 GiB", "2147483648 byte", "hard limit"] { assert!(text.contains(part), "{text}"); } + assert!(text.ends_with(&build_limit_scope(build_memory_limit())), "{text}"); + } + + #[test] + fn bytes_text_picks_a_readable_unit() { + assert_eq!(bytes_text(&17), "17 B"); + assert_eq!(bytes_text(&(900 << 20)), "900 MiB"); + assert_eq!(bytes_text(&(2 << 30)), "2.0 GiB"); + assert_eq!(bytes_text(&23_049_071_616), "21.5 GiB"); + assert!(bytes_text(&u64::MAX).ends_with(" GiB")); } #[test] diff --git a/crates/raster/src/spill.rs b/crates/raster/src/spill.rs index 422cd98357..c72d31999e 100644 --- a/crates/raster/src/spill.rs +++ b/crates/raster/src/spill.rs @@ -872,9 +872,13 @@ mod tests { #[cfg(windows)] #[test] fn free_space_finds_the_temp_volume_on_windows() { - let (available, total) = free_space(&std::env::temp_dir()).unwrap(); + let temp = std::env::temp_dir(); + let (available, total) = free_space(&temp).unwrap(); assert!(total > 0); assert!(available <= total); + let mut temp_volume_total = 0; + winsafe::GetDiskFreeSpaceEx(Some(temp.to_str().unwrap()), None, Some(&mut temp_volume_total), None).unwrap(); + assert_eq!(total, temp_volume_total); } #[cfg(not(target_arch = "wasm32"))] @@ -892,7 +896,10 @@ mod tests { let temp = std::fs::canonicalize(std::env::temp_dir()).unwrap(); let temp = without_verbatim_disk_prefix(&temp).unwrap_or(temp); let root = temp.ancestors().last().unwrap(); - assert!(available_to_caller(root).is_some()); + let mut temp_volume_total = 0; + winsafe::GetDiskFreeSpaceEx(Some(temp.to_str().unwrap()), None, Some(&mut temp_volume_total), None).unwrap(); + assert!(temp_volume_total > 0); + assert!(available_to_caller(root).unwrap() <= temp_volume_total); } #[test] diff --git a/docs/development.md b/docs/development.md index b6e7a1e9e9..bbfabf4340 100644 --- a/docs/development.md +++ b/docs/development.md @@ -825,6 +825,7 @@ physical RAM (excluding swap) and, where measurable, system commit headroom: a string, and it releases the queued source. Its message says where the limit comes from: the computer's memory on 64-bit desktop; the web build names its fixed cap; on 32-bit desktop the limit may be either, so the message mentions the 2 GiB cap and that the 64-bit build may open the file. + Those two also link the releases page. Sizes appear in GiB or MiB next to the exact bytes. - A layered TIFF (Photoshop layer data in tags 37724/34377) is admitted by the same policy before its composite is decoded. The page's tags are read into the PSD model first, without pixels; then that model's layer/workspace estimate, the decoded composite and its From 7fc39a61c10049b90afd7284bc90ebe9ded1b246 Mon Sep 17 00:00:00 2001 From: Khawar Zamman Wani Date: Fri, 6 Nov 2026 14:57:19 +0530 Subject: [PATCH 5/5] Caller-space test queries the temp directory, not its drive root A temp folder on a volume mounted without a drive letter has a root on a different volume, so the comparison could fail on correct code. --- crates/raster/src/spill.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/raster/src/spill.rs b/crates/raster/src/spill.rs index c72d31999e..af658cc275 100644 --- a/crates/raster/src/spill.rs +++ b/crates/raster/src/spill.rs @@ -895,11 +895,10 @@ mod tests { fn available_to_caller_reads_the_temp_volume() { let temp = std::fs::canonicalize(std::env::temp_dir()).unwrap(); let temp = without_verbatim_disk_prefix(&temp).unwrap_or(temp); - let root = temp.ancestors().last().unwrap(); let mut temp_volume_total = 0; winsafe::GetDiskFreeSpaceEx(Some(temp.to_str().unwrap()), None, Some(&mut temp_volume_total), None).unwrap(); assert!(temp_volume_total > 0); - assert!(available_to_caller(root).unwrap() <= temp_volume_total); + assert!(available_to_caller(&temp).unwrap() <= temp_volume_total); } #[test]