Comfy-Org / Comfy-Org/ComfyUI_frontend

fix(load3d): reset OBJLoader2Parallel materials between loads to avoid stale-state corruption

Open
#11,637 0 comments 0 reactions 1 assignee Claimed by @jtydhr88 View on GitHub
area:3d Potential Bug
Dominant language
TypeScript
Stars
2k
Forks
699
Avg merge
1d 7h
Merged PRs (30d)
490

Description

## Summary

When `MeshModelAdapter.loadOBJ()` is called with `ctx.materialMode === 'original'`, `this.objLoader.setMaterials(materialsFromMtl)` mutates the shared `OBJLoader2Parallel` instance's internal state. On a subsequent OBJ load where `materialMode` is `'normal'` or `'wireframe'` (or when no `.mtl` sidecar exists), nothing clears the previously set materials, so the second model is rendered with stale materials from the first load, causing visual corruption.

This pitfall was inherited faithfully from the original `LoaderManager.loadModelInternal` `case 'obj':` block during the adapter refactor in #11627 — it is not a regression introduced by that PR.

## Fix

In `src/extensions/core/load3d/MeshModelAdapter.ts` → `loadOBJ()`, add a material reset on the path where `materialMode !== 'original'`:

```ts
// Reset any materials from a prior load before loading without MTL
this.objLoader.setMaterials([])
```

Or, for a simpler/safer approach, construct a fresh `OBJLoader2Parallel` on each call to `loadOBJ` rather than reusing the shared instance.

## Regression test scenario

- Load OBJ #1 with `materialMode: 'original'` and a valid `.mtl` sidecar → materials applied correctly.
- Load OBJ #2 with `materialMode: 'normal'` (no MTL) → should render with standard/normal material, **not** with MTL materials from load #1.

A regression test covering this back-to-back load sequence (with and without a `materialMode` flip, and also the missing-MTL combo) should be added alongside the fix.

## References

- PR where the adapter was introduced: https://github.com/Comfy-Org/ComfyUI_frontend/pull/11627
- Review comment flagging the issue: https://github.com/Comfy-Org/ComfyUI_frontend/pull/11627#discussion_r3142383623

Reported by @jtydhr88.

┆Issue is synchronized with this [Notion page](https://www.notion.so/Issue-11637-fix-load3d-reset-OBJLoader2Parallel-materials-between-loads-to-avoid-stale-state-co-34e6d73d365081d09937ca80e818ec82) by [Unito](https://www.unito.io)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.