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

Jump to bottom

Handle materials referenced by name specified outside the link object #72

Merged
merged 8 commits into from
Oct 31, 2018
Merged

Handle materials referenced by name specified outside the link object #72

Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b5c129a
STL loader Mesh basic material handling. Material map for links tags …
Oct 12, 2018
d8cac12
Change material node traverse from children to descendents
Oct 12, 2018
f2e22b8
Prevent reloading texture when already loaded for a material name
Oct 13, 2018
ad12889
Material map copy changed properties instead of entire material object
Oct 15, 2018
39f2750
Minor linting correction
Oct 15, 2018
6c73100
Change copy to Object.assign
Oct 15, 2018
38f2c75
Use THREE.Color. Changes to _processMaterial and _copyMaterialAttribu…
Oct 29, 2018
b53fb19
Merge branch 'master' of https://github.com/gkjohnson/urdf-loaders
Oct 30, 2018
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
95 changes: 59 additions & 36 deletions javascript/URDFLoader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,8 @@ class URDFLoader {
if (/\.stl$/i.test(path)) {

this.STLLoader.load(path, geom => {

const mesh = new THREE.Mesh();
mesh.geometry = geom;
const mesh = new THREE.Mesh(geom, new THREE.MeshPhongMaterial());
done(mesh);

});

} else if (/\.dae$/i.test(path)) {
Expand Down Expand Up @@ -210,6 +207,7 @@ class URDFLoader {
// Process the <robot> node
_processRobot(robot, packages, path, loadMeshCb) {

const materials = robot.querySelectorAll('material');
const links = [];
const joints = [];
const obj = new THREE.Object3D();
Expand All @@ -221,15 +219,30 @@ class URDFLoader {
const type = n.nodeName.toLowerCase();
if (type === 'link') links.push(n);
else if (type === 'joint') joints.push(n);
});

// Create the <material> map
const materialMap = {};
this.forEach(materials, m => {
const name = m.getAttribute('name');
this.forEach(m.children, c => {
const material = new THREE.MeshPhongMaterial();
materialMap[name] = materialMap[name] || this._processMaterial(
Copy link
Owner

Choose a reason for hiding this comment

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

Using || like this means that once the first object is created the return this._processMaterial function will never get called and the parameters defined by the second node will never get used. I'd suggest passing an object in to the _processMaterial function. Maybe something like:

const name = m.getAttribute('name');
materialMap[name] = {};
this.forEach(m.children, c => {
    this._processMaterial(materialMap[name], c, packages, path)
});

That way _processMaterial can modify the existing object and add any fields it needs to.

material,
c.nodeName.toLowerCase(),
c.getAttribute('rgba') || c.getAttribute('filename').replace(/^(package:\/\/)/, ''),
packages,
path
);
});
});

// Create the <link> map
const linkMap = {};
this.forEach(links, l => {

const name = l.getAttribute('name');
linkMap[name] = this._processLink(l, packages, path, loadMeshCb);
linkMap[name] = this._processLink(l, materialMap, packages, path, loadMeshCb);

});

Expand Down Expand Up @@ -403,23 +416,41 @@ class URDFLoader {
}

// Process the <link> nodes
_processLink(link, packages, path, loadMeshCb) {
_processLink(link, materialMap, packages, path, loadMeshCb) {

const visualNodes = this.filter(link.children, n => n.nodeName.toLowerCase() === 'visual');
const obj = new THREE.Object3D();
obj.name = link.getAttribute('name');
obj.isURDFLink = true;
obj.type = 'URDFLink';

this.forEach(visualNodes, vn => this._processVisualNode(vn, obj, packages, path, loadMeshCb));
this.forEach(visualNodes, vn => this._processVisualNode(vn, obj, materialMap, packages, path, loadMeshCb));

return obj;

}

// Process the visual nodes into meshes
_processVisualNode(vn, linkObj, packages, path, loadMeshCb) {
_processMaterial(material, type, value, packages, path) {
if (type === 'color') {
const rgba = value.split(/\s/g)
.map(v => parseFloat(v));

material.color.r = rgba[0];
material.color.g = rgba[1];
material.color.b = rgba[2];
material.opacity = rgba[3];

if (material.opacity < 1) material.transparent = true;
} else if (type === 'texture') {
const filename = value.replace(/^(package:\/\/)/, '');
const filePath = this._resolvePackagePath(packages, filename, path);
material.map = this.TextureLoader.load(filePath);
}
return material;
}

// Process the visual nodes into meshes
_processVisualNode(vn, linkObj, materialMap, packages, path, loadMeshCb) {
let xyz = [0, 0, 0];
let rpy = [0, 0, 0];
let scale = [1, 1, 1];
Expand Down Expand Up @@ -515,35 +546,27 @@ class URDFLoader {
rpy = this._processTuple(n.getAttribute('rpy'));

} else if (type === 'material') {
const materialName = n.getAttribute('name');
if (n.children.length) {
this.forEach(n.children, c => {
switch (c.nodeName.toLowerCase()) {

case 'color':
this._processMaterial(material, 'color', c.getAttribute('rgba'));
break;
case 'texture':
if (materialName in materialMap) {
material.copy(materialMap[materialName]);
} else {
this._processMaterial(material, 'texture', c.getAttribute('filename'), packages, path);
}

this.forEach(n.children, c => {

if (c.nodeName.toLowerCase() === 'color') {

const rgba = c.getAttribute('rgba')
.split(/\s/g)
.map(v => parseFloat(v));

material.color.r = rgba[0];
material.color.g = rgba[1];
material.color.b = rgba[2];
material.opacity = rgba[3];

if (material.opacity < 1) material.transparent = true;

} else if (c.nodeName.toLowerCase() === 'texture') {

const filename = c.getAttribute('filename').replace(/^(package:\/\/)/, '');
const filePath = this._resolvePackagePath(packages, filename, path);

material.map = this._textureloader.load(filePath);

}

});

}
});
} else {
material.copy(materialMap[materialName]);
Copy link
Owner

Choose a reason for hiding this comment

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

Using Material.copy concerns me a little bit because it does a lot more than just copy the color and texture and it doesn't work well when using different material types. new THREE.MeshStandardMaterial().copy(new THREE.MeshLambertMaterial()) throws an error, for example, so it's not always safe to call copy on a material of a different type. And if someone has created a custom material as a part of the mesh load function then things like the specular values will be overwritten. It would overwrite the color even if it wasn't specified in the URDF, too.

How do you feel about copying just those specific values:

const sharedMatData = materialMap[materialName];
if (sharedMatData.color) material.color.copy(sharedMatData.color);
if (sharedMatData.map) material.map = sharedMatData.map;

And then the material map could store just those preprocessed values in an object. The color and texture node parsing could be pulled out into separate functions that return the texture and color objects.

}
}

});

// apply the position and rotation to the primitive geometry after
Expand Down