diff --git a/.kilo/plans/1784862272150-kra-format-fix.md b/.kilo/plans/1784862272150-kra-format-fix.md new file mode 100644 index 0000000..b938066 --- /dev/null +++ b/.kilo/plans/1784862272150-kra-format-fix.md @@ -0,0 +1,163 @@ +# KRA Format Fix Plan + +## Root Causes (confirmed by analysis) + +### 1. VERSION Header Mismatch — Krita Crash + Self-Import Failures +**File:** `hcie-kra/src/kra_saver.rs` lines 324–329, 409–415 + +The saver writes `VERSION 2\n` in the VERS tile header, but the compressed payload uses +version byte `1` (meaning **no** delta decoding). Krita's VERS parser sees VERSION 2 and +applies delta decoding after decompression, producing garbage or a crash. + +Our own importer (`hcie-kra/src/lib.rs` line 158) also checks `if version >= 2` and +would apply delta decoding, producing wrong pixel data. + +**Fix:** Change both `encode_layer_vers` and `encode_mask_vers` to write `VERSION 1` +instead of `VERSION 2`. + +### 2. Fake "LZF" Compression — Bloated Files + Non-Standard +**File:** `hcie-kra/src/kra_saver.rs` lines 308–316, 394–403 + +The "LZF" compression is a custom RLE scheme that adds 513 bytes of overhead per tile +(1 version byte + 512 control bytes), making files **larger** than uncompressed. A full +1920×1080 layer (510 tiles) goes from 8.29 MB to 8.62 MB (+4%). The format happens to +look like real LZF literal runs, so our `lzf_decompress` function processes it correctly, +but a real LZF decoder would fail on tiles requiring back-references. + +**Fix:** Store tiles as RAW (uncompressed). Remove the fake compression logic and write +raw planar pixel data. Label tiles as `RAW` instead of `LZF` in the tile header. The +importer already handles RAW tiles correctly (lib.rs line 166–168). + +### 3. Invalid UUID Format — Krita Rejects Our XML +**File:** `hcie-kra/src/kra_saver.rs` line 240 + +UUIDs are generated from a single `u64` (8 bytes) and formatted as 32 hex digits with +leading zeros: `{00000000000000003aeba3f7cf423ad9}`. Proper UUIDs are 128 bits (16 bytes) +with dashed formatting: `{3aeba3f7-cf42-3ad9-…}`. Krita may validate UUID format and +reject our non-standard forms. + +**Fix:** Generate proper 128-bit random UUIDs using two `rand::random::()` calls +and format with dashes per standard UUID layout. + +### 4. Color Space / Bit Depth Ignored — Wrong Colors on Import +**File:** `hcie-kra/src/lib.rs` lines 647–698 + +The importer reads `colorspacename`, `channeldepth`, and `profile` attributes from the +`` and `` tags but ignores them. If the source file uses a non-RGBA color +space (e.g., CMYK, Grayscale) or 16-bit channel depth, the raw pixel bytes are +misinterpreted, producing wrong colors. + +**Fix:** At minimum, validate on import that: +- `colorspacename` is `"RGBA"` +- `channeldepth` is `"U8"` or `"U16"` (for U16, convert to U8 via shifting) +- Return an error if the format is unsupported + +### 5. Silent Layer Failures — Data Loss Without Notice +**File:** `hcie-kra/src/lib.rs` lines 790, 808, 814, 837 + +When a layer file can't be found, VERS parsing fails, or image decoding fails, the +layer is silently skipped or turned into a transparent layer. The user has no indication +that data was lost. + +**Fix:** Add `log::warn!` calls at each failure point, including the layer name and +reason for failure. + +### 6. Missing `layers.xml` Fallback +**File:** `hcie-kra/src/lib.rs` line 626 + +The main importer only tries `maindoc.xml`. Some Krita variants write `layers.xml` +instead. The v2 loader (`custom_loaders_kra_v2.rs`) has this fallback but it's separate. + +**Fix:** Add `layers.xml` as a secondary attempt in `try_import_krita_maindoc`. + +### 7. Inconsistent `DATA` Line Format +**File:** `hcie-kra/src/kra_saver.rs` lines 329 vs 415 + +`encode_layer_vers` writes `DATA {tiles.len()}\n` but `encode_mask_vers` writes `DATA\n` +(no count). The parser handles both, but it's inconsistent. + +**Fix:** Make both use `DATA\n` (no count) — the parser ignores the count anyway, and +removing it avoids an extra format requirement. + +--- + +## Implementation Tasks + +### Task A: Fix VERS header and compression (`kra_saver.rs`) +1. Change `writeln!(output, "VERSION 2")` → `writeln!(output, "VERSION 1")` in `encode_layer_vers` +2. Change `b"VERSION 2\n"` → `b"VERSION 1\n"` in `encode_mask_vers` +3. Remove the fake LZF compression loop in `encode_layer_vers` (lines 308–316) +4. Replace with writing raw planar pixel data directly (no version byte, no control bytes) +5. Update tile header tag from `"LZF"` to `"RAW"` (line 333) +6. Repeat steps 3–5 for `encode_mask_vers` (lines 394–403) + +### Task B: Fix UUID format (`kra_saver.rs`) +7. Replace `let uuid = format!("{{{:032x}}}", layer.id)` with proper 128-bit UUID: + ```rust + let high = rand::random::(); + let low = rand::random::(); + let uuid = format!( + "{{{:08x}-{:04x}-{:04x}-{:04x}-{:012x}}}", + (high >> 32) as u32, + (high >> 16) as u16 & 0xFFFF, + high as u16, + (low >> 48) as u16, + low & 0x0000_FFFF_FFFF_FFFF + ); + ``` +8. Apply same fix to mask UUID generation on line 257 + +### Task C: Add color space validation (`lib.rs`) +9. In `try_import_krita_maindoc`, after reading width/height from ``: + - Read `colorspacename` and `channeldepth` attributes + - If `colorspacename != "RGBA"`, return error: "Unsupported color space: {name}" + - If `channeldepth != "U8"`, return error: "Unsupported channel depth: {depth}" + - (Future: add U16→U8 conversion) + +### Task D: Add error logging (`lib.rs`) +10. Add `log::warn!` before each `continue` / silent fallback: + - Line 790 (layer file not found): warn with layer name + - Line 808 (SVG rasterization failed): warn with layer name + - Line 814 (VERS parse failed): warn with layer name + - Line 837 (image load failed): warn with layer name + +### Task E: Add `layers.xml` fallback (`lib.rs`) +11. In `try_import_krita_maindoc`, after the `maindoc.xml` attempt fails, try + `archive.by_name("layers.xml")` before returning `Ok(vec![])`. + +### Task F: Normalize `DATA` line format (`kra_saver.rs`) +12. Change line 329: `writeln!(output, "DATA {}", tiles.len())` → `writeln!(output, "DATA")` +13. The mask encoder already uses `b"DATA\n"` — leave unchanged. + +--- + +## Files to Modify + +| File | Changes | +|------|---------| +| `hcie-kra/src/kra_saver.rs` | Tasks A, B, F | +| `hcie-kra/src/lib.rs` | Tasks C, D, E | + +--- + +## Validation + +After implementing, verify with: + +1. **Roundtrip test:** Save a multi-layer document as KRA, then import it back. Compare + pixels and layer names against the original. + +2. **Krita compatibility:** Open the exported KRA in Krita ≥ 5.x. Verify it opens without + crash and displays correct colors. Also open the PSD→KRA exported file and verify it + matches the mergedimage.png preview. + +3. **Existing test suite:** Run `cargo test -p hcie-kra` and + `cargo test -p hcie-engine-api --test visual_regression` to ensure no regressions. + +4. **Edge cases:** + - Empty layers (all-transparent pixels) + - Layers with masks + - Group layers + - Layer names containing special characters (`&`, `<`, `>`, Unicode) + - Error on unsupported color space (e.g., CMYK, Grayscale) + - Error on unsupported bit depth (e.g., U16, F16) diff --git a/hcie-kra/src/kra_saver.rs b/hcie-kra/src/kra_saver.rs index c1cf48e..ff95002 100644 --- a/hcie-kra/src/kra_saver.rs +++ b/hcie-kra/src/kra_saver.rs @@ -1,5 +1,6 @@ use hcie_protocol::{Layer, LayerData, LayerType, VectorShape}; use hcie_protocol::BlendMode; +use rand; use std::io::{Cursor, Write}; use std::path::Path; @@ -231,13 +232,22 @@ fn generate_maindoc(layers: &[Layer], w: u32, h: u32, name: &str) -> String { let node_type = match layer.layer_type { LayerType::Raster | LayerType::Mask => "paintlayer", LayerType::Vector | LayerType::Text => "shapelayer", - LayerType::Group => "paintlayer", + LayerType::Group => "grouplayer", }; let visible = if layer.visible { "1" } else { "0" }; let opacity = (layer.opacity * 255.0) as u8; let filename = format!("layer{}", i + 1); - let uuid = format!("{{{:032x}}}", layer.id); + let uuid_high = rand::random::(); + let uuid_low = rand::random::(); + let uuid = format!( + "{{{:08x}-{:04x}-{:04x}-{:04x}-{:012x}}}", + (uuid_high >> 32) as u32, + (uuid_high >> 16) as u16 & 0xFFFF, + uuid_high as u16, + (uuid_low >> 48) as u16, + uuid_low & 0x0000_FFFF_FFFF_FFFF + ); let compositeop = blend_mode_to_krita_compositeop(layer.blend_mode); let escaped_name = layer.name @@ -254,9 +264,18 @@ fn generate_maindoc(layers: &[Layer], w: u32, h: u32, name: &str) -> String { if layer.mask_pixels.is_some() { xml.push_str(" \n"); let mask_filename = format!("{}_mask", filename); - let mask_uuid = format!("{{{:032x}}}", layer.id.wrapping_add(1)); // pseudo uuid + let mask_high = rand::random::(); + let mask_low = rand::random::(); + let mask_uuid = format!( + "{{{:08x}-{:04x}-{:04x}-{:04x}-{:012x}}}", + (mask_high >> 32) as u32, + (mask_high >> 16) as u16 & 0xFFFF, + mask_high as u16, + (mask_low >> 48) as u16, + mask_low & 0x0000_FFFF_FFFF_FFFF + ); xml.push_str(&format!( - " \n", + " \n", mask_filename, mask_uuid )); xml.push_str(" \n"); @@ -293,10 +312,11 @@ fn encode_layer_vers(layer: &Layer) -> Vec { let idx = (ly as usize * layer.width as usize + lx as usize) * 4; let tidx = y as usize * tw as usize + x as usize; - tile_raw[tidx] = layer.pixels[idx + 2]; - tile_raw[tidx + 1 * plane_size] = layer.pixels[idx + 1]; - tile_raw[tidx + 2 * plane_size] = layer.pixels[idx]; - tile_raw[tidx + 3 * plane_size] = layer.pixels[idx + 3]; + // Krita planar order: B, G, R, A + tile_raw[tidx] = layer.pixels[idx + 2]; // B + tile_raw[tidx + 1 * plane_size] = layer.pixels[idx + 1]; // G + tile_raw[tidx + 2 * plane_size] = layer.pixels[idx]; // R + tile_raw[tidx + 3 * plane_size] = layer.pixels[idx + 3]; // A if layer.pixels[idx + 3] > 0 { has_content = true; @@ -306,15 +326,14 @@ fn encode_layer_vers(layer: &Layer) -> Vec { } if has_content { - let mut compressed = vec![1u8]; - let mut i = 0; - while i < tile_raw.len() { - let chunk = (tile_raw.len() - i).min(32); - compressed.push((chunk - 1) as u8); - compressed.extend_from_slice(&tile_raw[i..i + chunk]); - i += chunk; - } - tiles.push((tx, ty, compressed)); + // Krita VERSION 2 tile format: + // payload = [version_byte = 1][lzf_compressed_planar_data] + // version_byte < 2 → no delta-decode on read (straightforward LZF). + let compressed = lzf_compress(&tile_raw); + let mut payload = Vec::with_capacity(1 + compressed.len()); + payload.push(1u8); // LZF version byte + payload.extend_from_slice(&compressed); + tiles.push((tx, ty, payload)); } tx += tw; } @@ -329,15 +348,93 @@ fn encode_layer_vers(layer: &Layer) -> Vec { writeln!(output, "DATA {}", tiles.len()).unwrap(); tiles.sort_by_key(|(x, y, _)| (*y, *x)); - for (tx, ty, tile_data) in tiles { - let header = format!("{},{},LZF,{}\n", tx, ty, tile_data.len()); + for (tx, ty, payload) in tiles { + let header = format!("{},{},LZF,{}\n", tx, ty, payload.len()); output.extend_from_slice(header.as_bytes()); - output.extend_from_slice(&tile_data); + output.extend_from_slice(&payload); } output } +/// Purpose: LZF compress a byte slice for use as Krita tile payload. +/// Logic: Simple LZF implementation — hash-table based back-reference search. +/// Each control byte describes either a literal run (ctrl < 32) or a +/// back-reference (ctrl >= 32). This matches the format expected by the +/// lzf_decompress reader in lib.rs. +fn lzf_compress(input: &[u8]) -> Vec { + // Worst-case output: all literals → 1 control byte per 32 data bytes. + let mut output = Vec::with_capacity(input.len() + input.len() / 20 + 32); + let mut hash_table: Vec = vec![usize::MAX; 1 << 16]; + + let mut ip = 0usize; + let mut lit_start = 0usize; + + // Flush pending literal bytes [lit_start .. lit_end) into output. + let flush_literals = |out: &mut Vec, data: &[u8], from: usize, to: usize| { + let mut i = from; + while i < to { + let run = (to - i).min(32); + out.push((run - 1) as u8); // ctrl: literal run length − 1 + out.extend_from_slice(&data[i..i + run]); + i += run; + } + }; + + while ip + 2 < input.len() { + // 16-bit hash of three bytes at ip. + let h = ((input[ip] as u32) + .wrapping_mul(2654435761) + ^ (input[ip + 1] as u32) + .wrapping_mul(2246822519) + ^ (input[ip + 2] as u32)) + as usize + & 0xFFFF; + + let ref_pos = hash_table[h]; + hash_table[h] = ip; + + // Check if back-reference is valid (within last 8191 bytes, matches >= 3 bytes). + let max_len = (input.len() - ip).min(264); + if ref_pos != usize::MAX + && ip > ref_pos + && (ip - ref_pos) <= 8191 + && ref_pos + 2 < input.len() + && input[ref_pos] == input[ip] + && input[ref_pos + 1] == input[ip + 1] + && input[ref_pos + 2] == input[ip + 2] + { + // Count match length (capped at 264). + let ofs = ip - ref_pos - 1; + let mut mlen = 3usize; + while mlen < max_len && input[ref_pos + mlen] == input[ip + mlen] { + mlen += 1; + } + + // Flush literals accumulated before this back-reference. + flush_literals(&mut output, input, lit_start, ip); + lit_start = ip + mlen; + ip += mlen; + + // Encode back-reference. + let len_code = mlen - 2; // stored as (len − 2) + if len_code < 7 { + output.push(((len_code << 5) | (ofs >> 8)) as u8); + } else { + output.push(((7 << 5) | (ofs >> 8)) as u8); + output.push((len_code - 7) as u8); + } + output.push((ofs & 0xFF) as u8); + } else { + ip += 1; + } + } + + // Flush any remaining literal bytes. + flush_literals(&mut output, input, lit_start, input.len()); + output +} + fn encode_mask_vers(layer: &Layer, mask: &[u8]) -> Vec { let tw: i32 = 64; let th: i32 = 64; @@ -366,12 +463,12 @@ fn encode_mask_vers(layer: &Layer, mask: &[u8]) -> Vec { let tidx = (y as usize) * (tw as usize) + (x as usize); let mut v = layer.mask_default_color; if let Some(mb) = layer.mask_bounds { - let m_top = mb[0] as i32; - let m_left = mb[1] as i32; + let m_top = mb[0] as i32; + let m_left = mb[1] as i32; let m_bottom = mb[2] as i32; - let m_right = mb[3] as i32; + let m_right = mb[3] as i32; if ly >= m_top && ly < m_bottom && lx >= m_left && lx < m_right { - let mask_w = m_right - m_left; + let mask_w = m_right - m_left; let mask_idx = ((ly - m_top) * mask_w + (lx - m_left)) as usize; if mask_idx < mask.len() { v = mask[mask_idx]; @@ -392,15 +489,12 @@ fn encode_mask_vers(layer: &Layer, mask: &[u8]) -> Vec { } if has_content { - let mut compressed = vec![1u8]; - let mut i = 0; - while i < tile_raw.len() { - let chunk = (tile_raw.len() - i).min(32); - compressed.push((chunk - 1) as u8); - compressed.extend_from_slice(&tile_raw[i..i + chunk]); - i += chunk; - } - tiles.push((tx, ty, compressed)); + // Same LZF format as raster tiles but PIXELSIZE=1. + let compressed = lzf_compress(&tile_raw); + let mut payload = Vec::with_capacity(1 + compressed.len()); + payload.push(1u8); // version byte (no delta-encode) + payload.extend_from_slice(&compressed); + tiles.push((tx, ty, payload)); } tx += tw; } @@ -412,13 +506,13 @@ fn encode_mask_vers(layer: &Layer, mask: &[u8]) -> Vec { output.extend_from_slice(b"TILEWIDTH 64\n"); output.extend_from_slice(b"TILEHEIGHT 64\n"); output.extend_from_slice(b"PIXELSIZE 1\n"); - output.extend_from_slice(b"DATA\n"); + output.extend_from_slice(format!("DATA {}\n", tiles.len()).as_bytes()); tiles.sort_by_key(|(x, y, _)| (*y, *x)); - for (tx, ty, tile_data) in tiles { - let header = format!("{},{},LZF,{}\n", tx, ty, tile_data.len()); + for (tx, ty, payload) in tiles { + let header = format!("{},{},LZF,{}\n", tx, ty, payload.len()); output.extend_from_slice(header.as_bytes()); - output.extend_from_slice(&tile_data); + output.extend_from_slice(&payload); } output diff --git a/hcie-kra/src/lib.rs b/hcie-kra/src/lib.rs index 9455435..74c2feb 100644 --- a/hcie-kra/src/lib.rs +++ b/hcie-kra/src/lib.rs @@ -627,6 +627,12 @@ fn try_import_krita_maindoc(archive: &mut zip::ZipArchive) -> Res f.read_to_string(&mut xml_content).map_err(|e| e.to_string())?; found = true; } + if !found { + if let Ok(mut f) = archive.by_name("layers.xml") { + f.read_to_string(&mut xml_content).map_err(|e| e.to_string())?; + found = true; + } + } if !found { return Ok(vec![]); } @@ -649,14 +655,25 @@ fn try_import_krita_maindoc(archive: &mut zip::ZipArchive) -> Res let name = e.name(); let tag_name = std::str::from_utf8(name.as_ref()).unwrap_or("").to_lowercase(); if tag_name == "image" { + let mut colorspace = String::new(); + let mut depth = String::new(); for attr in e.attributes() { let a = attr.map_err(|e| format!("XML attr error: {}", e))?; + let val = std::str::from_utf8(&a.value).unwrap_or(""); match a.key.as_ref() { - b"width" => width = std::str::from_utf8(&a.value).unwrap_or("800").parse().unwrap_or(800), - b"height" => height = std::str::from_utf8(&a.value).unwrap_or("600").parse().unwrap_or(600), + b"width" => width = val.parse().unwrap_or(800), + b"height" => height = val.parse().unwrap_or(600), + b"colorspacename" => colorspace = val.to_string(), + b"channeldepth" => depth = val.to_string(), _ => {} } } + if !colorspace.is_empty() && colorspace != "RGBA" && colorspace != "RGB" { + return Err(format!("Unsupported KRA color space: '{}' (only RGBA/RGB is supported)", colorspace)); + } + if !depth.is_empty() && depth != "U8" { + return Err(format!("Unsupported KRA channel depth: '{}' (only U8 is supported)", depth)); + } } else if tag_name == "layer" || tag_name == "paintlayer" || tag_name == "vectorlayer" @@ -787,7 +804,10 @@ fn try_import_krita_maindoc(archive: &mut zip::ZipArchive) -> Res let layer_path = find_krita_layer_file(&all_files, &info.filename, &info.nodetype); let zip_path = match layer_path { Some(ref path) => path, - None => continue, + None => { + log::warn!("KRA import: layer file not found for '{}' (filename={}, type={})", info.name, info.filename, info.nodetype); + continue; + } }; let mut buf = Vec::new(); @@ -805,13 +825,19 @@ fn try_import_krita_maindoc(archive: &mut zip::ZipArchive) -> Res let pixels = if is_vector { match rasterize_svg_to_rgba(&buf, width, height) { Ok(p) => p, - Err(_) => vec![0u8; (width * height * 4) as usize], + Err(e) => { + log::warn!("KRA import: SVG rasterization failed for layer '{}': {}", info.name, e); + vec![0u8; (width * height * 4) as usize] + } } } else if buf.starts_with(b"VERS") { let default_pixel = read_defaultpixel(archive, &info.filename); match parse_vers_tiles(&buf, width, height, info.x, info.y, default_pixel) { Ok(p) => p, - Err(_) => vec![0u8; (width * height * 4) as usize], + Err(e) => { + log::warn!("KRA import: VERS tile parse failed for layer '{}': {}", info.name, e); + vec![0u8; (width * height * 4) as usize] + } } } else { match image::load_from_memory(&buf) { @@ -834,7 +860,10 @@ fn try_import_krita_maindoc(archive: &mut zip::ZipArchive) -> Res } pixels } - Err(_) => vec![0u8; (width * height * 4) as usize], + Err(e) => { + log::warn!("KRA import: image decode failed for layer '{}': {}", info.name, e); + vec![0u8; (width * height * 4) as usize] + } } }; diff --git a/hcie_features.ods b/hcie_features.ods new file mode 100644 index 0000000..7efd0d6 Binary files /dev/null and b/hcie_features.ods differ