Skip to content

Commit 6b96fe2

Browse files
Only use d/f switching / dynamic space direction for d/f (#8786)
The d/f switching / dynamic space direction got wonky since rotating the viewports is allowed (see #8614). It impacted the moving via alt+mouse to move in wrong directions. Therefore, this PR disables the dynamic space direction to be only used / applied when using the acutal d/f short cuts. The other possibilities should not be affected by d/f switching anymore. ### URL of deployed dev instance (used for testing): - https://___.webknossos.xyz ### Steps to test: - do the same on master as well as on this branch - Open an annotation. - Brush in z layer X - Brush in z layer x-5 or so (should be smaller than before, to cause the "flipping") - Turn on d/f switching in the left settings drawer - move via alt + mouse. On master this should not move as expected. On this branch, this should be fixed ### Issues: - verbally reported bug ------ (Please delete unneeded items, merge only when none are left open) - [x] Added changelog entry (create a `$PR_NUMBER.md` file in `unreleased_changes` or use `./tools/create-changelog-entry.py`) --------- Co-authored-by: Philipp Otto <philippotto@users.noreply.github.com>
1 parent 99d6ec8 commit 6b96fe2

File tree

6 files changed

+64
-15
lines changed

6 files changed

+64
-15
lines changed

frontend/javascripts/test/reducers/flycam_reducer.spec.ts

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,11 +163,20 @@ describe("Flycam", () => {
163163
let newState = FlycamReducer(initialState, FlycamActions.setDirectionAction([0, 0, -2]));
164164
newState = FlycamReducer(
165165
newState,
166-
FlycamActions.moveFlycamOrthoAction([2, 0, 2], OrthoViews.PLANE_XY),
166+
FlycamActions.moveFlycamOrthoAction([2, 0, 2], OrthoViews.PLANE_XY, true),
167167
);
168168
equalWithEpsilon(getPosition(newState.flycam), [2, 0, -2]);
169169
});
170170

171+
it("should move not in ortho mode with dynamicSpaceDirection if action does not explicity say so", () => {
172+
let newState = FlycamReducer(initialState, FlycamActions.setDirectionAction([0, 0, -2]));
173+
newState = FlycamReducer(
174+
newState,
175+
FlycamActions.moveFlycamOrthoAction([2, 0, 2], OrthoViews.PLANE_XY),
176+
);
177+
equalWithEpsilon(getPosition(newState.flycam), [2, 0, 2]);
178+
});
179+
171180
it("should move by plane in ortho mode (1/3)", () => {
172181
const moveAction = FlycamActions.movePlaneFlycamOrthoAction(
173182
[2, 0, 0],
@@ -202,11 +211,20 @@ describe("Flycam", () => {
202211
let newState = FlycamReducer(initialState, FlycamActions.setDirectionAction([0, 0, -2]));
203212
newState = FlycamReducer(
204213
newState,
205-
FlycamActions.movePlaneFlycamOrthoAction([0, 0, 2], OrthoViews.PLANE_XY, true),
214+
FlycamActions.movePlaneFlycamOrthoAction([0, 0, 2], OrthoViews.PLANE_XY, true, true),
206215
);
207216
equalWithEpsilon(getPosition(newState.flycam), [0, 0, -2]);
208217
});
209218

219+
it("should not move by plane in ortho mode with dynamicSpaceDirection if action does not explicity say so", () => {
220+
let newState = FlycamReducer(initialState, FlycamActions.setDirectionAction([0, 0, -2]));
221+
newState = FlycamReducer(
222+
newState,
223+
FlycamActions.movePlaneFlycamOrthoAction([0, 0, 2], OrthoViews.PLANE_XY, true),
224+
);
225+
equalWithEpsilon(getPosition(newState.flycam), [0, 0, 2]);
226+
});
227+
210228
it("should not change additional coordinates value when layers don't have any", () => {
211229
const newState = FlycamReducer(
212230
initialState,

frontend/javascripts/viewer/controller/combinations/move_handlers.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,11 @@ export const moveU = (deltaU: number): void => {
3636
export const moveV = (deltaV: number): void => {
3737
movePlane([0, deltaV, 0]);
3838
};
39-
export const moveW = (deltaW: number, oneSlide: boolean): void => {
39+
export const moveW = (
40+
deltaW: number,
41+
oneSlide: boolean,
42+
useDynamicSpaceDirection: boolean = false,
43+
): void => {
4044
const state = Store.getState();
4145
if (is2dDataset(state.dataset)) {
4246
return;
@@ -58,10 +62,13 @@ export const moveW = (deltaW: number, oneSlide: boolean): void => {
5862
moveFlycamOrthoAction(
5963
Dimensions.transDim([0, 0, Math.sign(deltaW) * Math.max(1, wStep)], activeViewport),
6064
activeViewport,
65+
useDynamicSpaceDirection,
6166
),
6267
);
6368
} else {
64-
Store.dispatch(movePlaneFlycamOrthoAction([0, 0, deltaW], activeViewport, false));
69+
Store.dispatch(
70+
movePlaneFlycamOrthoAction([0, 0, deltaW], activeViewport, false, useDynamicSpaceDirection),
71+
);
6572
}
6673
};
6774
export function moveWhenAltIsPressed(delta: Point2, position: Point2, _id: any, event: MouseEvent) {

frontend/javascripts/viewer/controller/viewmodes/plane_controller.tsx

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -220,11 +220,18 @@ class BoundingBoxKeybindings {
220220
}
221221
}
222222

223-
function createDelayAwareMoveHandler(multiplier: number) {
223+
function createDelayAwareMoveHandler(
224+
multiplier: number,
225+
useDynamicSpaceDirection: boolean = false,
226+
) {
224227
// The multiplier can be used for inverting the direction as well as for
225228
// speeding up the movement as it's done for shift+f, for example.
226229
const fn = (timeFactor: number, first: boolean) =>
227-
MoveHandlers.moveW(getMoveOffset(Store.getState(), timeFactor) * multiplier, first);
230+
MoveHandlers.moveW(
231+
getMoveOffset(Store.getState(), timeFactor) * multiplier,
232+
first,
233+
useDynamicSpaceDirection,
234+
);
228235

229236
fn.customAdditionalDelayFn = () => {
230237
// Depending on the float fraction of the current position, we want to
@@ -251,7 +258,7 @@ function createDelayAwareMoveHandler(multiplier: number) {
251258
const voxelPerSecond =
252259
state.userConfiguration.moveValue / state.dataset.dataSource.scale.factor[thirdDim];
253260

254-
if (state.userConfiguration.dynamicSpaceDirection) {
261+
if (state.userConfiguration.dynamicSpaceDirection && useDynamicSpaceDirection) {
255262
// Change direction of the value connected to space, based on the last direction
256263
direction *= state.flycam.spaceDirectionOrtho[thirdDim];
257264
}
@@ -441,15 +448,15 @@ class PlaneController extends React.PureComponent<Props> {
441448
// KeyboardJS is sensitive to ordering (complex combos first)
442449
"shift + i": () => VolumeHandlers.changeBrushSizeIfBrushIsActiveBy(-1),
443450
"shift + o": () => VolumeHandlers.changeBrushSizeIfBrushIsActiveBy(1),
444-
"shift + f": createDelayAwareMoveHandler(5),
445-
"shift + d": createDelayAwareMoveHandler(-5),
451+
"shift + f": createDelayAwareMoveHandler(5, true),
452+
"shift + d": createDelayAwareMoveHandler(-5, true),
446453
"shift + space": createDelayAwareMoveHandler(-1),
447454
"ctrl + space": createDelayAwareMoveHandler(-1),
448455
enter: () => Store.dispatch(enterAction()),
449456
esc: () => Store.dispatch(escapeAction()),
450457
space: createDelayAwareMoveHandler(1),
451-
f: createDelayAwareMoveHandler(1),
452-
d: createDelayAwareMoveHandler(-1),
458+
f: createDelayAwareMoveHandler(1, true),
459+
d: createDelayAwareMoveHandler(-1, true),
453460
// Zoom in/out
454461
i: () => MoveHandlers.zoom(1, false),
455462
o: () => MoveHandlers.zoom(-1, false),

frontend/javascripts/viewer/model/actions/flycam_actions.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,23 +101,30 @@ export const setDirectionAction = (direction: Vector3) =>
101101
direction,
102102
}) as const;
103103

104-
export const moveFlycamOrthoAction = (vector: Vector3, planeId: OrthoView | null | undefined) =>
104+
export const moveFlycamOrthoAction = (
105+
vector: Vector3,
106+
planeId: OrthoView | null | undefined,
107+
useDynamicSpaceDirection: boolean = false,
108+
) =>
105109
({
106110
type: "MOVE_FLYCAM_ORTHO",
107111
vector,
108112
planeId,
113+
useDynamicSpaceDirection,
109114
}) as const;
110115

111116
export const movePlaneFlycamOrthoAction = (
112117
vector: Vector3,
113118
planeId: OrthoView,
114119
increaseSpeedWithZoom: boolean = true,
120+
useDynamicSpaceDirection: boolean = false,
115121
) =>
116122
({
117123
type: "MOVE_PLANE_FLYCAM_ORTHO",
118124
vector,
119125
planeId,
120126
increaseSpeedWithZoom,
127+
useDynamicSpaceDirection,
121128
}) as const;
122129

123130
export const moveFlycamAction = (vector: Vector3) =>

frontend/javascripts/viewer/model/reducers/flycam_reducer.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,11 @@ function FlycamReducer(state: WebknossosState, action: Action): WebknossosState
336336
.toArray();
337337

338338
// if planeID is given, use it to manipulate z
339-
if (planeId != null && state.userConfiguration.dynamicSpaceDirection) {
339+
if (
340+
planeId != null &&
341+
state.userConfiguration.dynamicSpaceDirection &&
342+
action.useDynamicSpaceDirection
343+
) {
340344
// change direction of the value connected to space, based on the last direction
341345
deltaInWorldV3 = V3.multiply(deltaInWorldV3, state.flycam.spaceDirectionOrtho);
342346
}
@@ -363,8 +367,12 @@ function FlycamReducer(state: WebknossosState, action: Action): WebknossosState
363367
scaleFactor,
364368
);
365369

366-
if (planeId != null && state.userConfiguration.dynamicSpaceDirection) {
367-
// change direction of the value connected to space, based on the last direction
370+
if (
371+
planeId != null &&
372+
state.userConfiguration.dynamicSpaceDirection &&
373+
action.useDynamicSpaceDirection
374+
) {
375+
// Change direction of the value connected to space, based on the last direction
368376
deltaInWorldZoomed = V3.multiply(deltaInWorldZoomed, state.flycam.spaceDirectionOrtho);
369377
}
370378

unreleased_changes/8786.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
### Changed
2+
- Changed that the d/f switching setting no longer affects movements other than moving via the d/f keyboard shortcuts.

0 commit comments

Comments
 (0)