-
Notifications
You must be signed in to change notification settings - Fork 21
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.
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
fix: point cloud custom transformation #2550
fix: point cloud custom transformation #2550
Conversation
There were failures in the visual tests workflow. |
There were failures in the visual tests workflow. |
Codecov Report
@@ Coverage Diff @@
## feat/point-cloud-styling #2550 +/- ##
============================================================
- Coverage 70.37% 68.34% -2.04%
============================================================
Files 271 239 -32
Lines 11072 9716 -1356
Branches 1405 1274 -131
============================================================
- Hits 7792 6640 -1152
+ Misses 3123 2918 -205
- Partials 157 158 +1
|
There were failures in the visual tests workflow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just a couple tiny nits.
Co-authored-by: Christopher J. Tannum <christopher.tannum@cognite.com>
Not in commit message
* fix: point cloud bounding box shenanigans * test: add test for point cloud transformation * fix: height visualization for point clouds * chore: add type to test utility function * Update viewer/test-utilities/src/createPointCloudModel.ts Co-authored-by: Christopher J. Tannum <christopher.tannum@cognite.com> * chore: reduce number of characters in code Not in commit message * chore: lint fix Co-authored-by: Christopher J. Tannum <christopher.tannum@cognite.com>
* fix: point cloud bounding box shenanigans * test: add test for point cloud transformation * fix: height visualization for point clouds * chore: add type to test utility function * Update viewer/test-utilities/src/createPointCloudModel.ts Co-authored-by: Christopher J. Tannum <christopher.tannum@cognite.com> * chore: reduce number of characters in code Not in commit message * chore: lint fix Co-authored-by: Christopher J. Tannum <christopher.tannum@cognite.com>
* feat: Add ability to style objects in point clouds (#2104) * feat: point cloud API for styling (#2179) * feat: Webassembly setup (#2353) * Update point cloud feature branch from master (#2416) * feat: Rust/Webassembly octree implementation for faster point-object assignment (#2327) * feat: custom classification (#2320) * docs: finalize point cloud styling docs (#2500) * refactor: move point cloud object provider to data-providers and expose object bounding boxes (#2522) * fix: point cloud custom transformation (#2550) * improvement: various fixes for point clouds (#2551) * fix: export point cloud object metadata (#2552) * Various big and small fixes Co-authored-by: Lars Moastuen <lars.moastuen@cognite.com> Co-authored-by: Christopher J. Tannum <christopher.tannum@cognite.com> Co-authored-by: Savokr <savelii.novikov@cognite.com> Co-authored-by: Pramod S <87521752+pramodcog@users.noreply.github.com>
Description
Point clouds were not translated properly and had their bounding boxes translated wrongly when a custom transformation was applied to them. This fixes that issue, and makes sure that stylable bounding boxes also receive the appropriate transformation when traversed by the user.
A fundamental mistake in the previous code, is that it uses
updateMatrix
on the model, which computes the local matrix fromscale
,position
androtation
, overwriting the matrix we just set. (See https://threejs.org/docs/#manual/en/introduction/Matrix-transformations)Checklist:
Here is a checklist that should completed before merging this given feature.
Any shortcomings from the items below should be explained and detailed within the contents of this PR.