LoadProject with hierarchical annotations (like annotations_performance.html)

[ilisalis]

Dans LoadProject.js, la fonction loadAnnotations ne gère pas le chargement d'annotations hiérarchiques.

A proposed correction, it considers the possibility of loading lower nodes for an already existing annotation :

function loadAnnotations(viewer, data) {
    if(!data || data.length === 0){
        return;
    }
    const existingAnnotations = [];
    viewer.scene.annotations.traverseDescendants(annotation => {
        existingAnnotations.push(annotation);
    });

    let loadAnnotationItem = (item, annotationParent) => {
        const duplicate = existingAnnotations.find(ann => ann.uuid === item.uuid);
        if(duplicate){
            if(item.children) {
                for(let child of item.children) {
                    loadAnnotationItem(child, duplicate);
                }
            }
        } else {
            const annotation = new Annotation({
                position: item.position,
                title: item.title,
            });

            annotation.description = item.description;
            annotation.uuid = item.uuid;

            if(item.cameraPosition)
                annotation.cameraPosition = new THREE.Vector3(...item.cameraPosition);

            if(item.cameraTarget)
                annotation.cameraTarget = new THREE.Vector3(...item.cameraTarget);

            if(item.offset)
                annotation.offset.set(...item.offset);

            if(item.children) {
                for(let child of item.children) {
                    loadAnnotationItem(child, annotation);
                }
            }

            existingAnnotations.push(annotation);
            annotationParent.add(annotation);
        }
    };

    for(const item of data){
        loadAnnotationItem(item, viewer.scene.annotations);
    }
}

[midnight-dev]

You make a compelling point. Currently, if there are any duplicates found, they're skipped so they won't have a collision. However, that assumes that each annotation has no children annotations. I'm not sure how often people actually make annotations of annotations (and then load/import them later on), but we could stand to have it be as robust as possible.

Some minor notes. loadAnnotationItem could be defined as a constant and it would be easier on my eyes if formatting was consistent. A space can make a big difference with readability, at least for me.

More importantly, I feel like there's likely a better way to run this recursion. I'm not expert in algorithms, but if loadAnnotationItem is called for both the case where there is a duplicate found and when no duplicate is found, couldn't it be called once after the check for duplicates? I dunno. Might have to revisit optimization later.

[ilisalis]

For recursion, there may be a way to do it differently; however, this will be at the expense of readability, without being more effective. The important thing is that one must create an annotation and add it to its parent, the other must not; while continuing to treat the children.

Otherwise, it is only a proposal. In general, I use little variables in my codes. I don't really have an opinion on formatting (it already changes a lot depending on the parts of the project).

[midnight-dev]

I agree, and although I haven't spent much time on it, I couldn't come up with a more efficient method. I say submit it 👍