Skip to content

Commit f679209

Browse files
authored
Merge pull request #108 from budlibu500/fix/matrix44-transpose
fix(ltk_ritobin): transpose Mat4 for correct row-major text output
2 parents da0826e + 9e5e4cf commit f679209

File tree

3 files changed

+77
-2
lines changed

3 files changed

+77
-2
lines changed

crates/ltk_ritobin/src/parser.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,9 @@ fn parse_mtx44(input: Span) -> ParseResult<Mat4> {
431431

432432
let (remaining, _) = preceded(ws, char('}'))(remaining)?;
433433

434-
Ok((remaining, Mat4::from_cols_array(&values)))
434+
// text values are row-major but from_cols_array expects column-major,
435+
// so transpose to get the correct internal layout.
436+
Ok((remaining, Mat4::from_cols_array(&values).transpose()))
435437
}
436438

437439
/// Parse rgba: { r, g, b, a }

crates/ltk_ritobin/src/writer.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,9 @@ impl<'a, H: HashProvider> TextWriter<'a, H> {
209209
PropertyValueEnum::Matrix44(v) => {
210210
self.write_raw("{\n");
211211
self.indent();
212-
let arr = v.value.to_cols_array();
212+
// ritobin text stores matrices row-major, glam::Mat4 is column-major.
213+
// transpose so to_cols_array() yields values in row-major order.
214+
let arr = v.value.transpose().to_cols_array();
213215
for (i, val) in arr.iter().enumerate() {
214216
if i % 4 == 0 {
215217
self.pad();

crates/ltk_ritobin/tests/parse_sample.rs

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,77 @@ broken: unknowntype = 123
227227
}
228228
}
229229

230+
/// Make sure mtx44 values don't get mangled during a text round-trip.
231+
/// uh the text format is row-major but glam stores column-major so we
232+
/// transpose on write and on parse, i guess this just makes sure that
233+
/// doesn't break anything and the values come out right.
234+
#[test]
235+
fn test_matrix44_roundtrip_ordering() {
236+
use glam::Mat4;
237+
use ltk_meta::property::PropertyValueEnum;
238+
239+
// non-symmetric so we'd notice if it got transposed wrong
240+
// text layout (row-major):
241+
// 1 2 3 4
242+
// 5 6 7 8
243+
// 9 10 11 12
244+
// 13 14 15 16
245+
//
246+
// glam is column-major internally so:
247+
let expected = Mat4::from_cols_array(&[
248+
1.0, 5.0, 9.0, 13.0, // col0
249+
2.0, 6.0, 10.0, 14.0, // col1
250+
3.0, 7.0, 11.0, 15.0, // col2
251+
4.0, 8.0, 12.0, 16.0, // col3
252+
]);
253+
254+
let input = r#"#PROP_text
255+
type: string = "PROP"
256+
version: u32 = 3
257+
entries: map[hash,embed] = {
258+
"test/object" = TestClass {
259+
transform: mtx44 = {
260+
1, 2, 3, 4
261+
5, 6, 7, 8
262+
9, 10, 11, 12
263+
13, 14, 15, 16
264+
}
265+
}
266+
}
267+
"#;
268+
269+
// 1) Parse and verify the Mat4 matches expected column-major layout
270+
let file = parse(input).expect("Failed to parse matrix input");
271+
let tree = file.to_bin_tree();
272+
let obj = tree.objects.values().next().expect("Expected one object");
273+
let parsed_mat = match &obj.properties.values().next().unwrap().value {
274+
PropertyValueEnum::Matrix44(v) => v.value,
275+
other => panic!("Expected Matrix44, got {:?}", other),
276+
};
277+
assert_eq!(
278+
parsed_mat, expected,
279+
"Parsed Mat4 should match expected column-major layout"
280+
);
281+
282+
// 2) Write back to text, parse again, verify values survive the round-trip
283+
let output = write(&tree).expect("Failed to write tree");
284+
let file2 = parse(&output).expect("Failed to re-parse written output");
285+
let tree2 = file2.to_bin_tree();
286+
let obj2 = tree2
287+
.objects
288+
.values()
289+
.next()
290+
.expect("Expected one object after round-trip");
291+
let roundtrip_mat = match &obj2.properties.values().next().unwrap().value {
292+
PropertyValueEnum::Matrix44(v) => v.value,
293+
other => panic!("Expected Matrix44 after round-trip, got {:?}", other),
294+
};
295+
assert_eq!(
296+
roundtrip_mat, expected,
297+
"Matrix44 should survive text round-trip unchanged"
298+
);
299+
}
300+
230301
#[test]
231302
fn test_error_is_miette_diagnostic() {
232303
use miette::Diagnostic;

0 commit comments

Comments
 (0)