Improve handling of movements in 3rd dimension

CHANGELOG.unreleased.md

@@ -20,7 +20,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released
 - The context menu in the data viewport also allows to compute an ad-hoc mesh for the selected segment. [#5696](https://github.com/scalableminds/webknossos/pull/5696)
 
 ### Changed
--
+- Improved the movement behavior when using shortcuts to navigate along the 3rd dimension. Instead of clamping the sub-voxel position to .0 or .99 on the initial movement, the keyboard delay is adapted dynamically to avoid too fast movements. This change especially improves the move behavior when quickly changing the direction backwards/forwards. [#5801](https://github.com/scalableminds/webknossos/pull/5801)
 
 ### Fixed
 - Fixed two volume tracing related bugs which could occur when using undo with a slow internet connection or when volume-annotating more than 5000 buckets (32**3 vx) in one session. [#5728](https://github.com/scalableminds/webknossos/pull/5728)

frontend/javascripts/libs/input.js

@@ -34,6 +34,7 @@ type KeyboardLoopHandler = {
   (number, isOriginalEvent: boolean): void,
   delayed: boolean,
   lastTime: ?number,
+  customAdditionalDelayFn?: () => number,
 };
 type KeyboardBindingPress = [KeyboardKey, KeyboardHandler, KeyboardHandler];
 type KeyboardBindingDownUp = [KeyboardKey, KeyboardHandler, KeyboardHandler];
@@ -124,7 +125,7 @@ export class InputKeyboardNoLoop {
 }
 
 // This module is "main" keyboard handler.
-// It is able to handle key-presses and will continously
+// It is able to handle key-presses and will continuously
 // fire the attached callback.
 export class InputKeyboard {
   keyCallbackMap = {};
@@ -185,10 +186,14 @@ export class InputKeyboard {
           this.buttonLoop();
         }
 
-        if (this.delay >= 0) {
+        const totalDelay =
+          this.delay +
+          (callback.customAdditionalDelayFn != null ? callback.customAdditionalDelayFn() : 0);
+        if (totalDelay >= 0) {
           setTimeout(() => {
             callback.delayed = false;
-          }, this.delay);
+            callback.lastTime = new Date().getTime();
+          }, totalDelay);
         }
       },
 

frontend/javascripts/oxalis/controller/combinations/move_handlers.js

@@ -38,15 +38,15 @@ export const movePlane = (v: Vector3, increaseSpeedWithZoom: boolean = true) =>
 
 export const handleMovePlane = (delta: Point2) => movePlane([-delta.x, -delta.y, 0]);
 
-export const moveX = (x: number): void => {
-  movePlane([x, 0, 0]);
+export const moveU = (deltaU: number): void => {
+  movePlane([deltaU, 0, 0]);
 };
 
-export const moveY = (y: number): void => {
-  movePlane([0, y, 0]);
+export const moveV = (deltaV: number): void => {
+  movePlane([0, deltaV, 0]);
 };
 
-export const moveZ = (z: number, oneSlide: boolean): void => {
+export const moveW = (deltaW: number, oneSlide: boolean): void => {
   if (is2dDataset(Store.getState().dataset)) {
     return;
   }
@@ -57,18 +57,17 @@ export const moveZ = (z: number, oneSlide: boolean): void => {
 
   if (oneSlide) {
     const logZoomStep = getRequestLogZoomStep(Store.getState());
-    const w = Dimensions.getIndices(activeViewport)[2];
-    const zStep = getResolutions(Store.getState().dataset)[logZoomStep][w];
+    const wDim = Dimensions.getIndices(activeViewport)[2];
+    const wStep = getResolutions(Store.getState().dataset)[logZoomStep][wDim];
 
     Store.dispatch(
       moveFlycamOrthoAction(
-        Dimensions.transDim([0, 0, (z < 0 ? -1 : 1) * Math.max(1, zStep)], activeViewport),
+        Dimensions.transDim([0, 0, Math.sign(deltaW) * Math.max(1, wStep)], activeViewport),
         activeViewport,
-        true,
       ),
     );
   } else {
-    movePlane([0, 0, z], false);
+    movePlane([0, 0, deltaW], false);
   }
 };
 

frontend/javascripts/oxalis/controller/combinations/tool_controls.js

@@ -56,7 +56,7 @@ export class MoveTool {
       scroll: (delta: number, type: ?ModifierKeys) => {
         switch (type) {
           case null: {
-            MoveHandlers.moveZ(delta, true);
+            MoveHandlers.moveW(delta, true);
             break;
           }
           case "alt":

frontend/javascripts/oxalis/controller/viewmodes/plane_controller.js

@@ -8,6 +8,7 @@ import BackboneEvents from "backbone-events-standalone";
 import * as React from "react";
 import _ from "lodash";
 import api from "oxalis/api/internal_api";
+import dimensions from "oxalis/model/dimensions";
 import {
   deleteActiveNodeAsUserAction,
   createTreeAction,
@@ -19,7 +20,7 @@ import {
 import { InputKeyboard, InputKeyboardNoLoop, InputMouse } from "libs/input";
 import { document } from "libs/window";
 import { getBaseVoxel } from "oxalis/model/scaleinfo";
-import { getRequestLogZoomStep } from "oxalis/model/accessors/flycam_accessor";
+import { getPosition, getRequestLogZoomStep } from "oxalis/model/accessors/flycam_accessor";
 import { listenToStoreProperty } from "oxalis/model/helpers/listener_helpers";
 import { setViewportAction } from "oxalis/model/actions/view_mode_actions";
 import { updateUserSettingAction } from "oxalis/model/actions/settings_actions";
@@ -139,6 +140,64 @@ class VolumeKeybindings {
   }
 }
 
+const getMoveValue = timeFactor => {
+  const state = Store.getState();
+  return (
+    (state.userConfiguration.moveValue * timeFactor) /
+    getBaseVoxel(state.dataset.dataSource.scale) /
+    constants.FPS
+  );
+};
+
+function createDelayAwareMoveHandler(multiplier: number) {
+  // The multiplier can be used for inverting the direction as well as for
+  // speeding up the movement as it's done for shift+f, for example.
+
+  const fn = (timeFactor, first) =>
+    MoveHandlers.moveW(getMoveValue(timeFactor) * multiplier, first);
+
+  fn.customAdditionalDelayFn = () => {
+    // Depending on the float fraction of the current position, we want to
+    // delay subsequent movements longer or shorter.
+    // For example, when being at z=10.0 and keeping `f` pressed, the first
+    // move action will simply set z=11.0. Afterwards, a user-defined keyboard
+    // delay is awaited after which the continuous movement can begin (z=11.1,
+    // z=11.2, ... z=11.9, z=12.0...).
+    // However, doing the same logic with a starting z=10.99 would mean, that
+    // the initial movement bumps z to 11.99 and after the delay has passed,
+    // the slice will immediately switch to 12 (which is too fast).
+    // To compensate this effect, this code here takes the current fraction (and
+    // direction) into account to adapt the initial keyboard delay.
+
+    const state = Store.getState();
+    let direction = Math.sign(multiplier);
+
+    const { activeViewport } = state.viewModeData.plane;
+    const thirdDim = dimensions.thirdDimensionForPlane(activeViewport);
+    const voxelPerSecond =
+      state.userConfiguration.moveValue / state.dataset.dataSource.scale[thirdDim];
+
+    if (activeViewport === OrthoViews.TDView) {
+      // Nothing should happen then, anyway.
+      return 0;
+    }
+
+    if (state.userConfiguration.dynamicSpaceDirection) {
+      // Change direction of the value connected to space, based on the last direction
+      direction *= state.flycam.spaceDirectionOrtho[thirdDim];
+    }
+    const fraction = getPosition(state.flycam)[thirdDim] % 1;
+    const passedFraction = direction === 1 ? fraction : 1 - fraction;
+
+    // Note that a passed fraction of 0 (e.g., z=11.0 and the direction
+    // goes towards 12), means that no additional delay is needed.
+    // The 1000 factor converts to ms.
+    return (1000 / voxelPerSecond) * passedFraction;
+  };
+
+  return fn;
+}
+
 class PlaneController extends React.PureComponent<Props> {
   // See comment in Controller class on general controller architecture.
   //
@@ -266,21 +325,12 @@ class PlaneController extends React.PureComponent<Props> {
       }
     });
 
-    const getMoveValue = timeFactor => {
-      const state = Store.getState();
-      return (
-        (state.userConfiguration.moveValue * timeFactor) /
-        getBaseVoxel(state.dataset.dataSource.scale) /
-        constants.FPS
-      );
-    };
-
     this.input.keyboard = new InputKeyboard({
       // Move
-      left: timeFactor => MoveHandlers.moveX(-getMoveValue(timeFactor)),
-      right: timeFactor => MoveHandlers.moveX(getMoveValue(timeFactor)),
-      up: timeFactor => MoveHandlers.moveY(-getMoveValue(timeFactor)),
-      down: timeFactor => MoveHandlers.moveY(getMoveValue(timeFactor)),
+      left: timeFactor => MoveHandlers.moveU(-getMoveValue(timeFactor)),
+      right: timeFactor => MoveHandlers.moveU(getMoveValue(timeFactor)),
+      up: timeFactor => MoveHandlers.moveV(-getMoveValue(timeFactor)),
+      down: timeFactor => MoveHandlers.moveV(getMoveValue(timeFactor)),
     });
 
     const notLoopedKeyboardControls = this.getNotLoopedKeyboardControls();
@@ -290,19 +340,17 @@ class PlaneController extends React.PureComponent<Props> {
     this.input.keyboardLoopDelayed = new InputKeyboard(
       {
         // KeyboardJS is sensitive to ordering (complex combos first)
-        "shift + f": (timeFactor, first) => MoveHandlers.moveZ(getMoveValue(timeFactor) * 5, first),
-        "shift + d": (timeFactor, first) =>
-          MoveHandlers.moveZ(-getMoveValue(timeFactor) * 5, first),
-
         "shift + i": () => VolumeHandlers.changeBrushSizeIfBrushIsActiveBy(-1),
         "shift + o": () => VolumeHandlers.changeBrushSizeIfBrushIsActiveBy(1),
 
-        "shift + space": (timeFactor, first) =>
-          MoveHandlers.moveZ(-getMoveValue(timeFactor), first),
-        "ctrl + space": (timeFactor, first) => MoveHandlers.moveZ(-getMoveValue(timeFactor), first),
-        space: (timeFactor, first) => MoveHandlers.moveZ(getMoveValue(timeFactor), first),
-        f: (timeFactor, first) => MoveHandlers.moveZ(getMoveValue(timeFactor), first),
-        d: (timeFactor, first) => MoveHandlers.moveZ(-getMoveValue(timeFactor), first),
+        "shift + f": createDelayAwareMoveHandler(5),
+        "shift + d": createDelayAwareMoveHandler(-5),
+
+        "shift + space": createDelayAwareMoveHandler(-1),
+        "ctrl + space": createDelayAwareMoveHandler(-1),
+        space: createDelayAwareMoveHandler(1),
+        f: createDelayAwareMoveHandler(1),
+        d: createDelayAwareMoveHandler(-1),
 
         // Zoom in/out
         i: () => MoveHandlers.zoom(1, false),
@@ -316,7 +364,9 @@ class PlaneController extends React.PureComponent<Props> {
         },
         ...loopedKeyboardControls,
       },
-      { delay: Store.getState().userConfiguration.keyboardDelay },
+      {
+        delay: Store.getState().userConfiguration.keyboardDelay,
+      },
     );
 
     this.input.keyboardNoLoop = new InputKeyboardNoLoop(notLoopedKeyboardControls);

frontend/javascripts/oxalis/model/actions/flycam_actions.js

@@ -17,7 +17,6 @@ type MoveFlycamOrthoAction = {
   type: "MOVE_FLYCAM_ORTHO",
   vector: Vector3,
   planeId: ?OrthoView,
-  clampToEdge: boolean,
 };
 type MovePlaneFlycamOrthoAction = {
   type: "MOVE_PLANE_FLYCAM_ORTHO",
@@ -100,11 +99,7 @@ export const setDirectionAction = (direction: Vector3): SetDirectionAction => ({
 export const moveFlycamOrthoAction = (
   vector: Vector3,
   planeId: ?OrthoView,
-  // If clampToEdge is true, the z coordinate is clamped to the edge (either .0 or 0.999)
-  // so that subsequent, fractional movements go from .0 to .999 (or the other way round).
-  // Only works if planeId is provided.
-  clampToEdge: boolean = false,
-): MoveFlycamOrthoAction => ({ type: "MOVE_FLYCAM_ORTHO", vector, planeId, clampToEdge });
+): MoveFlycamOrthoAction => ({ type: "MOVE_FLYCAM_ORTHO", vector, planeId });
 export const movePlaneFlycamOrthoAction = (
   vector: Vector3,
   planeId: OrthoView,

frontend/javascripts/oxalis/model/reducers/flycam_reducer.js

@@ -5,7 +5,7 @@ import update from "immutability-helper";
 import type { Action } from "oxalis/model/actions/actions";
 import { M4x4, type Matrix4x4 } from "libs/mjs";
 import type { OxalisState } from "oxalis/store";
-import type { Vector3, OrthoView } from "oxalis/constants";
+import type { Vector3 } from "oxalis/constants";
 import { getBaseVoxelFactors } from "oxalis/model/scaleinfo";
 import { getValidZoomRangeForUser } from "oxalis/model/accessors/flycam_accessor";
 import Dimensions from "oxalis/model/dimensions";
@@ -105,32 +105,13 @@ function resetMatrix(matrix: Matrix4x4, dataSetScale: Vector3) {
   return newMatrix;
 }
 
-function moveReducer(
-  state: OxalisState,
-  vector: Vector3,
-  clampToEdge: boolean,
-  planeId: ?OrthoView,
-): OxalisState {
+function moveReducer(state: OxalisState, vector: Vector3): OxalisState {
   const matrix = cloneMatrix(state.flycam.currentMatrix);
   if (!vector.includes(NaN)) {
     matrix[12] += vector[0];
     matrix[13] += vector[1];
     matrix[14] += vector[2];
   }
-  if (clampToEdge && planeId != null) {
-    const dim = Dimensions.getIndices(planeId)[2];
-    if (vector[dim] > 0) {
-      // If the direction is incrementing, we clamp to .0 so that subsequent movements
-      // go from .0 to 0.999 before the next slice is shown.
-      // $FlowIgnore[invalid-tuple-index] Flow does not understand that 12 + dim cannot exceed 14.
-      matrix[12 + dim] = Math.floor(matrix[12 + dim]);
-    } else {
-      // If the direction is decrementing, we clamp to .999 so that subsequent movements
-      // go from .999 to .0 before the next slice is shown.
-      // $FlowIgnore[invalid-tuple-index] Flow does not understand that 12 + dim cannot exceed 14.
-      matrix[12 + dim] = Math.floor(matrix[12 + dim]) + 0.999;
-    }
-  }
   return update(state, { flycam: { currentMatrix: { $set: matrix } } });
 }
 
@@ -251,7 +232,7 @@ function FlycamReducer(state: OxalisState, action: Action): OxalisState {
         const dim = Dimensions.getIndices(planeId)[2];
         vector[dim] *= state.flycam.spaceDirectionOrtho[dim];
       }
-      return moveReducer(state, vector, action.clampToEdge, planeId);
+      return moveReducer(state, vector);
     }
 
     case "MOVE_PLANE_FLYCAM_ORTHO": {
@@ -273,7 +254,7 @@ function FlycamReducer(state: OxalisState, action: Action): OxalisState {
           delta[dim] *= state.flycam.spaceDirectionOrtho[dim];
         }
 
-        return moveReducer(state, delta, false, null);
+        return moveReducer(state, delta);
       }
       return state;
     }

frontend/javascripts/oxalis/view/components/border_toggle_button.js

@@ -44,7 +44,7 @@ function BorderToggleButton(props: Props) {
         its presence interferes with the existing mouse drag / touch drag
         behavior to move tabs around.
         For this reason, we have to call stopPropagation and preventDefault.
-        Additionally, we need to detect whether the user has dragged a tap
+        Additionally, we need to detect whether the user has dragged a tab
         across screen (to move a tab). In that case, onTouchEnd does nothing.
         */
         onClick={event => {

frontend/javascripts/oxalis/view/layouting/layout_persistence.js

@@ -45,7 +45,7 @@ function readStoredLayoutConfigs() {
   try {
     const version = JSON.parse(storedLayoutVersion);
     const layouts = JSON.parse(layoutString);
-    if (currentLayoutVersion > version) {
+    if (currentLayoutVersion !== version) {
       return defaultLayoutConfig;
     }
     if (
@@ -170,7 +170,7 @@ export function setActiveLayout(layoutKey: LayoutKeys, activeLayout: string) {
     persistLayoutConfigsDebounced();
   } else {
     throw new Error(
-      `Active layout could not be set. The given layout ${layoutKey}  with name ${activeLayout} 
+      `Active layout could not be set. The given layout ${layoutKey}  with name ${activeLayout}
       was not found in layouts for ${mapLayoutKeysToLanguage[layoutKey]}.`,
     );
   }

frontend/javascripts/oxalis/view/left-border-tabs/layer_settings_tab.js

@@ -332,7 +332,7 @@ class DatasetSettings extends React.PureComponent<DatasetSettingsProps, State> {
     isDisabled: boolean,
     onChange: (boolean, SyntheticMouseEvent<>) => void,
   ) => (
-    <Tooltip title={isDisabled ? "Enable" : "Disable"} placement="top">
+    <Tooltip title={isDisabled ? "Show" : "Hide"} placement="top">
       {/* This div is necessary for the tooltip to be displayed */}
       <div style={{ display: "inline-block", marginRight: 8 }}>
         <Switch size="small" onChange={onChange} checked={!isDisabled} />

frontend/javascripts/test/api/api_skeleton_latest.spec.js

@@ -153,7 +153,7 @@ test("User Api: setConfiguration should set and get a user configuration value",
 
 test("User Api: setConfiguration should clamp a user configuration value if it is outside of the valid range", t => {
   const { api } = t.context;
-  const MOVE_VALUE = 10;
+  const MOVE_VALUE = 1;
   api.user.setConfiguration("moveValue", MOVE_VALUE);
   t.is(api.user.getConfiguration("moveValue"), userSettings.moveValue.minimum);
 });