Skip to content
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 incorrect rotation matrix after selecting nodes #6978

Merged
merged 7 commits into from
Apr 13, 2023

Conversation

philippotto
Copy link
Member

@philippotto philippotto commented Apr 12, 2023

Nodes that are created in arbitrary mode have an attached non-default rotation. Selecting that node will center their position and change apply their rotation. In orthogonal mode, this shouldn't happen as that rotation is used for bucket picking.

URL of deployed dev instance (used for testing):

Steps to test:

  • switch to flight mode with non-default angles (e.g., 45**3)
  • create some nodes
  • switch back to ortho mode and select nodes
  • data should be rendered without problems

Issues:


(Please delete unneeded items, merge only when none are left open)

@philippotto philippotto self-assigned this Apr 12, 2023
Copy link
Member

@daniel-wer daniel-wer left a comment

Choose a reason for hiding this comment

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

Code LGTM 👍

@daniel-wer
Copy link
Member

daniel-wer commented Apr 12, 2023

I followed the testing instructions and the issue still occured 🤔

image

Edit: The issue actually already occurs when switching back to orthogonal, no node selection needed. Maybe, the additional condition prevents the rotation reset that's triggered when switching to orthogonal?

@philippotto
Copy link
Member Author

philippotto commented Apr 13, 2023

I followed the testing instructions and the issue still occured thinking

Argh, I think I had "prepared" nodes with rotations and after building the fix I only ensured that selecting these nodes didn't cause rendering bugs.

Edit: The issue actually already occurs when switching back to orthogonal, no node selection needed. Maybe, the additional condition prevents the rotation reset that's triggered when switching to orthogonal?

Yes, that was exactly it. Thanks for catching this 🙏 🙏 🙏

Could you have another look for a final thumbs up?

Copy link
Member

@daniel-wer daniel-wer left a comment

Choose a reason for hiding this comment

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

Nice, works well 👍

@philippotto philippotto enabled auto-merge (squash) April 13, 2023 11:10
@philippotto philippotto merged commit bc6443c into master Apr 13, 2023
@philippotto philippotto deleted the fix-changing-rotation branch April 13, 2023 11:28
philippotto added a commit that referenced this pull request Apr 14, 2023
* fix that rotation could be changed in orthogonal mode when selecting nodes that were created in arbitrary mode

* fix spec

* update changelog

* Update frontend/javascripts/oxalis/model/reducers/flycam_reducer.ts

* fix regression which prevented the flycam matrix to be reset when switching to ortho mode
philippotto added a commit that referenced this pull request Apr 14, 2023
* Fix missing buckets when annotation has set rotation value (#6967)

* don't use stored rotation from annotation when mode is set to orthogonal

* update changelog

* Update frontend/javascripts/oxalis/model_initialization.ts

* read viewMode from store instead of from url state

---------

Co-authored-by: Daniel <[email protected]>

* Release 23.04.1

* Fix incorrect rotation matrix after selecting nodes (#6978)

* fix that rotation could be changed in orthogonal mode when selecting nodes that were created in arbitrary mode

* fix spec

* update changelog

* Update frontend/javascripts/oxalis/model/reducers/flycam_reducer.ts

* fix regression which prevented the flycam matrix to be reset when switching to ortho mode

* Release 23.04.2

---------

Co-authored-by: Daniel <[email protected]>
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