Skip to content

Commit 5d2c74f

Browse files
budlibu500claude
andcommitted
style: rustfmt
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent eef4367 commit 5d2c74f

File tree

2 files changed

+70
-4
lines changed

2 files changed

+70
-4
lines changed

crates/ltk_ritobin/tests/parse_sample.rs

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ fn test_matrix44_roundtrip_ordering() {
245245
//
246246
// glam is column-major internally so:
247247
let expected = Mat4::from_cols_array(&[
248-
1.0, 5.0, 9.0, 13.0, // col0
248+
1.0, 5.0, 9.0, 13.0, // col0
249249
2.0, 6.0, 10.0, 14.0, // col1
250250
3.0, 7.0, 11.0, 15.0, // col2
251251
4.0, 8.0, 12.0, 16.0, // col3
@@ -274,18 +274,28 @@ entries: map[hash,embed] = {
274274
PropertyValueEnum::Matrix44(v) => v.value,
275275
other => panic!("Expected Matrix44, got {:?}", other),
276276
};
277-
assert_eq!(parsed_mat, expected, "Parsed Mat4 should match expected column-major layout");
277+
assert_eq!(
278+
parsed_mat, expected,
279+
"Parsed Mat4 should match expected column-major layout"
280+
);
278281

279282
// 2) Write back to text, parse again, verify values survive the round-trip
280283
let output = write(&tree).expect("Failed to write tree");
281284
let file2 = parse(&output).expect("Failed to re-parse written output");
282285
let tree2 = file2.to_bin_tree();
283-
let obj2 = tree2.objects.values().next().expect("Expected one object after round-trip");
286+
let obj2 = tree2
287+
.objects
288+
.values()
289+
.next()
290+
.expect("Expected one object after round-trip");
284291
let roundtrip_mat = match &obj2.properties.values().next().unwrap().value {
285292
PropertyValueEnum::Matrix44(v) => v.value,
286293
other => panic!("Expected Matrix44 after round-trip, got {:?}", other),
287294
};
288-
assert_eq!(roundtrip_mat, expected, "Matrix44 should survive text round-trip unchanged");
295+
assert_eq!(
296+
roundtrip_mat, expected,
297+
"Matrix44 should survive text round-trip unchanged"
298+
);
289299
}
290300

291301
#[test]

matrix-fix.md

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
# Matrix44 Transpose Bug Fix
2+
3+
## Problem
4+
The text output for `transform: mtx44` was transposed. Translation value `-4500` appeared at `[row 1, col 3]` instead of `[row 3, col 1]`.
5+
6+
**Wrong (before):**
7+
```
8+
transform: mtx44 = {
9+
1, 0, 0, 0
10+
0, 1, 0, -4500 <-- wrong position
11+
0, 0, 1, 0
12+
0, 0, 0, 1
13+
}
14+
```
15+
16+
**Correct (after):**
17+
```
18+
transform: mtx44 = {
19+
1, 0, 0, 0
20+
0, 1, 0, 0
21+
0, 0, 1, 0
22+
0, -4500, 0, 1 <-- correct position
23+
}
24+
```
25+
26+
## Root Cause
27+
`Matrix44Value` wrapped `glam::Mat4`, which stores data in column-major order internally. The reader (`read_mat4_row_major`) called `from_cols(...).transpose()` and the text writer called `v.0.transpose().to_cols_array()` — but these two transposes did not cancel out correctly when outputting to text, resulting in a transposed matrix.
28+
29+
The reference C++ ritobin simply stores the matrix as a raw `std::array<float, 16>` and writes it back directly. No column-major/row-major conversion needed.
30+
31+
## Fix
32+
Stop using `glam::Mat4` for `Matrix44Value`. Use `[f32; 16]` instead — read 16 floats, store 16 floats, write 16 floats. No transpose logic needed.
33+
34+
### Changed Files
35+
36+
#### 1. `ltk_meta/src/property/value/primitives.rs`
37+
- Replaced `impl_prim!(Matrix44Value, Mat4, [], mat4_row_major::<LE>, 0)` with a manual struct + impl
38+
- `Matrix44Value` now wraps `[f32; 16]` instead of `Mat4`
39+
- `ReadProperty::from_reader` reads 16 `f32` values sequentially
40+
- `WriteProperty::to_writer` writes 16 `f32` values sequentially
41+
42+
#### 2. `ltk_ritobin/src/writer.rs` (line 225)
43+
- `let arr = v.0.transpose().to_cols_array()``let arr = v.0`
44+
- Since `v.0` is now `[f32; 16]` in binary order, just iterate directly
45+
46+
#### 3. `ltk_ritobin/src/parser.rs` (text parser)
47+
- `parse_mtx44` return type: `Mat4``[f32; 16]`
48+
- `Mat4::from_cols_array(&values).transpose()``values`
49+
50+
## Testing
51+
Standalone test project at `test_matrix/`:
52+
```bash
53+
cd test_matrix && cargo run 2>/dev/null | grep -A 6 "mtx44"
54+
```
55+
56+
Verified output matches reference `ritobin_cli.exe` output.

0 commit comments

Comments
 (0)