Image Assets: two layers naming the same file no longer overwrite each other (#6614) (#6668)

* Image Assets: two layers naming the same file no longer overwrite each other (#6614) [WIP]

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Image Assets: two layers naming the same file no longer overwrite each other (#6614)

generate_assets wrote every requested asset, so two layers (or two
default variants) naming one file silently replaced each other and the
result listed the file twice. Each file is now written once per run:
layers go top first, as the Layers panel lists them, the first writer
keeps the name, and later ones are reported in errors. Names are
compared ignoring case, since Logo.png and logo.png are one file on
Windows and macOS.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Zuujis
2026-10-17 11:55:54 -06:00
committed by GitHub
co-authored by Claude Opus 5.5
parent 589941a536
commit 796ee3b103
2 changed files with 57 additions and 5 deletions
+16 -5
View File
@@ -1358,10 +1358,15 @@ fn asset_output_path(dir: &str, relative: &str) -> Result<(String, String)> {
}
/// Exports every asset named by `doc`'s layers into `dir`. Returns (files, errors).
///
/// Each file is written once per run (#6614): when two layers (or two default variants) name the
/// same file, the one higher in the Layers panel keeps it and the other is reported in `errors`.
/// Names are compared ignoring case, as `Logo.png` and `logo.png` are one file on Windows and
/// macOS.
pub fn generate_assets(doc: &Document, dir: &str) -> (Vec<String>, Vec<Value>) {
let dpi = f64::from(doc.resolution_dpi.max(1.0));
let mut defaults: Option<Vec<AssetDefault>> = None;
let mut jobs: Vec<(LayerId, AssetSpec)> = Vec::new();
let mut by_layer: Vec<Vec<(LayerId, AssetSpec)>> = Vec::new();
for (_, _, l) in doc.walk() {
if let Some(d) = parse_defaults(&l.name, dpi) {
defaults = Some(d);
@@ -1370,22 +1375,26 @@ pub fn generate_assets(doc: &Document, dir: &str) -> (Vec<String>, Vec<Value>) {
if matches!(l.content, LayerContent::Adjustment(_)) {
continue;
}
for spec in parse_asset_name(&l.name, dpi) {
jobs.push((l.id, spec));
}
by_layer.push(parse_asset_name(&l.name, dpi).into_iter().map(|spec| (l.id, spec)).collect());
}
// Top layer first, as the Layers panel lists them (`walk` goes bottom to top).
let jobs: Vec<(LayerId, AssetSpec)> = by_layer.into_iter().rev().flatten().collect();
let variants: Vec<AssetDefault> = match defaults {
Some(d) if !d.is_empty() => d,
_ => vec![AssetDefault::default()],
};
let mut files = Vec::new();
let mut errors = Vec::new();
let mut written = std::collections::HashSet::new();
for (id, spec) in &jobs {
for v in &variants {
let r = (|| -> Result<String> {
let (name_stem, ext) = spec.file.rsplit_once('.').unwrap_or((spec.file.as_str(), "png"));
let rel = format!("{}{}{}.{ext}", v.folder, name_stem, v.suffix);
asset_path_components(&rel)?;
let key = asset_path_components(&rel)?.join("/").to_lowercase();
if written.contains(&key) {
return Err(other(format!("{rel}: another layer already writes this file; rename one of the layers")));
}
let mut one = crate::layer_menu_cmds::layer_document(doc, *id)?;
one.metadata = Default::default();
let (w0, h0) = (f64::from(one.size.width), f64::from(one.size.height));
@@ -1413,12 +1422,14 @@ pub fn generate_assets(doc: &Document, dir: &str) -> (Vec<String>, Vec<Value>) {
if spec.format == "webp" {
let (out, reported) = asset_output_path(dir, &rel)?;
crate::file_cmds::save_doc(&wdoc, &out, None)?;
written.insert(key);
return Ok(reported);
}
let buf = photocraft_compose::flatten(&wdoc);
let o = optimize(&buf.px, wdoc.size.width as usize, wdoc.bounds(), &st, None, None, wdoc.resolution_dpi, false)?;
let (out, reported) = asset_output_path(dir, &rel)?;
write_file(&out, &o.bytes)?;
written.insert(key);
Ok(reported)
})();
match r {
+41
View File
@@ -594,6 +594,47 @@ fn image_assets_are_generated_now_and_on_save() {
assert_eq!(decode(&files[0]).dimensions(), (128, 80));
}
/// #6614: two layers, or two default variants, naming one file never overwrite each other: the
/// layer higher in the Layers panel keeps the file, whatever the case of its name, and the
/// others are reported.
#[test]
fn image_assets_never_write_one_file_twice() {
let dir = tmp("asset-collisions");
let mut s = session(8);
s.edit("paint", |doc, active| {
let fmt = doc.pixel_format();
let mut l = Layer::raster("Top", fmt);
l.surface_mut().unwrap().fill_rect(Rect::new(0, 0, 4, 4), &photocraft_raster::from_rgba(&fmt, [0.0, 0.0, 1.0, 1.0]));
let id = doc.insert_above(None, l);
*active = Some(id);
Ok(())
})
.unwrap();
let mut d = (*s.active().unwrap().doc).clone();
let id = |d: &photocraft_doc::Document, name: &str| d.walk().into_iter().find(|(_, _, l)| l.name == name).unwrap().2.id;
let (top, logo) = (id(&d, "Top"), id(&d, "Logo"));
d.layer_mut(top).unwrap().name = "Logo.PNG".into();
d.layer_mut(logo).unwrap().name = "logo.png, logo.png".into();
let (files, errors) = generate_assets(&d, &dir);
assert_eq!(files.iter().map(|p| std::path::PathBuf::from(p.as_str())).collect::<Vec<_>>(), vec![std::path::Path::new(&dir).join("Logo.png")]);
assert_eq!(decode(&files[0]).dimensions(), (4, 4), "the top layer keeps the file");
assert_eq!(errors.len(), 2, "{errors:?}");
assert!(errors.iter().all(|e| e["error"].as_str().unwrap().contains("another layer already writes this file")), "{errors:?}");
assert_eq!(std::fs::read_dir(&dir).unwrap().count(), 1, "one file, not a second one under another case");
// Two default variants that land on one name: the first keeps it.
let dir = tmp("asset-variant-collisions");
d.layer_mut(top).unwrap().name = "icon.png".into();
d.layer_mut(logo).unwrap().name = "Logo".into();
d.layers
.push(Layer::new("default 50% + 200% + 200% @2x", LayerContent::Group(photocraft_doc::Group { children: vec![], expanded: false, artboard: None })));
let (files, errors) = generate_assets(&d, &dir);
let names: Vec<_> = files.iter().map(|p| std::path::Path::new(p).file_name().unwrap().to_string_lossy().into_owned()).collect();
assert_eq!(names, ["icon.png", "icon@2x.png"]);
assert_eq!(decode(&files[0]).dimensions(), (2, 2), "the 50% variant keeps icon.png");
assert_eq!(errors.len(), 1, "{errors:?}");
}
#[test]
fn image_assets_reject_traversal_from_names_and_defaults() {
let base = tmp("asset-path-guard");