Skip to content

Commit b9edf9e

Browse files
Nixxx19claude
andcommitted
fix: gracefully handle mixed-material OBJ models instead of crashing
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. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 83415b7 commit b9edf9e

File tree

2 files changed

+25
-8
lines changed

2 files changed

+25
-8
lines changed

src/webgl/loading.js

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -652,9 +652,19 @@ function loading(p5, fn){
652652
if (model.vertexNormals.length === 0) {
653653
model.computeNormals();
654654
}
655-
if (hasColoredVertices === hasColorlessVertices) {
656-
// If both are true or both are false, throw an error because the model is inconsistent
657-
throw new Error('Model coloring is inconsistent. Either all vertices should have colors or none should.');
655+
if (hasColoredVertices && hasColorlessVertices) {
656+
// Mixed model: some faces have a material diffuse color assigned, others do not.
657+
// This is common in real-world OBJ exports (e.g. Blender, Sketchfab) where only
658+
// some mesh groups have an explicit MTL material. Rather than crashing the sketch,
659+
// we degrade gracefully: strip the partial vertex colors so the model renders
660+
// with the default fill color instead of corrupted per-vertex coloring.
661+
console.warn(
662+
'p5.js: This OBJ model has mixed material coloring — some faces have a ' +
663+
'material diffuse color and some do not. Vertex colors will not be applied. ' +
664+
'Consider assigning a material to every face group in your 3D software, ' +
665+
'or use a model where all faces share the same material.'
666+
);
667+
model.vertexColors = [];
658668
}
659669

660670
return model;

test/unit/io/loadModel.js

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -79,11 +79,18 @@ suite('loadModel', function() {
7979
assert.deepEqual(model.vertexColors, expectedColors);
8080
});
8181

82-
test('inconsistent vertex coloring throws error', async function() {
83-
// Attempt to load the model and catch the error
84-
await expect(mockP5Prototype.loadModel(inconsistentColorObjFile))
85-
.rejects
86-
.toThrow('Model coloring is inconsistent. Either all vertices should have colors or none should.');
82+
test('mixed material coloring loads model with empty vertexColors instead of crashing', async function() {
83+
// eg1.obj has some faces without a material and some with one.
84+
// Real-world exports from Blender/Sketchfab frequently produce this structure.
85+
// The loader should degrade gracefully rather than throwing, so beginners'
86+
// sketches don't crash when loading common 3D assets.
87+
const model = await mockP5Prototype.loadModel(inconsistentColorObjFile);
88+
assert.instanceOf(model, Geometry);
89+
assert.equal(
90+
model.vertexColors.length,
91+
0,
92+
'Mixed-material model should have no vertex colors (graceful degradation)'
93+
);
8794
});
8895

8996
test('missing MTL file shows OBJ model without vertexColors', async function() {

0 commit comments

Comments
 (0)