Skip to content

[Pointcloud] Add per-node priority update in pointclouds#2654

Open
Desplandis wants to merge 1 commit intoiTowns:masterfrom
Desplandis:feat/pointcloud-priority
Open

[Pointcloud] Add per-node priority update in pointclouds#2654
Desplandis wants to merge 1 commit intoiTowns:masterfrom
Desplandis:feat/pointcloud-priority

Conversation

@Desplandis
Copy link
Contributor

@Desplandis Desplandis commented Dec 10, 2025

Description

This PR introduces a priority-based update mechanism for point cloud layers.

Before this PR, the update logic traversed the tree in a depth-first fashion, issued requests during traversal, and only afterward limited the number of point clouds rendered. This often resulted in unnecessary requests and complex update logic.

The new approach uses a priority queue (à la potree) to traverse only the most relevant nodes, sorted by their screen-space size.

Depends #2685.
Blocked by issue #2694 .

Benefits:

  • Limit network requests up front
  • Avoid traversing unimportant parts of the tree
  • Simplify the overall update logic
  • Improve performance and predictability

Motivation and Context

@Desplandis Desplandis force-pushed the feat/pointcloud-priority branch from 0fb1469 to a6ea0ef Compare February 2, 2026 11:13
root.voxelOBB.quaternion.copy(rotation).invert();

root.voxelOBB.updateMatrix();
// Update matrixWorld since OBB is not part of scene hierarchy
Copy link
Contributor

Choose a reason for hiding this comment

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

It is has there is this line

this.object3d.add(this.root.clampOBB);
this.root.clampOBB.updateMatrixWorld(true);

In all PointCloudLayer after we call setOBBes(). So I decided not to call it 2 times...

@Desplandis Desplandis force-pushed the feat/pointcloud-priority branch 2 times, most recently from 3d0d6ea to 6e614f9 Compare February 11, 2026 14:18
@Desplandis Desplandis force-pushed the feat/pointcloud-priority branch from 6e614f9 to a9ac7fe Compare February 11, 2026 14:21
@Desplandis Desplandis marked this pull request as ready for review February 11, 2026 14:21
Comment on lines +122 to +127
const min = this.box3D.min;
const max = this.box3D.max;

const center = new THREE.Vector3()
.addVectors(min, max)
.multiplyScalar(0.5);
Copy link
Contributor

Choose a reason for hiding this comment

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

const center = new THREE.Vector3();
this.box3D.getCenter(center);

.addVectors(min, max)
.multiplyScalar(0.5);

const extents = new THREE.Vector3()
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this really an extents ? I feel like it's more the halfed size of the Box3 then:

const size  = new THREE.Vector3();
const halfSize = this.box3D.getSize(size).multiplyScalar(0.5);

// Get bounding sphere from the OBB's box3D
const box3 = object.box3D;
const sphere = new THREE.Sphere();
box3.getBoundingSphere(sphere);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why using the sphere of the cubique OBB ? there is a clampOBB closer to the data...

return true;
}

getBoundingSphere(sphere: THREE.Sphere = new THREE.Sphere()): THREE.Sphere {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see where you use it. Is it just for having it available ?

}

node.link[layer.id] = helper;
layer.addEventListener('tile-visibility-change', (event) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would say that it's not a bad idea to remove the layer and use event, But I think it should be done in antoher PR and done for the other debuger as well...

@ftoromanoff
Copy link
Contributor

@Desplandis The checks have not all passed...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants