Skip to content

Fix normals on Imported CAD files#134

Merged
vnoves merged 2 commits intodevelopfrom
86b7ch6fy/NormalsImportedCAD
Nov 6, 2025
Merged

Fix normals on Imported CAD files#134
vnoves merged 2 commits intodevelopfrom
86b7ch6fy/NormalsImportedCAD

Conversation

@vnoves
Copy link
Member

@vnoves vnoves commented Nov 6, 2025

Summary by CodeRabbit

  • Bug Fixes

    • Improved normal data handling in glTF exports to ensure more accurate geometry normals and rendering.
  • Documentation

    • Added a Sponsors section to the README with a brief sponsor note and image.

@coderabbitai
Copy link

coderabbitai bot commented Nov 6, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

The AddNormals method in Common_glTF_Exporter/Utils/glTFExportUtils.cs now emits each normal's X,Y,Z coordinates three times in the OnEachFacet branch (looped emission) instead of emitting a single triplet. A new Sponsors block is added to README.md; no public signatures changed.

Changes

Cohort / File(s) Summary
Normal emission change
Common_glTF_Exporter/Utils/glTFExportUtils.cs
AddNormals — in the OnEachFacet branch, replaced single triplet emission of normal X,Y,Z with a loop that emits the X,Y,Z values three times per normal.
Documentation addition
README.md
Added a new "Sponsors" section (heading, sponsor note, and Visiofy image link) before the "About us" section; purely additive.

Sequence Diagram(s)

sequenceDiagram
    participant Exporter as Caller
    participant AddNormals as AddNormals()
    participant Buffer as NormalsBuffer

    Exporter->>AddNormals: request normals emission (OnEachFacet)
    alt OnEachFacet branch
        note right of AddNormals `#DDEBF7`: loop repeats 3 times per normal
        AddNormals->>Buffer: emit X,Y,Z
        AddNormals->>Buffer: emit X,Y,Z
        AddNormals->>Buffer: emit X,Y,Z
    else Other branches
        AddNormals->>Buffer: emit normals (unchanged)
    end
    Buffer-->>Exporter: normals written
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Areas needing extra attention:
    • Confirm the triplication of normals is intentional for the target glTF/mesh layout (vertex duplication vs. index expectations).
    • Verify no off-by-one or buffer indexing issues from the repeated writes.
    • Check performance/memory implications if large meshes are affected.

Poem

🐰
Three little normals hop in a line,
Each one echoed, three times they shine.
Facets hum a rhythmic code,
Tripled notes down the export road. ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly describes the main change: fixing normals on imported CAD files, which aligns with the code modification in GLTFExportUtils that adjusts how normals are emitted.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1fbb50e and 4e2d5ba.

📒 Files selected for processing (1)
  • README.md (1 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@vnoves vnoves merged commit ebe4d19 into develop Nov 6, 2025
1 of 2 checks passed
@vnoves vnoves deleted the 86b7ch6fy/NormalsImportedCAD branch November 9, 2025 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant