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 10 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
108 changes: 97 additions & 11 deletions src/webgl/loading.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ import './p5.Geometry';
* @param {String} [fileType]
* @return {p5.Geometry} the <a href="#/p5.Geometry">p5.Geometry</a> object
*/

let materials = {}; //currently local
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 use this btw? Looks like we always pass in materials as a parameter

Copy link
Contributor Author

@diyaayay diyaayay Jan 11, 2024

Choose a reason for hiding this comment

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

Ah, yes I had to remove it , missed it though.

p5.prototype.loadModel = function(path) {
p5._validateParameters('loadModel', arguments);
let normalize;
Expand Down Expand Up @@ -150,16 +152,38 @@ p5.prototype.loadModel = function(path) {
} else if (fileType.match(/\.obj$/i)) {
this.loadStrings(
path,
strings => {
parseObj(model, strings);

if (normalize) {
model.normalize();
async lines => {
const mtlLine = lines.find(line => line.startsWith('mtllib '));
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the obj spec supports multiple mtls, so we might need to filter to get all the lines that have mtllib and then parse all of them (maybe waiting for them all via Promise.all(...)?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought there could be just one mtl file to an obj file. After searching it through, yes there can be more. filter should be used . Thanks.


// console.log(mtlPath);
try{
let parsedMaterials=null;
if(mtlLine){
const mtlPath = path.replace(/\.obj$/, '.mtl');
parsedMaterials=await parseMtl(mtlPath); // Pass parsed materials to OBJ parsing
}
if (parsedMaterials){
await parseObj(model,lines, parsedMaterials);
}else {
await parseObj(model,lines); // No MTL file, parse OBJ directly
}
} catch (error) {
if (failureCallback) {
failureCallback(error);
} else {
p5._friendlyError('Error during parsing: ' + error.message);
}
}
finally{

self._decrementPreload();
if (typeof successCallback === 'function') {
successCallback(model);
if (normalize) {
model.normalize();
}

self._decrementPreload();
if (typeof successCallback === 'function') {
successCallback(model);
}
}
},
failureCallback
Expand All @@ -178,6 +202,54 @@ p5.prototype.loadModel = function(path) {
return model;
};

function parseMtl(mtlPath){ //accepts mtlPath to load file
return new Promise((resolve,reject)=>{
// console.log('parser is called');
let currentMaterial = null;
p5.prototype.loadStrings(
Copy link
Contributor

Choose a reason for hiding this comment

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

I think rather than calling this on p5.prototype, which will have no this, you'd want to call loadStrings on a p5 instance. That could mean calling await parseMtl(this, mtlPath); inside loadModel and then call loadStrings on the first argument, although it might be a nicer separation of concerns to separate loading the strings and the parsing, so it becomes:

this.loadStrings(
  mtlPath,
  (mtlLines) => {
    const materials = parseMtl(mtlLines);
    parseObj(model, lines, materials)
  },
  () => {
    // handle failure
  }
)

Another benefit of this is that you maybe don't need parseMtl to resolve or reject, since the loadStrings part handles the case where it can't fetch the file, so parseMtl can look more like parseObj.

mtlPath,
lines => {
for (let line = 0; line < lines.length; ++line){
const tokens = lines[line].trim().split(/\s+/);
if(tokens[0] === 'newmtl') {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice work on this parser, looks great!

const materialName = tokens[1];
currentMaterial = materialName;
materials[currentMaterial] = {};
}else if (tokens[0] === 'Kd'){
//Diffuse color
materials[currentMaterial].diffuseColor = [
parseFloat(tokens[1]),
parseFloat(tokens[2]),
parseFloat(tokens[3])
];
} else if (tokens[0] === 'Ka'){
//Ambient Color
materials[currentMaterial].ambientColor = [
parseFloat(tokens[1]),
parseFloat(tokens[2]),
parseFloat(tokens[3])
];
}else if (tokens[0] === 'Ks'){
//Specular color
materials[currentMaterial].specularColor = [
parseFloat(tokens[1]),
parseFloat(tokens[2]),
parseFloat(tokens[3])
];

}else if (tokens[0] === 'map_Kd') {
//Texture path
materials[currentMaterial].texturePath = tokens[1];
}
}
resolve(materials);
},
reject
);
});

}

/**
* Parse OBJ lines into model. For reference, this is what a simple model of a
* square might look like:
Expand All @@ -189,7 +261,7 @@ p5.prototype.loadModel = function(path) {
*
* f 4 3 2 1
*/
function parseObj(model, lines) {
function parseObj(model, lines, materials= {}) {
// OBJ allows a face to specify an index for a vertex (in the above example),
// but it also allows you to specify a custom combination of vertex, UV
// coordinate, and vertex normal. So, "3/4/3" would mean, "use vertex 3 with
Expand All @@ -205,6 +277,8 @@ function parseObj(model, lines) {
vn: []
};
const indexedVerts = {};
let currentMaterial = null;


for (let line = 0; line < lines.length; ++line) {
// Each line is a separate object (vertex, face, vertex normal, etc)
Expand All @@ -213,7 +287,20 @@ function parseObj(model, lines) {
const tokens = lines[line].trim().split(/\b\s+/);

if (tokens.length > 0) {
if (tokens[0] === 'v' || tokens[0] === 'vn') {
if (tokens[0] === 'usemtl') {
// Switch to a new material
currentMaterial = tokens[1];
if (currentMaterial && materials[currentMaterial]) {
const diffuseColor = materials[currentMaterial].diffuseColor;
model.vertexColors.push([
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we want to add to the diffuse colors for each vertex, where currently this is adding to the array once for each usemtl line. So maybe that means just setting the currentMaterial in this if branch, and in the branch below where we add to loadedVerts, add to vertexColors based on the material.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yes, the logic in my head was a little haywire and that's why I added to the array to vertexColors for each usemtl . Got your point, thanks for explaining the changes to be made also.

diffuseColor[0],
diffuseColor[1],
diffuseColor[2]
]);
} else {
model.vertexColors.push([1, 1, 1]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Just thinking this through: when a model has vertex colors, then fill() has no effect on the model, unless you call .clearColors() on it. Currently, all imported models have no colors, so they take on whatever fill is used. We want to keep that behaviour for objs that have no materials, so maybe that means if no materials are present in the obj, rather than pushing to vertexColors, we just leave it empty.

This would make for a good unit test btw: testing that if you import an obj that has materials, it uses the material colors when you draw it with model(), and when you import an obj that has no materials, it draws using whatever the fill color is.

}
}else if (tokens[0] === 'v' || tokens[0] === 'vn') {
// Check if this line describes a vertex or vertex normal.
// It will have three numeric parameters.
const vertex = new p5.Vector(
Expand Down Expand Up @@ -285,7 +372,6 @@ function parseObj(model, lines) {
if (model.vertexNormals.length === 0) {
model.computeNormals();
}

return model;
}

Expand Down
72 changes: 72 additions & 0 deletions test/unit/assets/octa-color.mtl
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
newmtl m000001
Ns 100
Kd 0 0 0.5
Ks 1 1 1
Ka 0 0 0
Ni 1
d 1
illum 2

newmtl m003627
Ns 100
Kd 0 0 0.942654
Ks 1 1 1
Ka 0 0 0
Ni 1
d 1
illum 2

newmtl m010778
Ns 100
Kd 0 0.815632 1
Ks 1 1 1
Ka 0 0 0
Ni 1
d 1
illum 2

newmtl m012003
Ns 100
Kd 0 0.965177 1
Ks 1 1 1
Ka 0 0 0
Ni 1
d 1
illum 2

newmtl m019240
Ns 100
Kd 0.848654 1 0.151346
Ks 1 1 1
Ka 0 0 0
Ni 1
d 1
illum 2

newmtl m021392
Ns 100
Kd 1 0.888635 0
Ks 1 1 1
Ka 0 0 0
Ni 1
d 1
illum 2

newmtl m022299
Ns 100
Kd 1 0.77791 0
Ks 1 1 1
Ka 0 0 0
Ni 1
d 1
illum 2

newmtl m032767
Ns 100
Kd 0.5 0 0
Ks 1 1 1
Ka 0 0 0
Ni 1
d 1
illum 2

23 changes: 23 additions & 0 deletions test/unit/assets/octa-color.obj
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
mtllib octa-color.mtl
Copy link
Contributor

Choose a reason for hiding this comment

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

The plant obj is 9mb, which might slow down tests. Maybe for the missing material test, we can make a duplicate of this colored obj, but change the path from octa-color.mtl to something that doesn't exist?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, that's what I had thought of too, made the same changes already for plant.obj and will push them asap.

v 1 0 0
v 0 1 0
v 0 0 1
v -1 0 0
v 0 -1 0
v 0 0 -1
usemtl m000001
f 1 5 6
usemtl m003627
f 2 1 6
usemtl m010778
f 1 2 3
usemtl m012003
f 4 2 6
usemtl m019240
f 4 5 3
usemtl m021392
f 5 4 6
usemtl m022299
f 2 4 3
usemtl m032767
f 5 1 3
Loading
Loading