Skip to content

Fix box face winding order#24

Merged
shayancoin merged 1 commit intomainfrom
codex/update-mesh-generation-in-generate_reference_glbs.py
Oct 15, 2025
Merged

Fix box face winding order#24
shayancoin merged 1 commit intomainfrom
codex/update-mesh-generation-in-generate_reference_glbs.py

Conversation

@shayancoin
Copy link
Owner

@shayancoin shayancoin commented Oct 15, 2025

Summary

  • adjust box mesh triangle winding in generate_reference_glbs to ensure outward-facing normals

Testing

  • not run

https://chatgpt.com/codex/tasks/task_e_68ef016e4d908330bba1cd2b97482337

Summary by CodeRabbit

  • Bug Fixes
    • Corrected triangle orientation in generated box meshes to ensure proper backface culling and shading.
    • Resolves issues with faces appearing invisible or inside-out in exported reference GLBs across common viewers.
    • Improves visual consistency and reliability of generated 3D models without altering geometry.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 15, 2025

Walkthrough

Reversed the winding order of the two triangles composing each face in build_box within scripts/generate_reference_glbs.py by swapping vertex indices, potentially affecting backface culling. No vertices, topology, or exported interfaces were changed.

Changes

Cohort / File(s) Summary
Mesh face winding update
scripts/generate_reference_glbs.py
Reordered triangle indices for each face from [top_left, bottom_left, bottom_right] and [top_left, bottom_right, top_right] to [top_left, bottom_right, bottom_left] and [top_left, top_right, bottom_right], reversing winding without altering vertices or connectivity.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I flipped my ears like triangles spin,
A winding waltz, outside to in—
Faces turn, the cullers grin,
Mesh still whole, a subtle win.
Hop-hop, I tweak with gentle pride,
Same old box, just inside-outside. 🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The pull request description does not follow the repository’s required template: it omits the “PR Type” heading, uses “Summary” instead of “Short Description,” and lacks a “Tests Added” section, so the structured information expected by the template is incomplete. Please update the description to use the repository template by adding a “PR Type” section, renaming “Summary” to “Short Description,” and including a “Tests Added” section to clearly document testing coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title accurately and concisely describes the primary change of fixing the box face winding order in the mesh generation script, matching the main purpose of the pull request without unnecessary detail or ambiguity.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codex/update-mesh-generation-in-generate_reference_glbs.py

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 102fc53 and 59a5bc5.

📒 Files selected for processing (1)
  • scripts/generate_reference_glbs.py (1 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI
scripts/generate_reference_glbs.py

[error] 109-109: Mypy: No binding for nonlocal 'buffer_bytes' found (misc).

🔇 Additional comments (1)
scripts/generate_reference_glbs.py (1)

92-93: Verify outward-facing normals in generated GLB files.

Open base_600(.glb, @lod1), tall_600(.glb, @lod1), and wall_900(.glb, @lod1) in a 3D viewer with backface culling enabled to confirm all faces are correctly oriented.


Comment @coderabbitai help to get the list of available commands and usage tips.

@shayancoin shayancoin merged commit 220b151 into main Oct 15, 2025
2 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant