Skip to content

Fix subdivision memory handling, crease artifacts and stale texture references - #750

Closed
FreeCAD-Nut wants to merge 1 commit into
LuxCoreRender:mainfrom
FreeCAD-Nut:codex/subdivision-reliability
Closed

FreeCAD-Nut wants to merge 1 commit into
LuxCoreRender:mainfrom
FreeCAD-Nut:codex/subdivision-reliability

Conversation

@FreeCAD-Nut

Copy link
Copy Markdown

Subdivision could corrupt attribute buffers or fail on material meshes with unused vertices, while Enhanced mode introduced creases that depended on vertex numbering. This fixes those defects and related ownership/reference issues found while debugging a Blender subdivision/displacement scene.

Changes:

  • Size simple-mode stencil buffers for all outputs, copy UV/color components into correctly sized typed layers, and evaluate vertex AOVs from their own buffers only when present.
  • Compact unreferenced vertices before enhanced subdivision while preserving attributes and triangle order; preserve triangle AOV ancestry in both modes.
  • Retain typed ownership of interpolated arrays and release temporary mesh auxiliary allocations.
  • Classify creases using the unsigned angle between face normals; test the binary sharp-edge flag independently of the angle threshold.
  • Correct texture-reference update overrides and update pointer/reference holders by reference.
  • Release the temporary serialization unique_ptr alias on both success and exception so the mesh cache retains ownership.
  • Add optional native CTest regressions for subdivision attributes/copies, unused vertices, crease invariance and texture replacement.

Validation:

  • Windows x64/MSVC Release build against main ce1d641 passed. All 31 published files were compared with the tested checkout.
  • Both CTest cases passed, with explicit PASS markers for simple/enhanced subdivision with and without attributes, varying attributes with unused vertices, crease invariance and texture references.
  • Earlier patched-2.11.2 Blender 5.2.3 integration tests passed displacement geometry, viewport updates and a 1024x1024 close-up render; the jagged inner-rim artifacts disappeared. Those Blender tests used the release-based patch, not this current-main build. No cross-platform or exhaustive sanitizer validation is claimed.

The existing main fixes for phantom AOV layers and wheel packaging are retained rather than duplicated. Companion addon changes: LuxCoreRender/BlendLuxCore#1110

To run: configure with -DLUXCORE_SUBDIVISION_TESTS=ON, build subdivision_regression, then run ctest -C Release --output-on-failure from the build directory (runtime dependency libraries on PATH).

@CodeFHD

CodeFHD commented Oct 28, 2026

Copy link
Copy Markdown
Member

Similarly to the related BlendLuxCore PR, can you provide scene-files, blend-files, or some example renderings?
As this is obviously and AI generated commit (codex-branch), I would like to see if this is based on user-encountered bugs, or some automated finding.

Without having read and undestood all the code (I did not write the subdiv feature): I am wondering about the described situations like meshes with unused vertices. I would like to understand how (and perhaps why) such a mesh is being generated in the first place. In other words w.r.t. this review: I see code to remove "isolated vertices", but is this the correct place or should this be done during mesh export to begin with?

The PR might also benefit from being split into multiple commits to separate individual parts for easier understanding.

@CodeFHD
CodeFHD requested a review from howetuft October 28, 2026 10:56
@howetuft

Copy link
Copy Markdown
Contributor

Hello,

Sorry, but I'm gonna reject your PR.

  1. This is exactly what we do not want: a giant, unreviewable PR, modifying 31 (!) files at the same time (most of them being weakly related to subdivision) in a unique commit, which mixes style code (argument constness, override attribute), memory handling, feature-related code etc. It smells AI-generated code without any human control (have you got good command of C++?).

  2. Regrettably, no effort was made to coordinate in advance. Just as a reminder, Subdivision is in my roadmap, as stated here: https://discord.com/channels/476391564948668426/476391764798865409/1555136721455153214, and I'm about to work on it. So the last thing I need is a sloppy, unrequested PR that messes up the code...

@howetuft howetuft closed this Oct 28, 2026
@FreeCAD-Nut

Copy link
Copy Markdown
Author

BlendLuxCore was crashing when I tried to render a scene. I cannot share the scene because of licensing restrictions, and I am not sure how to reproduce the crash with a simpler scene. Since Codex AI was able to fix the crash and the remaining rendering artifacts, I thought sharing the fixes with the project would be helpful. If you find the submission unhelpful or confusing, by all means, reject it. I won’t be submitting any further fixes.

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.

3 participants