Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: make StationaryCameraManager always point the right way #4166

Merged
merged 2 commits into from
Feb 7, 2024

Conversation

haakonflatval-cognite
Copy link
Contributor

Type of change

Bug

Jira ticket 📘

https://cognitedata.atlassian.net/browse/

Description 📝

The calculateNewRotationFromTarget did not make sure the camera's up-vector was preserved.

How has this been tested? 🔍

In Fusion

Test instructions ℹ️

Not that important, but for future reference:

  • link into Fusion
  • Go to 3D in Data Explorer, open christj_maui_A
  • Add 360 image collection Helideck JPEG
  • Click lower-most 360 image icon, and simultaneously the water hose roll by lining them up behind each other
  • Previously (in production) this caused the camera to get a funny angle
  • No more

Checklist ☑️

  • I am proud of this feature.
  • I have performed a self-review of my own code.
  • I have added PR type (Feat, Bug, Chore, Test, Docs, Style, Refactor) to the PR title.
  • I have manually tested this for different scenarios (different models, formats, devices, browsers).
  • I have commented my code in hard-to-understand areas.
  • I have made corresponding changes to the public documentation.
  • I have added unit and visuals tests to prove that my feature works to the best of my ability.
  • I have refactored the code for readability to the best of my ability.
  • I have checked that my changes do not introduce regressions in the public documentation.
  • I have outlined any known defects / lacking capabilities in the contents of this PR.
  • I have listed any remaining work that should follow this PR in the description or jira/miro/etc.
  • I have added TSDoc to any public facing changes.
  • I have added "developer documentation" in any package README.md that is related / applicable to this PR.
  • I have noted down and am currently tracking any technical debt introduced in this PR.
  • I have thoroughly thought about the architecture of this implementation.
  • I have asked peers to test this feature.

@haakonflatval-cognite haakonflatval-cognite requested a review from a team as a code owner February 7, 2024 10:09
Copy link

codecov bot commented Feb 7, 2024

Codecov Report

Merging #4166 (5b75953) into master (b8f1925) will increase coverage by 0.00%.
The diff coverage is 0.00%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4166   +/-   ##
=======================================
  Coverage   73.62%   73.62%           
=======================================
  Files         359      359           
  Lines       35943    35942    -1     
  Branches     2752     2752           
=======================================
  Hits        26463    26463           
+ Misses       9375     9374    -1     
  Partials      105      105           
Files Coverage Δ
...packages/camera-manager/src/CameraManagerHelper.ts 90.05% <0.00%> (+0.52%) ⬆️

Copy link
Contributor

@pramodcog pramodcog left a comment

Choose a reason for hiding this comment

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

Simple & nice!

@haakonflatval-cognite haakonflatval-cognite enabled auto-merge (squash) February 7, 2024 11:12
@haakonflatval-cognite haakonflatval-cognite added the auto-update Makes bulldozer automatically update this PR when there are changes to the target branch label Feb 7, 2024
@haakonflatval-cognite haakonflatval-cognite merged commit 780f195 into master Feb 7, 2024
23 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
auto-update Makes bulldozer automatically update this PR when there are changes to the target branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants