Fix gltf: Vertex attribute は UnsignedInt に対応していない #351
Conversation
Walkthroughこの変更の概要は、 Changes
TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
demo/cesium/examples/ext_structural_metadata/tileset.jsonis excluded by:!**/*.json
Files selected for processing (1)
- nusamai/src/sink/gltf/gltf_writer.rs (2 hunks)
nusamai/src/sink/gltf/gltf_writer.rs
Outdated
| let feature_ids_offset = bin_content.len(); | ||
| let mut feature_ids_count = 0; | ||
| for vertex in buffer.vertices.clone() { | ||
| bin_content | ||
| .write_all(&vertex.feature_id.to_le_bytes()) | ||
| .unwrap(); | ||
| // MeshPrimitive does not support UnsignedInt | ||
| // https://github.com/CesiumGS/glTF/blob/proposal-EXT_mesh_features/extensions/2.0/Vendor/EXT_mesh_features/README.md#feature-id-by-vertex | ||
| let vertex = vertex.feature_id as f32; | ||
| bin_content.write_all(&vertex.to_le_bytes()).unwrap(); | ||
| feature_ids_count += 1; | ||
| } | ||
| let feature_ids_len = bin_content.len() - feature_ids_offset; |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [137-158]
feature_id のデータ型を u32 から f32 に変換する変更について、GLTFのEXT_mesh_features拡張機能に準拠するための適切な対応です。ただし、f32 への変換により精度が低下する可能性があるため、この変更がアプリケーションの要件に適合しているか検討する必要があります。また、feature_id の最小値と最大値を計算する際に、as f64 でキャストしている点に注意してください。f32 から f64 へのキャストは精度を向上させるものではなく、元の u32 の値の範囲を正確に反映しているか確認が必要です。
Codecov ReportAll modified and coverable lines are covered by tests ✅ Additional details and impacted files
📢 Thoughts on this report? Let us know! |
ciscorn
changed the title
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- nusamai/src/sink/gltf/gltf_writer.rs (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- nusamai/src/sink/gltf/gltf_writer.rs
ciscorn
changed the title
Summary by CodeRabbit
gltf_writer.rsの変更により、feature_idをu32からf32に変換しました。これは、MeshPrimitiveがUnsignedIntをサポートしていないために行われました。この変更により、バイナリコンテンツの書き込みとアクセサコンポーネントのタイプに影響があります。