feat: update example - #4
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b2e658df0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| r00 = matrix[0, 0] / scale_x | ||
| r01 = matrix[0, 1] / scale_x | ||
| r02 = matrix[0, 2] / scale_x | ||
| r10 = matrix[1, 0] / scale_y | ||
| r11 = matrix[1, 1] / scale_y |
There was a problem hiding this comment.
The transform matrices returned by node.local_transform are documented as column‑major, but this code treats the first index as a row and normalizes/extracts Euler angles using row vectors (matrix[0,1], matrix[0,2], etc.). For any node with non‑identity rotation or scale, this will report transposed axes (incorrect scale/rotation values) because the basis vectors live in the columns. Consider using column vectors (e.g., matrix[0,0], matrix[1,0], matrix[2,0]) or transposing the matrix before extracting Euler angles.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b2e658df0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| r00 = matrix[0, 0] / scale_x | ||
| r01 = matrix[0, 1] / scale_x | ||
| r02 = matrix[0, 2] / scale_x | ||
| r10 = matrix[1, 0] / scale_y | ||
| r11 = matrix[1, 1] / scale_y |
There was a problem hiding this comment.
The matrices returned by node.local_transform are documented as column‑major (see Node.local_transform docstring), and the rest of the function treats the first index as the column (eg, position uses matrix[3, 0..2], scale uses column vectors). Here, r00..r22 are filled as if matrix[row, col], which effectively transposes the rotation matrix. Any node with a non‑trivial rotation will therefore report incorrect Euler angles (often swapped axes or sign). To fix, either transpose once or populate r00..r22 using column‑major indexing (eg r01 = matrix[1,0], r02 = matrix[2,0], etc.).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR enhances the basic usage example to provide a richer walkthrough of loading and inspecting FBX scenes with ufbx-python, and removes the now-outdated test fixtures README.
Changes:
- Remove
tests/fixtures/README.md, which previously documented how to obtain FBX test files. - Expand
examples/basic_usage.pyto include scene statistics, coordinate system analysis, node hierarchy and transform inspection, mesh and material details, and more descriptive error handling.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/fixtures/README.md | Removes the fixtures README that explained how to download sample FBX files for tests. |
| examples/basic_usage.py | Substantially enriches the basic usage example with scene statistics, coordinate system info, node hierarchy and transforms, mesh/attribute details, and improved error reporting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def print_transform_info(node): | ||
| """Print node transformation information""" | ||
| import math | ||
|
|
||
| # Local transform (relative to parent) - returns 4x4 numpy array (column-major) | ||
| local_matrix = node.local_transform | ||
| print(" - Transform (Local, relative to parent):") | ||
|
|
||
| # Extract position (4th column: [3, 0], [3, 1], [3, 2]) | ||
| pos_x, pos_y, pos_z = local_matrix[3, 0], local_matrix[3, 1], local_matrix[3, 2] | ||
| print(f" Position: ({pos_x:8.3f}, {pos_y:8.3f}, {pos_z:8.3f})") | ||
|
|
||
| # Extract scale (length of each column vector) | ||
| scale_x = math.sqrt(local_matrix[0, 0] ** 2 + local_matrix[0, 1] ** 2 + local_matrix[0, 2] ** 2) | ||
| scale_y = math.sqrt(local_matrix[1, 0] ** 2 + local_matrix[1, 1] ** 2 + local_matrix[1, 2] ** 2) | ||
| scale_z = math.sqrt(local_matrix[2, 0] ** 2 + local_matrix[2, 1] ** 2 + local_matrix[2, 2] ** 2) | ||
| print(f" Scale: ({scale_x:8.3f}, {scale_y:8.3f}, {scale_z:8.3f})") | ||
|
|
||
| # Extract rotation (Euler angles) | ||
| rot_x, rot_y, rot_z = extract_euler_angles(local_matrix, scale_x, scale_y, scale_z) | ||
| print(f" Rotation: ({rot_x:8.3f}°, {rot_y:8.3f}°, {rot_z:8.3f}°) [XYZ Euler]") | ||
|
|
||
| # Check if there are significant transformations | ||
| has_translation = abs(pos_x) > 0.001 or abs(pos_y) > 0.001 or abs(pos_z) > 0.001 | ||
| has_rotation = abs(rot_x) > 0.01 or abs(rot_y) > 0.01 or abs(rot_z) > 0.01 | ||
| has_scale = abs(scale_x - 1.0) > 0.001 or abs(scale_y - 1.0) > 0.001 or abs(scale_z - 1.0) > 0.001 | ||
|
|
||
| transform_type = [] | ||
| if has_translation: | ||
| transform_type.append("Translation") | ||
| if has_rotation: | ||
| transform_type.append("Rotation") | ||
| if has_scale: | ||
| transform_type.append("Scale") | ||
|
|
||
| if transform_type: | ||
| print(f" Has: {', '.join(transform_type)}") | ||
| else: | ||
| print(" Has: Identity (no transform)") | ||
|
|
||
| # World transform | ||
| world_matrix = node.world_transform | ||
| print(" - Transform (World):") | ||
| world_pos_x = world_matrix[3, 0] | ||
| world_pos_y = world_matrix[3, 1] | ||
| world_pos_z = world_matrix[3, 2] | ||
| print(f" Position: ({world_pos_x:8.3f}, {world_pos_y:8.3f}, {world_pos_z:8.3f})") | ||
|
|
||
| # World space scale and rotation | ||
| world_scale_x = math.sqrt(world_matrix[0, 0] ** 2 + world_matrix[0, 1] ** 2 + world_matrix[0, 2] ** 2) | ||
| world_scale_y = math.sqrt(world_matrix[1, 0] ** 2 + world_matrix[1, 1] ** 2 + world_matrix[1, 2] ** 2) | ||
| world_scale_z = math.sqrt(world_matrix[2, 0] ** 2 + world_matrix[2, 1] ** 2 + world_matrix[2, 2] ** 2) | ||
| world_rot_x, world_rot_y, world_rot_z = extract_euler_angles(world_matrix, world_scale_x, world_scale_y, world_scale_z) |
There was a problem hiding this comment.
print_transform_info() assumes node.local_transform and node.world_transform are 4x4 NumPy arrays (and the comment explicitly documents them that way), but elsewhere in this repo node.local_transform is used as a Transform object with .translation, .scale, and .to_matrix() (see tests/test_real_fbx.py). Unless the API has changed to make these properties matrices, this example will either raise at indexing (local_matrix[3, 0]) or at least be misleadingly documented. Please either use the Transform API (e.g. access translation/scale or call to_matrix() before indexing) and update the comment accordingly, or switch to the correct matrix property if one exists.
| r00 = matrix[0, 0] / scale_x | ||
| r01 = matrix[0, 1] / scale_x | ||
| r02 = matrix[0, 2] / scale_x | ||
| r10 = matrix[1, 0] / scale_y |
There was a problem hiding this comment.
Variable r10 is not used.
| r10 = matrix[1, 0] / scale_y |
| r10 = matrix[1, 0] / scale_y | ||
| r11 = matrix[1, 1] / scale_y | ||
| r12 = matrix[1, 2] / scale_y | ||
| r20 = matrix[2, 0] / scale_z |
There was a problem hiding this comment.
Variable r20 is not used.
| r20 = matrix[2, 0] / scale_z |
No description provided.