kra_saver_fixed

.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::<u64>()` 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
+`<IMAGE>` and `<layer>` 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::<u64>();
+   let low = rand::random::<u64>();
+   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 `<IMAGE>`:
+   - 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)

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::<u64>();
+        let uuid_low = rand::random::<u64>();
+        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("        <layers>\n");
             let mask_filename = format!("{}_mask", filename);
-            let mask_uuid = format!("{{{:032x}}}", layer.id.wrapping_add(1)); // pseudo uuid
+            let mask_high = rand::random::<u64>();
+            let mask_low = rand::random::<u64>();
+            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!(
-                "          <layer name=\"Transparency Mask\" filename=\"{}\" nodetype=\"transparencymask\" visible=\"1\" opacity=\"255\" uuid=\"{}\" compositeop=\"normal\" colorspacename=\"Alpha\" channeldepth=\"U8\" profile=\"\" x=\"0\" y=\"0\" />\n",
+                "          <layer name=\"Transparency Mask\" filename=\"{}\" nodetype=\"transparencymask\" visible=\"1\" opacity=\"255\" uuid=\"{}\" compositeop=\"normal\" colorspacename=\"GRAYA\" channeldepth=\"U8\" profile=\"Gray-D50-elle-V2-g10.icc\" x=\"0\" y=\"0\" />\n",
                 mask_filename, mask_uuid
             ));
             xml.push_str("        </layers>\n");
@@ -293,10 +312,11 @@ fn encode_layer_vers(layer: &Layer) -> Vec<u8> {
                         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<u8> {
             }
 
             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<u8> {
     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<u8> {
+    // 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<usize> = 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<u8>, 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<u8> {
     let tw: i32 = 64;
     let th: i32 = 64;
@@ -366,12 +463,12 @@ fn encode_mask_vers(layer: &Layer, mask: &[u8]) -> Vec<u8> {
                     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<u8> {
             }
 
             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<u8> {
     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

hcie-kra/src/lib.rs

@@ -627,6 +627,12 @@ fn try_import_krita_maindoc(archive: &mut zip::ZipArchive<std::fs::File>) -> 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<std::fs::File>) -> 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<std::fs::File>) -> 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<std::fs::File>) -> 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<std::fs::File>) -> 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]
+                }
             }
         };