fix: gracefully handle mixed-material OBJ models instead of crashing#8666
Open
Nixxx19 wants to merge 1 commit intoprocessing:dev-2.0from
Open
fix: gracefully handle mixed-material OBJ models instead of crashing#8666Nixxx19 wants to merge 1 commit intoprocessing:dev-2.0from
Nixxx19 wants to merge 1 commit intoprocessing:dev-2.0from
Conversation
|
🎉 Thanks for opening this pull request! For guidance on contributing, check out our contributor guidelines and other resources for contributors! Thank You! |
Previously, `parseObj()` used `hasColoredVertices === hasColorlessVertices`
to detect inconsistent vertex coloring, but this condition is true in two
very different cases:
1. Both flags `false` → model has no faces at all (should never crash)
2. Both flags `true` → model has some faces with material colors and
some without (the genuine "mixed" case)
Real-world OBJ exports from Blender, Sketchfab, and other tools commonly
produce files where only some mesh groups have an explicit MTL material
assignment. The previous `throw` caused a hard crash for any such model,
which is a significant barrier for beginners trying to use pre-made 3D
assets in p5.js.
This commit changes the check to `hasColoredVertices && hasColorlessVertices`
(only the genuinely inconsistent case) and replaces the thrown error with a
`console.warn` + `model.vertexColors = []` reset, so the model still loads
and renders using the default fill color.
The existing test that expected a throw is updated to assert that the model
loads successfully with an empty vertexColors array.
b9edf9e to
82122e1
Compare
Nixxx19
added a commit
to Nixxx19/p5.js
that referenced
this pull request
Mar 22, 2026
- change pr processing#8666 from "merged" to "open" (accurate status) - remove "this is itself a deliverable" over-explanation - remove "this is my honest answer on it" defensive phrasing - rewrite section 11 opener: drop "not because i am unsure" framing - remove draft footer from end of file Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a crash in
parseObj()(loading.js:655-658) that broke loading of real-world OBJ files exported from Blender, Sketchfab, and similar tools.Root cause
The check used to detect inconsistent vertex coloring was:
The
===comparison fires in two completely different situations:falsetrueMany standard OBJ exports have only some mesh groups with an explicit
usemtlentry — this is valid OBJ syntax and common in practice.Fix
Changed the condition to only fire when both flags are
true(the genuinely mixed case), and replaced the thrown error with aconsole.warn+ vertex color strip so the model still loads:Accessibility impact
Beginners loading common 3D assets from Sketchfab or Blender would get an opaque crash message with no path forward. This fix makes those models load and render with the default fill color, keeping the
loadModel()experience frustration-free.Test changes
The existing test
'inconsistent vertex coloring throws error'is updated to assert that a mixed-material model loads successfully and returns aGeometrywithvertexColors.length === 0.All 13 loadModel unit tests pass ✅
Test plan
npx vitest run test/unit/io/loadModel.js— all 13 tests passeg1.obj) now loads successfully and emits aconsole.warn