feat: improve materials - #5
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: 6216b63638
ℹ️ 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".
| ufbx_texture* texture | ||
| bint has_value | ||
| bint texture_enabled | ||
| bint feature_disabled | ||
| unsigned int value_components |
There was a problem hiding this comment.
In ufbx.h, ufbx_material_map stores the flags as bool and value_components as uint8_t, but the Cython struct maps them to bint/unsigned int. Because those are 4‑byte ints in Cython, the layout no longer matches the C struct, so has_value, texture_enabled, feature_disabled, and value_components will be read from the wrong offsets (often into the next map). Any access to MaterialMap properties can then return garbage or crash. Use C types that match bool/uint8_t to preserve layout.
Useful? React with 👍 / 👎.
| ctypedef struct ufbx_material: | ||
| ufbx_element element | ||
| ufbx_material_fbx_maps fbx | ||
| ufbx_material_pbr_maps pbr | ||
| int shader_type |
There was a problem hiding this comment.
The Cython ufbx_element/ufbx_material structs only include name, fbx, pbr, and shader_type, but in ufbx.h both structs include additional fields (props/IDs in element, and features, shader, shader_prop_prefix, textures in material). That means the offsets for fbx, pbr, shader_type, and shading_model_name are wrong in Cython, so accessing material maps or shader info will read unrelated memory. This was introduced by switching to direct struct access; the layout must match the full C definition to avoid corrupted data or segfaults.
Useful? React with 👍 / 👎.
| ufbx_element element | ||
| ufbx_string filename | ||
| ufbx_string relative_filename | ||
| ufbx_string absolute_filename | ||
| int type |
There was a problem hiding this comment.
The C struct ufbx_texture places type immediately after the element union and before the filename fields, and the element union itself includes props/IDs. Here the Cython struct omits those element fields and reorders the members (filename, relative_filename, absolute_filename, then type). As a result, Texture.filename/relative_filename/absolute_filename and Texture.type will decode the wrong offsets, returning garbage or crashing on access. Either mirror the full ufbx.h layout or keep using wrapper accessors.
Useful? React with 👍 / 👎.
1 participant
No description provided.