Merge pull request #11513 from wanikhawar/claude/investigate-issue-e27e90

Windows: count scratch room when opening large files; refusal names the build's limit
This commit is contained in:
FredApps authored and GitHub committed 2026-11-06 17:26:22 +01:00
commit c8e4376ec6
4 files changed
+235 -4

No files matched your search

+106 -1
View File
@@ -23,7 +23,7 @@ pub enum EngineError {
PsdApprovalRequired { name: String, required: u64, soft_limit: u64, hard_limit: u64, available_ram: Option<u64> },
/// 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_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.
@@ -36,6 +36,59 @@ 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,
/// The 32-bit desktop build: its cap, or the computer's memory if lower.
Desktop32Bit,
/// Fixed by the browser build.
FixedWeb,
}
/// This build's memory-limit source, from its target.
fn build_memory_limit() -> BuildMemoryLimit {
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 is_32_bit {
BuildMemoryLimit::Desktop32Bit
} else {
BuildMemoryLimit::Measured
}
}
/// 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".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")
}
}
pub type Result<T> = std::result::Result<T, EngineError>;
impl EngineError {
@@ -70,3 +123,55 @@ impl From<photocraft_raster::AllocationError> 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");
let releases = crate::updates_cmds::RELEASES;
assert_eq!(
build_limit_scope(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}"
)
);
assert_eq!(
build_limit_scope(BuildMemoryLimit::FixedWeb),
format!(" of the browser version of PhotoCraft. To open it, download the desktop app from {releases}")
);
}
#[test]
fn hard_budget_message_names_file_numbers_and_scope() {
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]
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);
}
}
+119 -2
View File
@@ -504,12 +504,60 @@ fn no_room(dir: &Path, cap: Option<u64>) -> 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. 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<u64> {
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<u64> {
None
}
/// 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_disk_prefix(path: &Path) -> Option<PathBuf> {
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")]
@@ -784,6 +832,75 @@ fn log_error(msg: &str) {
mod tests {
use super::*;
#[cfg(windows)]
#[test]
fn verbatim_disk_prefix_is_stripped_only_for_disks() {
let cases: &[(&str, Option<&str>)] = &[
(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),
(r"\\server\share\x", None),
];
for (input, want) in cases {
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<u16> = 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)]
#[test]
fn free_space_finds_the_temp_volume_on_windows() {
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"))]
#[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 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(&temp).unwrap() <= temp_volume_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()));
+4 -1
View File
@@ -822,7 +822,10 @@ 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; 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
+6
View File
@@ -46,6 +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.
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