Skip to content

Fix uninitialized UVs, missing CUDA_CHECK in cleanup, face-skip warning, and unused alias#24

Merged
pmudry merged 2 commits intoTexture-and-MTL-supportfrom
copilot/sub-pr-20
Mar 17, 2026
Merged

Fix uninitialized UVs, missing CUDA_CHECK in cleanup, face-skip warning, and unused alias#24
pmudry merged 2 commits intoTexture-and-MTL-supportfrom
copilot/sub-pr-20

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 17, 2026

Five unresolved review issues from the texture/MTL PR, all in the GPU scene build and OBJ loading paths.

Changes

  • scene_description.hppaddTriangle() and addTriangleWithNormals() set has_uvs = false but left uv0/uv1/uv2 uninitialized. Now explicitly zero-initialized to Vec3(0,0,0).

  • scene_builder_cuda.cuconvertGeometry() — UVs were unconditionally copied from GeometryDesc even when has_uvs = false, reading uninitialized memory into GPU buffers. Now guarded:

    if (desc.data.triangle.has_uvs) {
        geom.data.triangle.uv0 = f2(...);
        // ...
    } else {
        geom.data.triangle.uv0 = f2(0.0f, 0.0f);
        // ...
    }
  • scene_builder_cuda.cufreeGPUScene() — Bare CUDA calls in the texture teardown path (cudaMemcpy, cudaGetTextureObjectResourceDesc, cudaDestroyTextureObject, cudaFreeArray, cudaFree) were not error-checked. All wrapped with CUDA_CHECK for consistency.

  • obj_loader.hpp — Faces encountered before any usemtl (with no fallback) were silently skipped despite the docstring claiming a warning was emitted. Added a per-call face_skip_warned flag that emits one std::cerr diagnostic per loadOBJ() invocation, and updated the docstring to match.

  • generate_uv_models.pyN = normal = (0, 1, 0) left normal unused (N was the actual use site). Simplified to normal = (0, 1, 0) with the one use site updated accordingly.


📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.

Copilot AI changed the title [WIP] Add texture and MTL support for OBJ meshes Fix uninitialized UVs, missing CUDA_CHECK in cleanup, face-skip warning, and unused alias Mar 17, 2026
Copilot AI requested a review from pmudry March 17, 2026 20:52
@github-actions
Copy link
Copy Markdown

Docs out of sync?

This PR changes 3 source file(s) but no pages under website/docs/ were updated.

If this is a significant change (new feature, new CLI flag, new material/geometry type, performance improvement), please update the relevant documentation page. See the mapping in .github/copilot-instructions.md.

If this change genuinely doesn't affect the docs, you can ignore this message.

@pmudry pmudry marked this pull request as ready for review March 17, 2026 20:52
@pmudry pmudry merged commit 9b21f49 into Texture-and-MTL-support Mar 17, 2026
1 check passed
@pmudry pmudry deleted the copilot/sub-pr-20 branch March 17, 2026 20:53
pmudry added a commit that referenced this pull request Mar 17, 2026
* Milestone explorer script for documentation

* Add MTL material loading and diffuse texture support for OBJ meshes

- MTL loader (mtl_loader.hpp): parse Kd/Ke/Ns/Ni/d/map_Kd, heuristic material
  type assignment (LAMBERTIAN/ROUGH_MIRROR/GLASS/LIGHT/MIRROR)
- Texture loader (texture_loader.cc): stb_image-based RGBA loader, dedup by path
- OBJ loader rewrite: parse mtllib/usemtl, UV indices; calls appropriate
  addTriangle variant (normals+UVs / UVs only / normals / plain)
- scene_description.hpp: TextureDesc struct, textures vector, addTexture(),
  addTriangleWithUVs(), addTriangleWithNormalsAndUVs(), UV fields on triangle
- yaml_scene_loader: material: now optional for OBJ entries (fallback -1);
  texture: key in material definitions
- CUDA backend: UV interpolation in hit_triangle(), tex2D sampling in
  apply_material(), cudaTextureObject upload/cleanup in scene_builder_cuda.cu
- OptiX backend: UV via intersection attributes (numAttributeValues 5),
  texture sampling in __closesthit__ch, d_textures in launch params + cleanup
- CMakeLists: fix OptiX source ordering race (target_sources after find_path);
  add texture_loader.cc to SOURCE_RAYON
- Assets: scripts/generate_grid_texture.py, scripts/generate_uv_models.py,
  resources/textures/grid_512.png, resources/models/{plane,cube,sphere}_uv.obj,
  resources/models/texture_test.mtl, resources/scenes/texture_test.yaml

* Updating things to do and .gitignore

* perf: fix BVH not activating in texture_test scene (20x CUDA speedup)

The YAML parser only recognizes 'camera:' and 'settings:' as top-level section
headers. texture_test.yaml nested its settings under 'scene:' (unrecognized),
so the 'use_bvh: true' key was stored as 'camera.use_bvh', never matching
'settings.use_bvh'. BVH was disabled, causing a linear O(4053) triangle scan
per ray instead of O(log n) BVH traversal.

Fixes:
- texture_test.yaml: use top-level 'camera:' and 'settings:' sections
- yaml_scene_loader.cc: also accept 'scene.use_bvh' as a legacy alias

Result: 1.9M -> 38M rays/sec (20x speedup, 85% of the non-textured baseline)

* - Reimplemented fibonacci dots and displaced sphere in OptiX
- Larger grid textures generation script
- Update to Suzanne model

* New test object from Sketchfab, CC0, https://sketchfab.com/pooyast/collections/shader-258e0160e20c4c80967e43cfa1141671

* Render sphere CCA information

* Working on textures projections

* Potential fix for pull request finding

Hard-coded paths correction

Co-authored-by: Copilot Autofix powered by AI <[email protected]>

* Updating TODOS.md in favor of git issues

* fix: address PR review comments

- Add missing <cmath>/<cctype> headers in mtl_loader.hpp; use std::tolower safely
- Resolve texture paths relative to YAML scene_dir in yaml_scene_loader.cc
- Fix comment/code order mismatch in cuda_raytracer.cuh (texture overwrites pattern)
- Fix cudaArray_t GPU memory leaks in optix_renderer.cu (rebuild + cleanup paths)
- Fix cudaArray_t GPU memory leaks in scene_builder_cuda.cu (freeGPUScene)
- Fix mtl_dir resolved from mtl file path, not OBJ dir in obj_loader.hpp
- Fix OBJ visible flag applied to whole triangle range, not just last triangle
- Document 'stripes' pattern in SCENE_FORMAT.md"

* fix: address PR reviews comments

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <[email protected]>

* Fix uninitialized UVs, missing CUDA_CHECK in cleanup, face-skip warning, and unused alias (#24)

* Initial plan

* fix: address unresolved review comments (UV init, CUDA_CHECK, warning, unused var)

Co-authored-by: pmudry <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: pmudry <[email protected]>

---------

Co-authored-by: Copilot Autofix powered by AI <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: pmudry <[email protected]>
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.

2 participants