Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.mtl files support for .obj files to render color in 3D custom geometry #6710

Merged
merged 39 commits into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
a47fbbc
fixed unit test
diyaayay Jan 7, 2024
2143dba
removed console logs
diyaayay Jan 7, 2024
3498ad0
changed calculation of diffuse to vertex colors
diyaayay Jan 9, 2024
5c7c07e
Merge branch 'processing:main' into Mtl-support-to-obj-files
diyaayay Jan 9, 2024
b46af55
removed console logs, changed default vertexColors to 1
diyaayay Jan 9, 2024
0b7a1fe
Merge branch 'Mtl-support-to-obj-files' of https://github.com/diyaaya…
diyaayay Jan 9, 2024
77637ba
Fixes vertexColor and mtlpath assignment
diyaayay Jan 10, 2024
1169993
remove console logs
diyaayay Jan 10, 2024
06c5e28
added unit test for loading obj mtl and assigned vertex colors
diyaayay Jan 10, 2024
2b46a55
unit test for missing mtl file
diyaayay Jan 10, 2024
0790146
unit tests for later when vertexColors are properly assigned
diyaayay Jan 13, 2024
1fa08a2
plant.obj to smaller file for testing
diyaayay Jan 13, 2024
be5cf6c
added multiple mtl file handling, still need to fix vertexColors
diyaayay Jan 13, 2024
cd6039d
handles other failing unit tests
diyaayay Jan 13, 2024
32f94f3
Merge branch 'processing:main' into Mtl-support-to-obj-files
diyaayay Jan 14, 2024
16fad39
vertex color assign and duplicate vertices
diyaayay Jan 19, 2024
c3538b7
Merge branch 'Mtl-support-to-obj-files' of https://github.com/diyaaya…
diyaayay Jan 19, 2024
3cb1a13
Merge branch 'main' into Mtl-support-to-obj-files
diyaayay Jan 19, 2024
90a3a53
commit before merging
diyaayay Jan 19, 2024
2fffa53
linting erros , conflicts
diyaayay Jan 19, 2024
48b07a3
had merge conflicts
diyaayay Jan 19, 2024
341b131
Merge remote-tracking branch 'upstream/main' into Mtl-support-to-obj-…
diyaayay Jan 25, 2024
60a3b06
fixes vertexColors to flat array
diyaayay Jan 25, 2024
9538edc
changes duplication logic
diyaayay Jan 25, 2024
56bc7f2
checks for existence of current material
diyaayay Jan 25, 2024
97950c7
fixes unit test for vertexColors being a flat array
diyaayay Jan 25, 2024
4e46332
added visual test for diffuse colors of obj files
diyaayay Jan 26, 2024
104463a
Fixed visual test for diffuse colors
diyaayay Jan 27, 2024
a4047af
screenshot for visual test
diyaayay Feb 1, 2024
7236340
removes console logs
diyaayay Feb 1, 2024
b32c935
normalize model for visual test
diyaayay Feb 2, 2024
19e4655
Merge remote-tracking branch 'upstream/main' into Mtl-support-to-obj-…
diyaayay Feb 2, 2024
14a2420
visual tests work for fill in .obj model and mtl color png fix
diyaayay Feb 2, 2024
687e010
adds check for either a fully colored model or colorless model
diyaayay Feb 4, 2024
2aa937f
Merge remote-tracking branch 'upstream/main' into Mtl-support-to-obj-…
diyaayay Feb 8, 2024
31bdeb9
colored model and unit test for color-no color
diyaayay Feb 9, 2024
ac68110
mtl file not found, obj displayed without materials
diyaayay Feb 12, 2024
d7aa2f7
unit test for missing mtl displayes obj file without color
diyaayay Feb 12, 2024
25bbdc9
Sequential to Parallel mtl file handling
diyaayay Feb 14, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 12 additions & 23 deletions src/webgl/loading.js
Original file line number Diff line number Diff line change
Expand Up @@ -229,18 +229,16 @@ p5.prototype.loadModel = function(path,options) {
const parsedMaterials=await getMaterials(lines);

if(parsedMaterials && typeof parsedMaterials==='object'){
try {
parseObj(model, lines, parsedMaterials);
} catch (error) {
console.error(error.message);
}
parseObj(model, lines, parsedMaterials);
}
}catch (error) {
console.error(error.message);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to console.error here when we _friendlyError below?

Copy link
Contributor Author

@diyaayay diyaayay Feb 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes! I thought of it but was only checking all errors in console while making this work. Sorry for the oversight while pushing. I'll make the changes.

if (failureCallback) {
failureCallback(error);
} else {
p5._friendlyError('Error during parsing: ' + error.message);
}
return;
}
finally{
if (normalize) {
Expand Down Expand Up @@ -351,9 +349,9 @@ function parseObj(model, lines, materials= {}) {
// Map from source index → Map of material → destination index
const usedVerts = {}; // Track colored vertices
let currentMaterial = null;
let uniqueVertices = 0; // all unique vertices count
const coloredVerts = new Set(); //unique vertices with color
let hasColoredVertices = false;
let hasColorlessVertices = false;
for (let line = 0; line < lines.length; ++line) {
// Each line is a separate object (vertex, face, vertex normal, etc)
// For each line, split it into tokens on whitespace. The first token
Expand All @@ -372,9 +370,6 @@ function parseObj(model, lines, materials= {}) {
parseFloat(tokens[2]),
parseFloat(tokens[3])
);
if (tokens[0]==='v'){
uniqueVertices++;
}
loadedVerts[tokens[0]].push(vertex);
} else if (tokens[0] === 'vt') {
// Check if this line describes a texture coordinate.
Expand Down Expand Up @@ -417,7 +412,6 @@ function parseObj(model, lines, materials= {}) {

usedVerts[vertParts[0]][currentMaterial] = vertIndex;
face.push(vertIndex);

if (currentMaterial
&& materials[currentMaterial]
&& materials[currentMaterial].diffuseColor) {
Expand All @@ -439,15 +433,18 @@ function parseObj(model, lines, materials= {}) {
if (currentMaterial
&& materials[currentMaterial]
&& materials[currentMaterial].diffuseColor) {
const materialDiffuseColor =
materials[currentMaterial].diffuseColor;
hasColoredVertices=true;
//flag to track color or no color model
hasColoredVertices = true;
davepagurek marked this conversation as resolved.
Show resolved Hide resolved
const materialDiffuseColor =
materials[currentMaterial].diffuseColor;
for (let i = 0; i < face.length; i++) {
model.vertexColors.push(materialDiffuseColor[0]);
model.vertexColors.push(materialDiffuseColor[1]);
model.vertexColors.push(materialDiffuseColor[2]);
}
}else{
hasColorlessVertices=true;
}
}
}
Expand All @@ -458,19 +455,11 @@ function parseObj(model, lines, materials= {}) {
if (model.vertexNormals.length === 0) {
model.computeNormals();
}
if (hasColoredVertices) {
// Ensure every vertex added has a color
if (coloredVerts.size !== uniqueVertices) {
throw new Error('Not all vertices have a color.');
}
} else {
// If no vertices are supposed to have color, ensure coloredVerts is empty
if (coloredVerts.size > 0) {
throw new Error('Unexpected colored vertices found.');
}
if (hasColoredVertices === hasColorlessVertices) {
// If both are true or both are false, throw an error because the model is inconsistent
throw new Error('Model coloring is inconsistent. Either all vertices should have colors or none should.');
}


return model;
}

Expand Down
9 changes: 9 additions & 0 deletions test/unit/assets/eg1.mtl
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
newmtl coloredMaterial
Ns 96.078431
Ka 1.000000 1.000000 1.000000
Kd 1.000000 0.000000 0.000000 # Only this material has a diffuse color
Ks 0.500000 0.500000 0.500000
Ke 0.0 0.0 0.0
Ni 1.000000
d 1.000000
illum 2
16 changes: 16 additions & 0 deletions test/unit/assets/eg1.obj
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
mtllib eg1.mtl

v 0 0 0
v 1 0 0
v 1 1 0
v 0 1 0
v 0.5 0.5 1

# Define faces without material
f 1 2 5
f 2 3 5

# Apply material to subsequent faces
usemtl coloredMaterial
f 1 4 5
f 4 1 5
37 changes: 20 additions & 17 deletions test/unit/io/loadModel.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ suite('loadModel', function() {
var validFile = 'unit/assets/teapot.obj';
var validObjFileforMtl='unit/assets/octa-color.obj';
var validSTLfile = 'unit/assets/ascii.stl';
// const missingMtltoObjFile= 'unit/assets/plant.obj';
var inconsistentColorObjFile = 'unit/assets/eg1.obj';
var validSTLfileWithoutExtension = 'unit/assets/ascii';

test('_friendlyFileLoadError is called', async function() {
Expand Down Expand Up @@ -100,7 +100,7 @@ suite('loadModel', function() {
};
});

test.only('loads OBJ file with associated MTL file correctly', async function(){
test('loads OBJ file with associated MTL file correctly', async function(){
const model = await promisedSketch(function (sketch,resolve,reject){
sketch.preload=function(){
sketch.loadModel(validObjFileforMtl,resolve,reject);
Expand Down Expand Up @@ -134,21 +134,24 @@ suite('loadModel', function() {
];
assert.deepEqual(model.vertexColors,expectedColors);
});
// test('throws an error for missing MTL file specified in OBJ file', async function() {
// try {
// await promisedSketch(function (sketch, resolve, reject) {
// sketch.preload = function () {
// sketch.loadModel(missingMtltoObjFile, resolve, reject);
// };
// });
// // If the promise resolves without throwing an error, fail the test
// assert.fail('Expected an error for missing MTL file, but the promise resolved successfully');
// } catch (error) {
// // Check if the error message indicates a missing MTL file
// console.log(error);
// assert.include(error.message, 'MTL file not found', 'Unexpected error message');
// }
// });
test('inconsistent vertex coloring throws error', async function() {
// Attempt to load the model and catch the error
let errorCaught = null;
try {
await promisedSketch(function(sketch, resolve, reject) {
sketch.preload = function() {
sketch.loadModel(inconsistentColorObjFile, resolve, reject);
};
});
} catch (error) {
errorCaught = error;
}

// Assert that an error was caught and that it has the expected message
assert.instanceOf(errorCaught, Error, 'No error thrown for inconsistent vertex coloring');
assert.equal(errorCaught.message, 'Model coloring is inconsistent. Either all vertices should have colors or none should.', 'Unexpected error message for inconsistent vertex coloring');
});

test('returns an object with correct data', async function() {
const model = await promisedSketch(function(sketch, resolve, reject) {
var _model;
Expand Down
Loading