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

Improve handling of movements in 3rd dimension #5801

Merged
merged 11 commits into from
Nov 3, 2021
Merged

Conversation

philippotto
Copy link
Member

@philippotto philippotto commented Oct 29, 2021

This PR removes the old "clamp to edge" behavior on initial third-dimension movements, since this had undesired side effects (such as #5233). As a replacement, the keyboard delay is adapted according to the current position. See code comments for more details.

URL of deployed dev instance (used for testing):

https://fixvisiblemove.webknossos.xyz

Steps to test:

  • open a dataset (ideally with a real-world scale, such as 11,11,24)
  • turn down the move value to something unbearably slow (e.g., 5 nm/s which I allowed temporarily for this PR)
  • play around with the position field (which now supports floats in this PR <-- I will undo this)
    • set the z-value to x.99 and move forward with f (keep the key pressed)
    • set the z-value to x.00 and move forward with f (keep the key pressed)
    • the experienced keyboard delay for the first scenario should be higher than for the second. you can also check the console output to see the calculated delays.

Issues:


@philippotto philippotto self-assigned this Oct 29, 2021
frontend/javascripts/types/schemas/user_settings.schema.js Outdated Show resolved Hide resolved
frontend/javascripts/libs/vector_input.js Outdated Show resolved Hide resolved
Comment on lines 193 to 194
// todo: remove
console.log({ moveDistanceIn1Second, passedFraction });
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
// todo: remove
console.log({ moveDistanceIn1Second, passedFraction });

@philippotto philippotto changed the title [WIP] Improve handling of continuous movements Improve handling of continuous movements Oct 29, 2021
@philippotto philippotto marked this pull request as ready for review October 29, 2021 11:57
@philippotto philippotto changed the title Improve handling of continuous movements Improve handling of z continuous movements Nov 1, 2021
@philippotto philippotto changed the title Improve handling of z continuous movements Improve handling of movements in 3rd dimension Nov 1, 2021
@philippotto
Copy link
Member Author

@fm3 Feel free to check out https://fixvisiblemove.webknossos.xyz to see whether it's an improvement with regard to #5233.

@fm3
Copy link
Member

fm3 commented Nov 1, 2021

Thanks! Yes, my use case is handled better now :)

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.

Very nice, feels exactly right now :)

LGTM once the temporary changes are reversed 👍

@philippotto philippotto enabled auto-merge (squash) November 3, 2021 13:46
@philippotto philippotto merged commit be15eea into master Nov 3, 2021
@philippotto philippotto deleted the fix-visible-move branch November 3, 2021 14:18
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.

Toggling between two slices does not visibly move other viewports
3 participants