Skip to content

Comments

feat(viewer): add support for pure HTML cluster feature - part 1#5504

Open
danpriori wants to merge 6 commits intomasterfrom
danpriori/BND3D-6392-pure-3d-clustering-360s-HTML-version-part1
Open

feat(viewer): add support for pure HTML cluster feature - part 1#5504
danpriori wants to merge 6 commits intomasterfrom
danpriori/BND3D-6392-pure-3d-clustering-360s-HTML-version-part1

Conversation

@danpriori
Copy link
Contributor

@danpriori danpriori commented Feb 8, 2026

Type of change

Feat

Jira ticket 📘

https://cognitedata.atlassian.net/browse/BND3D-6392

This PR is part of a stacked PR to support the pure HTML cluster feature with an internal counter.

In this PR, I'm adding mainly the new IconOctree method getLODByDistanceWithClustering to get LOD nodes based on camera distance with enhanced clustering for pure cluster visualization.

How has this been tested? 🔍

It was tested by building it locally and testing it on cog-3d in scenes that have a lot of 360s, such as the Hybrid Mapping scene and the Ivar Aasen Demo (CAD, PC, 360).

Test instructions ℹ️

  • Open unified 3d in a scene with a large collection of 360s (example: Hybrid Mapping).
  • Navigate through the 360s and clusters without entering a 360 image.
  • Observe the clusters appearing with the numbering of how many points that cluster represents.
  • Click on one of the clusters.
  • Observe the camera zooming in so that the cluster expands, showing the points it represents.
screen-capture.44.webm

Checklist ☑️

  • I am happy with this implementation.
  • I have performed a self-review of my own code.
  • I have added unit and visuals tests to prove that my feature works to the best of my ability.
  • I have added documentation to new and changed elements; both public and internally shared ones
  • I have refactored the code for testability, readability and extendibility to the best of my ability.
  • I have listed the JIRA tasks covering remaining work or tech debt related to this PR in the description.

@danpriori danpriori requested a review from a team as a code owner February 8, 2026 00:37
@codecov
Copy link

codecov bot commented Feb 8, 2026

Codecov Report

❌ Patch coverage is 82.89474% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.84%. Comparing base (db985e4) to head (372d74b).
⚠️ Report is 6 commits behind head on master.

Files with missing lines Patch % Lines
viewer/packages/360-images/src/types.ts 0.00% 11 Missing ⚠️
viewer/packages/3d-overlays/src/IconOctree.ts 96.92% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5504      +/-   ##
==========================================
- Coverage   72.84%   72.84%   -0.01%     
==========================================
  Files         463      463              
  Lines       46515    46508       -7     
  Branches     3590     3601      +11     
==========================================
- Hits        33884    33878       -6     
+ Misses      12537    12536       -1     
  Partials       94       94              
Files with missing lines Coverage Δ
viewer/packages/3d-overlays/src/IconOctree.ts 94.79% <96.92%> (+0.34%) ⬆️
viewer/packages/360-images/src/types.ts 0.00% <0.00%> (ø)

... and 7 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@danpriori
Copy link
Contributor Author

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new method getLODByDistanceWithClustering to IconOctree for enhanced clustering, along with a new data type and corresponding tests. The implementation is generally solid. However, I've identified a performance concern in the new getNodeSize helper method, where a new Vector3 object is allocated on each call within a loop. My feedback includes a suggestion to optimize this by reusing a Vector3 instance to prevent potential performance degradation from garbage collection.

@pramod-cog pramod-cog self-assigned this Feb 10, 2026
@pramod-cog
Copy link
Contributor

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new type for cluster intersection data and a new method getLODByDistanceWithClustering in IconOctree for enhanced clustering. The new method incorporates node size and maximum depth for better granularity. Tests for this new functionality have been added. However, there is a critical inconsistency regarding the getLODByDistance method: its calls in IconOctree.test.ts have been updated to remove the clusteringLevel parameter, but the method signature in IconOctree.ts still declares and uses this parameter, which will lead to a runtime error or unexpected behavior.

Copy link
Contributor

@nilsfremming nilsfremming left a comment

Choose a reason for hiding this comment

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

I had some comments

…when checking when the camera is close and node has children
Copy link
Contributor

@nilscognite nilscognite left a comment

Choose a reason for hiding this comment

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

LGTM

@danpriori danpriori added the waiting-for-risk-review Waiting for a member of the risk review team to take an action label Feb 17, 2026
@pramod-cog pramod-cog assigned pramod-cog and unassigned pramod-cog Feb 17, 2026
@pramod-cog pramod-cog added the risk-review-ongoing Risk review is in progress label Feb 17, 2026
@pramod-cog
Copy link
Contributor

risk review ok

@pramod-cog pramod-cog added waiting-for-team Waiting for the submitter or reviewer of the PR to take an action and removed waiting-for-risk-review Waiting for a member of the risk review team to take an action labels Feb 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk-review-ongoing Risk review is in progress waiting-for-team Waiting for the submitter or reviewer of the PR to take an action

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants