ColladaLoader object position/center swapped values

BenedictLang commented 2 years ago

Context: Im programming a 3D Room Search PWA for my university

Describe the bug

I was moving from OBJLoader to ColladaLoader and recognized, that my added Labels from CSS2DRenderer had wrong positions when they are correct in the OBJ file.

To Reproduce

Steps to reproduce the behavior:

Load collada.dae file
get Position of element
get geometry.boundingBox.getCenter(matrix);
Value Y is swapped with actual value Z

Code Context: the function creates a tooltip to show the room number linked as a label to the position above the searched room in my scene.


/**
 * Adds a tooltip above a room with the given room number if exists in model
 * @param roomNumber object.name value that is compared to the given room number
 */
export function addTooltip(roomNumber) {

    const roomCard_tooltip = createRoomCard(returnRoom(roomNumber), true) //some HTML content for the tooltip
    roomCard_tooltip.style.marginTop = '-10em'; //set center position up
    tooltip = new CSS2DObject( roomCard_tooltip ) //add as global instance of label
    const room = scene.getObjectByName(roomNumber); //return RoomID of room in my model
    if (room) {
        let offset = null;
        console.log(`Room:  ${room}`)
        console.log(`Room init pos:  ${room.position.x}, ${room.position.y}, ${room.position.z}`) //is always 0, 0, 0 because not set
        if (room.position.x === 0 && room.position.z === 0) { //if coordinates not already set
            room.geometry.computeBoundingBox();
            let matrix = new THREE.Vector3();
            offset = room.geometry.boundingBox.getCenter(matrix); //compute center point of room
            console.log(`Room center:  ${offset.x}, ${offset.y}, ${offset.z}`)
            room.geometry.applyMatrix4(new THREE.Matrix4().makeTranslation(-offset.x, -offset.y, -offset.z));
            room.position.copy(offset); //set position from offset data
        }

        console.log(`Room pos:  ${room.position.x}, ${room.position.y}, ${room.position.z}`)
        room.material = new THREE.MeshStandardMaterial( { color: 0xE31433}); //sets color of room highlighted
        tooltip.position.copy( room.position ); // ERROR!!!!!!!!!!!!!!, position is not correct when loaded with ColladaLoader()
        addLightPoint(room.position.x, room.position.y, room.position.z, scene) //highlight position of initial room point
        if (offset != null) tooltip.position.set( offset.x, offset.z, - offset.y); // SOLUTION: swapped Z and Y value
        scene.add(tooltip)
        console.log(`tooltip pos:  ${tooltip.position.x}, ${tooltip.position.y}, ${tooltip.position.z}`)

        addLightPoint(tooltip.position.x, tooltip.position.y, tooltip.position.z, scene) //highlight position of tooltip point

    } else {
        console.error("Sorry, can't find room in object!")
    }

/**
 * Adds a light point to the position given
 * @param $x X-coordinate
 * @param $y Y-coordinate
 * @param $z Z-coordinate
 * @param scene the scene/context
 */
export function addLightPoint($x, $y, $z, scene) {
    const pointLight = new THREE.PointLight( 0xff0000, 1, 200 );
    pointLight.position.set( $x, $y, $z );
    scene.add( pointLight );

    const sphereSize = 1;
    const pointLightHelper = new THREE.PointLightHelper( pointLight, sphereSize );
    scene.add( pointLightHelper );
}

Simple Loader:

let daeModelHdM = "" + new URL("../assets/models/dae/HdM_campus.dae",
    import.meta.url);

const loader = new ColladaLoader(manager);
loader.load( daeModelHdM, function ( collada ) {

    const object = collada.scene;
    const animations = object.animations;

    object.traverse( function ( child ) {
        if (child.isMesh) { //later and if is room or building
            //TODO: sorting into storys and buildings 2D-array

           //child.name <=> roomNumber/building

            objects.push(child); //saved in global instance
        }
    } );

    scene.add( object );

    let scaleFactor = 1;
    object.scale.set(scaleFactor, scaleFactor, scaleFactor);
    //object.rotateX(11);

    }, (xhr) => {
        console.debug(`${((xhr.loaded / xhr.total) * 100).toFixed(2)}% of model loaded`)
    }, function (error) {
        console.error(`An error happened: ${error}`);
    }
);

Live example

limited available project showcase might be not up to date, maybe on 30th of June 22
later if online

Expected behavior

Same position calculation with the other loaders (tested with OBJLoader and Rhino3dmLoader). I havent seen an obvious issue in the position of ColladaLoader code on a quick look so I guess the swapped values are in the getCenter, boundingBox or geometry.

Screenshots

Big PointLightHelper is 0,0,0 (Ground Zero, initial position) - small PointLightHelper is 21, 76, 10 (the collada room center -> wrong!) - tooltip + PointLightHelper is 21, 10 , 76 (if changed values correct position)

Platform:

Device: Desktop, Mobile
OS: Windows 11, MacOS, Android
Browser: Chrome
Three.js version: "three": "^0.140.0"
ColladaLoader: "three/examples/jsm/loaders/ColladaLoader"

Mugen87 commented 2 years ago

I was moving from OBJLoader to ColladaLoader

Consider to use glTF instead which is the better choice compared to Collada (and OBJ). More information about this topic in the following guide: https://threejs.org/docs/index.html#manual/en/introduction/Loading-3D-models

Value Y is swapped with actual value Z

Sounds like an issue in context of the up-vector of your models. Do you mind sharing one of the Collada files in this thread?

I could imagine that the error goes automatically away when using glTF (since it only supports Y-up).

BenedictLang commented 2 years ago

@Mugen87 I was trying now almost all loaders and ended up with collada because i had issues with the other formats. Im building the model in ArchiCAD but unfortunatly they are somehow not capable of not touching IDs, names, class and whatever and they also dont support exporting them in any file type. Thats why I add the labels for the rooms manually in blender. Communicating between them both was best in collada files. -> I will try again and see what the issues with gltf were as it was the first type I have tested.

Interesting, so it might be a bug in blender or archicad export. I would have thought the position info is 0 anyway for all objects and calculated by the loader/ThreeJs.

Thats the current model: DAE Model from local filestore server(valid until Dec 22)

Thanks for your fast help dude, love the library and constant maintain, its great although it took a while to make all the beginner mistakes and im not yet at the end :D If the project is live I will move it from gitlab to github if you are interested, as I haven't found real similar ThreeJS examples, close are Expo Dubai and Amnesty International Prison

BenedictLang commented 2 years ago

Consider to use glTF instead which is the better choice compared to Collada (and OBJ). More information about this topic in the following guide: https://threejs.org/docs/index.html#manual/en/introduction/Loading-3D-models

@Mugen87 So I swapped back to gltf and at least the export from blender does work well with it, GLB handles some things different but I will have a look if it is an issue for me. Also here with glb file and GLTFLoader the coordinates are correct.

I updated the file on the server with the current state.

Mugen87 commented 2 years ago

After reading some issues and PRs I remember that we've decided not to add a real up-axis conversion to ColladaLoader. We've given it a try in #11404 and #11540 but come to the conclusion that it's better to perform the conversion during the export. And not when loading the asset.

The loader just rotates the entire asset to display it correctly. However, the coordinates are of course not "as expected".

In other words, this issue is not going to be fixed in the loader. You have to perform the axis conversion when exporting to Collada. However, I recommend not doing this but keep using glTF even if that means you have to change your workflow. But you definitely end up with the better solution.

BenedictLang commented 2 years ago

In other words, this issue is not going to be fixed in the loader. You have to perform the axis conversion when exporting to Collada. However, I recommend not doing this but keep using glTF even if that means you have to change your workflow. But you definitely end up with the better solution.

Alright got you, thanks for pointing that out and for your help. Unfortunately you dont always search for the right things to find an existing issue.

I presented the project in my university in Stuttgart today and it worked well with GLB.

Schöne Grüße und vielen Dank 😉

mrdoob commented 2 years ago

@Mugen87

The loader just rotates the entire asset to display it correctly. However, the coordinates are of course not "as expected".

Maybe we should add a warning in the console when we do that?

Mugen87 commented 2 years ago

Yeah, that would help to avoid confusion. I'm struggling a bit with the wording though^^. How about:

THREE.ColladaLoader: You are loading an asset with a Z-UP coordinate system. The loader just rotates the asset to transform it into Y-UP. The vertex data are not converted, see https://github.com/mrdoob/three.js/issues/24289.

mrdoob commented 2 years ago

@Mugen87 That sounds good 👍

mrdoob / three.js

ColladaLoader object position/center swapped values #24289