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

ColladaLoader2: Added axis conversion support #11404

Closed
wants to merge 5 commits into from
Closed

ColladaLoader2: Added axis conversion support #11404

Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
db20491
ColladaLoader2: Added axis conversion support
Mugen87 May 27, 2017
fd88fe9
Merge remote-tracking branch 'upstream/dev' into dev
Mugen87 May 27, 2017
b67040c
ColladaLoader2: Clean up
Mugen87 May 27, 2017
0372bff
ColladaLoader2: More clean up
Mugen87 May 27, 2017
8815ba4
ColladaLoader2: Improved up axis conversion setup
Mugen87 May 30, 2017
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
198 changes: 151 additions & 47 deletions examples/js/loaders/ColladaLoader2.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
THREE.ColladaLoader = function ( manager ) {

this.manager = ( manager !== undefined ) ? manager : THREE.DefaultLoadingManager;
this.options = {
convertUpAxis: false
};

};

Expand All @@ -29,23 +32,16 @@ THREE.ColladaLoader.prototype = {

},

options: {

set convertUpAxis( value ) {

console.log( 'ColladaLoader.options.convertUpAxis: TODO' );

}

},
parse: function ( text, resourceDirectory ) {

setCrossOrigin: function ( value ) {
var scope = this;

this.crossOrigin = value;
// helper objects

},
var matrix = new THREE.Matrix4();
var vector = new THREE.Vector3();

parse: function ( text, resourceDirectory ) {
// helper functions

function getElementsByTagName( xml, name ) {

Expand Down Expand Up @@ -601,7 +597,7 @@ THREE.ColladaLoader.prototype = {

}

console.error( 'ColladaLoader: Undefined sampler', textureObject.id );
console.error( 'THREE.ColladaLoader: Undefined sampler', textureObject.id );

return null;

Expand Down Expand Up @@ -959,12 +955,11 @@ THREE.ColladaLoader.prototype = {
break;

case 'vertices':
// data.sources[ id ] = data.sources[ parseId( getElementsByTagName( child, 'input' )[ 0 ].getAttribute( 'source' ) ) ];
data.vertices = parseGeometryVertices( child );
break;

case 'polygons':
console.warn( 'ColladaLoader: Unsupported primitive type: ', child.nodeName );
console.warn( 'THREE.ColladaLoader: Unsupported primitive type: ', child.nodeName );
break;

case 'lines':
Expand Down Expand Up @@ -1115,21 +1110,21 @@ THREE.ColladaLoader.prototype = {
case 'VERTEX':
for ( var key in vertices ) {

geometry.addAttribute( key.toLowerCase(), buildGeometryAttribute( primitive, sources[ vertices[ key ] ], input.offset ) );
geometry.addAttribute( key.toLowerCase(), convertBufferAttribute( buildGeometryAttribute( primitive, sources[ vertices[ key ] ], input.offset, true ) ) );

}
break;

case 'NORMAL':
geometry.addAttribute( 'normal', buildGeometryAttribute( primitive, sources[ input.id ], input.offset ) );
geometry.addAttribute( 'normal', convertBufferAttribute( buildGeometryAttribute( primitive, sources[ input.id ], input.offset, true ) ) );
break;

case 'COLOR':
geometry.addAttribute( 'color', buildGeometryAttribute( primitive, sources[ input.id ], input.offset ) );
geometry.addAttribute( 'color', buildGeometryAttribute( primitive, sources[ input.id ], input.offset, false ) );
break;

case 'TEXCOORD':
geometry.addAttribute( 'uv', buildGeometryAttribute( primitive, sources[ input.id ], input.offset ) );
geometry.addAttribute( 'uv', buildGeometryAttribute( primitive, sources[ input.id ], input.offset, false ) );
break;

}
Expand Down Expand Up @@ -1227,7 +1222,7 @@ THREE.ColladaLoader.prototype = {

if ( maxcount > 0 ) {

console.log( 'ColladaLoader: Geometry has faces with more than 4 vertices.' );
console.log( 'THREE.ColladaLoader: Geometry has faces with more than 4 vertices.' );

}

Expand All @@ -1253,9 +1248,6 @@ THREE.ColladaLoader.prototype = {

// nodes

var matrix = new THREE.Matrix4();
var vector = new THREE.Vector3();

function parseNode( xml ) {

var data = {
Expand Down Expand Up @@ -1305,12 +1297,12 @@ THREE.ColladaLoader.prototype = {

case 'matrix':
var array = parseFloats( child.textContent );
data.matrix.multiply( matrix.fromArray( array ).transpose() ); // .transpose() when Z_UP?
data.matrix.multiply( convertMatrix4( matrix.fromArray( array ).transpose() ) );
break;

case 'translate':
var array = parseFloats( child.textContent );
vector.fromArray( array );
convertVector3( vector.fromArray( array ) );
data.matrix.multiply( matrix.makeTranslation( vector.x, vector.y, vector.z ) );
break;

Expand All @@ -1322,7 +1314,7 @@ THREE.ColladaLoader.prototype = {

case 'scale':
var array = parseFloats( child.textContent );
data.matrix.scale( vector.fromArray( array ) );
data.matrix.scale( convertVector3( vector.fromArray( array ), 1 ) );
break;

case 'extra':
Expand Down Expand Up @@ -1521,26 +1513,147 @@ THREE.ColladaLoader.prototype = {

}

console.time( 'ColladaLoader' );
// conversion

function setupConversion() {

if ( scope.options.convertUpAxis === true ) {

switch ( asset.upAxis ) {

case 'X_UP':
asset.upConversion = 'X-Y';
Copy link
Owner

@mrdoob mrdoob May 30, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

How about doing this instead?

switch ( asset.upAxis ) {

	case 'X_UP':
		convertVector3 = function ( vector, sign ) {
			if ( sign === undefined ) sign = - 1;
			var tmp = vector.x;
			vector.x = sign * vector.y;
			vector.y = tmp;
			return vector;
		};
		break;

	case 'Y_UP':
		convertVector3 = function ( vector, sign ) {
			return vector;
		};
		break;

	case 'Z_UP':
		convertVector3 = function ( vector, sign ) {
			if ( sign === undefined ) sign = - 1;
			var tmp = vector.y;
			vector.y = vector.z;
			vector.z = sign * tmp;
			return vector;
		};
		break;
}

break;

case 'Y_UP':
asset.upConversion = 'NONE';
break;

case 'Z_UP':
asset.upConversion = 'Z-Y';
break;

default:
console.log( asset.upAxis );

}

}

}

function convertVector3( vector, sign ) {

if ( sign === undefined ) sign = - 1;

if ( scope.options.convertUpAxis === true ) {

switch ( asset.upConversion ) {

case 'X-Y':
var tmp = vector.x;
vector.x = sign * vector.y;
vector.y = tmp;
break;

case 'Z-Y':
var tmp = vector.y;
vector.y = vector.z;
vector.z = sign * tmp;
break;

case 'NONE':
break;

default:
console.error( 'THREE.ColladaLoader: Invalid up axis conversion:', asset.upAxis );

}

}

return vector;

}

function convertBufferAttribute( attribute ) {

// assuming vector3 values

if ( scope.options.convertUpAxis === true ) {

for ( var i = 0, l = attribute.count; i < l; i ++ ) {

vector.fromBufferAttribute( attribute, i );
convertVector3( vector );
attribute.setX( i, vector.x );
attribute.setY( i, vector.y );
attribute.setZ( i, vector.z );

}

}

return attribute;

}

var convertMatrix4 = function() {

var xAxis = new THREE.Vector3();
var yAxis = new THREE.Vector3();
var zAxis = new THREE.Vector3();
var translation = new THREE.Vector3();

return function convertMatrix4( matrix ) {

if ( scope.options.convertUpAxis === true ) {

xAxis.setFromMatrixColumn( matrix, 0 );
yAxis.setFromMatrixColumn( matrix, 1 );
zAxis.setFromMatrixColumn( matrix, 2 );
translation.setFromMatrixColumn( matrix, 3 );

convertVector3( xAxis );
convertVector3( yAxis );
convertVector3( zAxis );
convertVector3( translation );

matrix.set(
xAxis.x, yAxis.x, zAxis.x, translation.x,
xAxis.y, yAxis.y, zAxis.y, translation.y,
xAxis.z, yAxis.z, zAxis.z, translation.z,
0, 0, 0, 1
);

}

return matrix;

};

} ();

console.time( 'THREE.ColladaLoader' );

if ( text.length === 0 ) {

return { scene: new THREE.Scene() };

}

console.time( 'ColladaLoader: DOMParser' );
console.time( 'THREE.ColladaLoader: DOMParser' );

var xml = new DOMParser().parseFromString( text, 'application/xml' );

console.timeEnd( 'ColladaLoader: DOMParser' );
console.timeEnd( 'THREE.ColladaLoader: DOMParser' );

var collada = getElementsByTagName( xml, 'COLLADA' )[ 0 ];

// metadata

var version = collada.getAttribute( 'version' );
console.log( 'ColladaLoader: File version', version );
console.log( 'THREE.ColladaLoader: File version', version );

var asset = parseAsset( getElementsByTagName( collada, 'asset' )[ 0 ] );
var textureLoader = new THREE.TextureLoader( this.manager ).setPath( resourceDirectory );
Expand All @@ -1558,7 +1671,9 @@ THREE.ColladaLoader.prototype = {
visualScenes: {}
};

console.time( 'ColladaLoader: Parse' );
setupConversion();

console.time( 'THREE.ColladaLoader: Parse' );

parseLibrary( collada, 'library_images', 'image', parseImage );
parseLibrary( collada, 'library_effects', 'effect', parseEffect );
Expand All @@ -1569,36 +1684,25 @@ THREE.ColladaLoader.prototype = {
parseLibrary( collada, 'library_nodes', 'node', parseNode );
parseLibrary( collada, 'library_visual_scenes', 'visual_scene', parseVisualScene );

console.timeEnd( 'ColladaLoader: Parse' );
console.timeEnd( 'THREE.ColladaLoader: Parse' );

console.time( 'ColladaLoader: Build' );
console.time( 'THREE.ColladaLoader: Build' );

buildLibrary( library.images, buildImage );
buildLibrary( library.effects, buildEffect );
buildLibrary( library.materials, buildMaterial );
buildLibrary( library.cameras, buildCamera );
buildLibrary( library.lights, buildLight );
buildLibrary( library.geometries, buildGeometry );
// buildLibrary( library.nodes, buildNode );
buildLibrary( library.visualScenes, buildVisualScene );

console.timeEnd( 'ColladaLoader: Build' );

// console.log( library );
console.timeEnd( 'THREE.ColladaLoader: Build' );

var scene = parseScene( getElementsByTagName( collada, 'scene' )[ 0 ] );

if ( asset.upAxis === 'Z_UP' ) {

scene.rotation.x = - Math.PI / 2;
Copy link
Owner

Choose a reason for hiding this comment

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

Here's another though...

Instead of modifying all the vertices, normals, matrices... would it be possible to just rotate the resulting scene? We could then add "flatten" methods if the user needed to convert...?

Copy link
Collaborator Author

@Mugen87 Mugen87 May 31, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

It think this approach should be okay in most cases. But let's say you use OrbitControls and attach a camera object from the collada scene. Then you will get something like this: https://yume.human-interactive.org/examples/collada_current/

There might be more scenarios where similar unexpected behaviors occur. The only way to avoid such problems is a true conversion of all geometry and animation data. And i also think that this solution avoids many support request/bug reports.

We could then add "flatten" methods if the user needed to convert...?

How would this look like?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Mugen87 I still do not see how your code can be correct when you do not "convert" rotations -- but I admit, I may be missing something.

In any event, I agree with @mrdoob, that it may be better to have one method that converts the entire hierarchy at the end, rather than doing it as-you-go. If it is always a simple rotation of the coordinate system, then it should be straight-forward.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps THREE.SceneUtils.convertFromZUp( root );. It would be understood that the method would convert "to y-up".

Maybe, convertToZUp(), too, for exporting. I am not aware of other coordinate systems in use, unless you would want to support left-handed ones. That is a bit messy, but it would make such a method even more useful.

Copy link
Owner

@mrdoob mrdoob May 31, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, converting as you go, even if more efficient, tends to get hairy quickly... The blender exporter may be a good example of that 🙁

Copy link
Collaborator

Choose a reason for hiding this comment

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

All true. If the blender exporter -- or any exporter -- is rewritten, it should avoid piecemeal conversions, and also Euler angles. Use quaternions or matrices, instead.

I do think a utility to convert to/from the three.js coordinate system could be useful.

Copy link
Collaborator Author

@Mugen87 Mugen87 May 31, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@WestLangley What part of the code is not correct from your point of view? And can you please define what you understand under "convert rotations" ? I thought the code does this actually (sry, i'm a little bit uncertain about this topic now).

Just for my understanding 😊 : If we implement a generic convertFromZUp() function, it needs to regard the entire hierarchy of objects and not only the root object, right? And we also have to convert geometry and animation data, correct?

Copy link
Owner

Choose a reason for hiding this comment

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

Correct 👌

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Mugen87 Maybe your handling of rotations was fine. I admit, I have not had the time to think about it sufficiently. Anyway, if you are taking a different approach now, it does not matter, I guess.... Also, the generic convertToZUp() could just involve the root -- I think...


}

scene.scale.multiplyScalar( asset.unit );

console.timeEnd( 'ColladaLoader' );

// console.log( scene );
console.timeEnd( 'THREE.ColladaLoader' );

return {
animations: [],
Expand Down