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.

Sign up for GitHub

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

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

Show file tree
Hide file tree
Changes from 5 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
144 changes: 78 additions & 66 deletions src/webgl/loading.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ import './p5.Geometry';
* @return {p5.Geometry} the <a href="#/p5.Geometry">p5.Geometry</a> object
*/

let materials = {}; //currently local
p5.prototype.loadModel = function(path) {
p5._validateParameters('loadModel', arguments);
let normalize;
Expand Down Expand Up @@ -150,18 +149,37 @@ p5.prototype.loadModel = function(path) {
failureCallback
);
} else if (fileType.match(/\.obj$/i)) {
const mtlFiles=[];
var mtlPath='';
var parsedMaterials={};
this.loadStrings(
path,
async lines => {
const mtlLine = lines.find(line => line.startsWith('mtllib '));

// console.log(mtlPath);
const mtlPromises = lines
.map(line => line.trim().split(/\b\s+/))
.filter(tokens => tokens[0] === 'mtllib')
.map(async tokens => {
const objFileDir = path.substring(0, path.lastIndexOf('/'));
//because path.dirname does not exist error to get path of mtlfiles

// Replace the file name in the path with the MTL file name
//find a better way to resolve path
if(objFileDir){
mtlPath = objFileDir + '/' + tokens[1];
}
else{
mtlPath=tokens[1];
}
// Check if the MTL file has already been loaded
const existingMtl = mtlFiles.find(mtl => mtl.path === mtlPath);
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 this is a bit of a race condition, since we only push to the array after awaiting an asynchronous function. Maybe you can synchronously push an object that just includes the path, and then after awaiting, add the materials to it? something like:

const file = { path: mtlPath };
mtlFiles.push(file);
const parsedMaterials = await parseMtl(self, mtlPath)
file.materials = parsedMaterials;

if (!existingMtl) {
// If not loaded, load and parse the MTL file
parsedMaterials = await parseMtl(self, mtlPath);
mtlFiles.push({ path: mtlPath, materials: parsedMaterials });
}
});
await Promise.all(mtlPromises);
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 {
Expand All @@ -173,8 +191,7 @@ p5.prototype.loadModel = function(path) {
} else {
p5._friendlyError('Error during parsing: ' + error.message);
}
}
finally{
}finally{

if (normalize) {
model.normalize();
Expand Down Expand Up @@ -202,52 +219,51 @@ 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(
mtlPath,
lines => {
for (let line = 0; line < lines.length; ++line){
const tokens = lines[line].trim().split(/\s+/);
if(tokens[0] === 'newmtl') {
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];
}
async function parseMtl(self,mtlPath){ //accepts mtlPath to load file
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can call self p5 instead so it's clearer what its type should be?

let materials= {};
let currentMaterial = null;
self.loadStrings(
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this function completes without waiting for the result of loadStrings now that it no longer returns a promise. So I think that means it will immediately return {}, and then asynchronously, properties will get added to it. I'm guessing it's important to have the complete materials object before continuing to parse .obj files later, so maybe it would make sense to add back in something like you had before?

const materials = {};
const doneLoading = new Promise((resolve, reject) => {
  self.loadStrings(
    mtlPath,
    lines => {
      // do parsing and add to materials here
      resolve();
    },
    reject
  )
});
await doneLoading;
return materials;

Copy link
Contributor Author

@diyaayay diyaayay Jan 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@davepagurek I changed my multiple mtl file support functionality to this:

else if (fileType.match(/\.obj$/i)) {
    this.loadStrings(
      path,
      async lines => {
        const parsedMaterialPromises=[];
        try{
          for (let i = 0; i < lines.length; i++) {
            const mtllibMatch = lines[i].match(/^mtllib (.+)/);
            if (mtllibMatch) {
              let mtlPath='';
              const mtlFilename = mtllibMatch[1];
              const objPathParts = path.split('/');
              if(objPathParts.length > 1){
                objPathParts.pop();
                const objFolderPath = objPathParts.join('/');
                mtlPath = objFolderPath + '/' + mtlFilename;
              }else{
                mtlPath = mtlFilename;
              }
              const parsedMaterialsIndividual = await parseMtl(self, mtlPath);
              parsedMaterialPromises.push(parsedMaterialsIndividual);
            }
          }
          if(parsedMaterialPromises.length>0){
            const parsedMaterialsArray=
            await Promise.all(parsedMaterialPromises);
            const parsedMaterials = Object.assign({}, ...parsedMaterialsArray);
            await parseObj(model, lines, parsedMaterials);
          } }catch (error) {
          if (failureCallback) {
            failureCallback(error);
          } else {
            p5._friendlyError('Error during parsing: ' + error.message);
          }
        }
        finally{
          if (normalize) {
            model.normalize();
          }

          self._decrementPreload();
          if (typeof successCallback === 'function') {
            successCallback(model);
          }
        }
      },
      failureCallback
    );
  } else {
    p5._friendlyFileLoadError(3, path);
    if (failureCallback) {
      failureCallback();
    } else {
      p5._friendlyError(
        'Sorry, the file type is invalid. Only OBJ and STL files are supported.'
      );
    }
  }
  return model;
};

now single mtl file' materials are fully loaded first and then only passed to parseObj but for multiple files, materials are still being passed before fully loading thus the error: p5.js says: Error during parsing: Cannot read properties of undefined (reading 'diffuseColor')

In the earlier approach even after adding resolve, reject back and some other changes too, still the parseObj was called beforehand.

where is the logic breaking in the current code that single mtl files are passed after materials fully loading but not multiple mtl files? I'm trying to still find since I used Promise.all also.

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 tried using a counter to ensure parseObj is called after parsedMaterials are loaded like this:

const parsedMaterialsIndividual = await parseMtl(self, mtlPath);
              mtlPaths.push(mtlPath);
              mtlfileno++;
              parsedMaterialPromises.push(parsedMaterialsIndividual);

and then,

const parsedMaterials =
            await Object.assign({}, ...parsedMaterialsArray);
            console.log(parsedMaterials);
            if(mtlfileno===mtlPaths.length){
              parseObj(model, lines, parsedMaterials);

Copy link
Contributor

Choose a reason for hiding this comment

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

A few little things:

  • Right now you do const parsedMaterialsIndividual = await parseMtl(self, mtlPath);. By awaiting this, this means synchronously waiting for the material to be loaded/parsed, and the returned value is the actual result and not a promise. This works, but it means all mtl files are loaded one after another, and the Promise.all has no effect due to the results not being promises any more. If you remove the await on this line, the result is a promise, and the code only waits for them to finish at the Promise.all, meaning they all get loaded in parallel, which seems more like what you want.
  • It sounds like right now, awaiting parseMtl doesn't fully wait for it to be loaded. What does your parseMtl implementation look like right now? In the first comment in this chain, I had added some suggestions on how to update parseMtl to await for loading to complete, have you tried any of that out yet?

Let me know if I can help elaborate on any of those points!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@davepagurek below is the parseMtl function:
you had suggested using doneLoading and this is how I'm currently implementing the function:

function parseMtl(p5,mtlPath){ //accepts mtlPath to load file
  return new Promise((resolve, reject)=>{
    let currentMaterial = null;
    let materials= {};
    p5.loadStrings(
      mtlPath,
      lines => {
        for (let line = 0; line < lines.length; ++line){
          const tokens = lines[line].trim().split(/\s+/);
          if(tokens[0] === 'newmtl') {
            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
    );
  });
}

mtlPath,
lines => {
for (let line = 0; line < lines.length; ++line){
const tokens = lines[line].trim().split(/\s+/);
if(tokens[0] === 'newmtl') {
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
);
});

}
},
error => {
p5._friendlyError('Error during parsing: ' + error.message);
}
);
return materials;
}

/**
Expand Down Expand Up @@ -290,16 +306,6 @@ function parseObj(model, lines, materials= {}) {
if (tokens[0] === 'usemtl') {
// Switch to a new material
currentMaterial = tokens[1];
if (currentMaterial && materials[currentMaterial]) {
const diffuseColor = materials[currentMaterial].diffuseColor;
model.vertexColors.push([
diffuseColor[0],
diffuseColor[1],
diffuseColor[2]
]);
} else {
model.vertexColors.push([1, 1, 1]);
}
}else if (tokens[0] === 'v' || tokens[0] === 'vn') {
// Check if this line describes a vertex or vertex normal.
// It will have three numeric parameters.
Expand All @@ -308,6 +314,11 @@ function parseObj(model, lines, materials= {}) {
parseFloat(tokens[2]),
parseFloat(tokens[3])
);
const diffuseColor =
currentMaterial && materials[currentMaterial] ?
materials[currentMaterial].diffuseColor : [1, 1, 1]; // Default to white if no material
model.vertexColors.push(diffuseColor);

loadedVerts[tokens[0]].push(vertex);
} else if (tokens[0] === 'vt') {
// Check if this line describes a texture coordinate.
Expand Down Expand Up @@ -399,6 +410,7 @@ function parseSTL(model, buffer) {
const lineArray = lines.split('\n');
parseASCIISTL(model, lineArray);
}
console.lof(model.vertexcolors);
return model;
}

Expand Down
Loading
Loading