Check user setting boundaries in reducer to avoid invalid values (#3861)

* check user setting boundaries in reducer to avoid invalid values

* update changelog

* avoid double definition of user settings boundaries

* do not round zoom in url hash

* send rendering errors to airbrake

* avoid remount of controller by setting proper error state in componentDidCatch

CHANGELOG.md

@@ -16,7 +16,8 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.md).
 - Tweaked the highlighting of the active node. The inner node looks exactly as a non-active node and is not round, anymore. An active node is circled by a "halo". In arbitrary mode, the halo is hidden and the active node is round. [#3868](https://github.com/scalableminds/webknossos/pull/3868)
 
 ### Fixed
-- 
+- Fixed the setting which enables to hide the planes within the 3D viewport. [#3857](https://github.com/scalableminds/webknossos/pull/3857)
+- Fixed a bug which allowed the brush size to become negative when using shortcuts. [#3861](https://github.com/scalableminds/webknossos/pull/3861)
 
 ### Removed
 - 

frontend/javascripts/dashboard/dataset/validation.js

@@ -3,7 +3,7 @@
 import jsonschema from "jsonschema";
 
 import DatasourceSchema from "libs/datasource.schema.json";
-import UserSettingsSchema from "libs/user_settings.schema.json";
+import UserSettingsSchema from "libs/user_settings.schema";
 
 const validator = new jsonschema.Validator();
 validator.addSchema(DatasourceSchema, "/");

frontend/javascripts/libs/user_settings.schema.js

@@ -0,0 +1,50 @@
+// @flow
+
+export const userSettings = {
+  clippingDistance: { type: "number", minimum: 1, maximum: 12000 },
+  clippingDistanceArbitrary: { type: "number", minimum: 1, maximum: 127 },
+  crosshairSize: { type: "number", minimum: 0.05, maximum: 0.5 },
+  displayCrosshair: { type: "boolean" },
+  displayScalebars: { type: "boolean" },
+  dynamicSpaceDirection: { type: "boolean" },
+  keyboardDelay: { type: "number", minimum: 0, maximum: 500 },
+  mouseRotateValue: { type: "number", minimum: 0.0001, maximum: 0.02 },
+  moveValue: { type: "number", minimum: 30, maximum: 14000 },
+  moveValue3d: { type: "number", minimum: 30, maximum: 14000 },
+  newNodeNewTree: { type: "boolean" },
+  highlightCommentedNodes: { type: "boolean" },
+  // The node radius is the actual radius of the node in nm, it's dependent on zoom and dataset scale
+  nodeRadius: { type: "number", minimum: 1, maximum: 5000 },
+  overrideNodeRadius: { type: "boolean" },
+  // The particle size is measured in pixels - it's independent of zoom and dataset scale
+  particleSize: { type: "number", minimum: 1, maximum: 20 },
+  radius: { type: "number", minimum: 1, maximum: 5000 },
+  rotateValue: { type: "number", minimum: 0.001, maximum: 0.08 },
+  sortCommentsAsc: { type: "boolean" },
+  sortTreesByName: { type: "boolean" },
+  sphericalCapRadius: { type: "number", minimum: 50, maximum: 500 },
+  tdViewDisplayPlanes: { type: "boolean" },
+  hideTreeRemovalWarning: { type: "boolean" },
+  fourBit: { type: "boolean" },
+  interpolation: { type: "boolean" },
+  quality: { type: "number", enum: [0, 1, 2] },
+  loadingStrategy: { enum: ["BEST_QUALITY_FIRST", "PROGRESSIVE_QUALITY"] },
+  segmentationOpacity: { type: "number", minimum: 0, maximum: 100 },
+  highlightHoveredCellId: { type: "boolean" },
+  zoom: { type: "number", minimum: 0.005 },
+  renderMissingDataBlack: { type: "boolean" },
+  brushSize: { type: "number", minimum: 5, maximum: 5000 },
+  layoutScaleValue: { type: "number", minimum: 1, maximum: 5 },
+  autoSaveLayouts: { type: "boolean" },
+};
+
+export default {
+  $schema: "http://json-schema.org/draft-06/schema#",
+  definitions: {
+    "types::UserSettings": {
+      type: ["object", "null"],
+      properties: userSettings,
+      additionalProperties: false,
+    },
+  },
+};

frontend/javascripts/messages.js

@@ -10,6 +10,7 @@ export const settings = {
   moveValue: "Move Value (nm/s)",
   newNodeNewTree: "Single-node-tree mode (Soma clicking)",
   highlightCommentedNodes: "Highlight Commented Nodes",
+  nodeRadius: "Node Radius",
   overrideNodeRadius: "Override Node Radius",
   particleSize: "Particle Size",
   tdViewDisplayPlanes: "Display Planes in 3D View",

frontend/javascripts/oxalis/constants.js

@@ -159,26 +159,8 @@ const Constants = {
   LOOK_UP_TEXTURE_WIDTH: 256,
 
   TDView_MOVE_SPEED: 150,
-  MIN_MOVE_VALUE: 30,
-  MAX_MOVE_VALUE: 14000,
-  MAX_MOVE_VALUE_SLIDER: 1500,
-
   FPS: 50,
 
-  MIN_LAYOUT_SCALE: 1,
-  MAX_LAYOUT_SCALE: 5,
-
-  MIN_BRUSH_SIZE: 5,
-  MAX_BRUSH_SIZE: 5000,
-
-  // The node radius is the actual radius of the node in nm, it's dependent on zoom and dataset scale
-  MIN_NODE_RADIUS: 1,
-  MAX_NODE_RADIUS: 5000,
-
-  // The particle size is measured in pixels - it's independent of zoom and dataset scale
-  MIN_PARTICLE_SIZE: 1,
-  MAX_PARTICLE_SIZE: 20,
-
   ZOOM_DIFF: 0.1,
 
   DEFAULT_SPHERICAL_CAP_RADIUS: 140,

frontend/javascripts/oxalis/controller.js

@@ -163,9 +163,7 @@ class Controller extends React.PureComponent<PropsWithRouter, State> {
   }
 
   setLayoutScale(multiplier: number): void {
-    let scale = Store.getState().userConfiguration.layoutScaleValue + 0.05 * multiplier;
-    scale = Math.min(constants.MAX_LAYOUT_SCALE, scale);
-    scale = Math.max(constants.MIN_LAYOUT_SCALE, scale);
+    const scale = Store.getState().userConfiguration.layoutScaleValue + 0.05 * multiplier;
     Store.dispatch(updateUserSettingAction("layoutScaleValue", scale));
   }
 

frontend/javascripts/oxalis/controller/url_manager.js

@@ -107,7 +107,7 @@ class UrlManager {
     const position = V3.floor(getPosition(Store.getState().flycam));
     const { viewMode } = Store.getState().temporaryConfiguration;
     const viewModeIndex = ViewModeValues.indexOf(viewMode);
-    const zoomStep = Store.getState().flycam.zoomStep.toFixed(2);
+    const zoomStep = Store.getState().flycam.zoomStep.toFixed(3);
     const rotation = constants.MODES_ARBITRARY.includes(viewMode)
       ? getRotation(Store.getState().flycam).map(e => e.toFixed(2))
       : [];

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

@@ -390,21 +390,12 @@ class ArbitraryController extends React.PureComponent<Props> {
   }
 
   changeMoveValue(delta: number): void {
-    let moveValue = Store.getState().userConfiguration.moveValue3d + delta;
-    moveValue = Math.min(constants.MAX_MOVE_VALUE, moveValue);
-    moveValue = Math.max(constants.MIN_MOVE_VALUE, moveValue);
-
+    const moveValue = Store.getState().userConfiguration.moveValue3d + delta;
     Store.dispatch(updateUserSettingAction("moveValue3d", moveValue));
-
-    const moveValueMessage = messages["tracing.changed_move_value"] + moveValue;
-    Toast.success(moveValueMessage, { key: "CHANGED_MOVE_VALUE" });
   }
 
   setParticleSize(delta: number): void {
-    let particleSize = Store.getState().userConfiguration.particleSize + delta;
-    particleSize = Math.min(constants.MAX_PARTICLE_SIZE, particleSize);
-    particleSize = Math.max(constants.MIN_PARTICLE_SIZE, particleSize);
-
+    const particleSize = Store.getState().userConfiguration.particleSize + delta;
     Store.dispatch(updateUserSettingAction("particleSize", particleSize));
   }
 

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

@@ -48,7 +48,6 @@ import constants, {
   VolumeToolEnum,
 } from "oxalis/constants";
 import getSceneController from "oxalis/controller/scene_controller_provider";
-import messages from "messages";
 import * as skeletonController from "oxalis/controller/combinations/skeletontracing_plane_controller";
 import * as volumeController from "oxalis/controller/combinations/volumetracing_plane_controller";
 
@@ -445,23 +444,17 @@ class PlaneController extends React.PureComponent<Props> {
   }
 
   changeMoveValue(delta: number): void {
-    let moveValue = Store.getState().userConfiguration.moveValue + delta;
-    moveValue = Math.min(constants.MAX_MOVE_VALUE, moveValue);
-    moveValue = Math.max(constants.MIN_MOVE_VALUE, moveValue);
-
+    const moveValue = Store.getState().userConfiguration.moveValue + delta;
     Store.dispatch(updateUserSettingAction("moveValue", moveValue));
-
-    const moveValueMessage = messages["tracing.changed_move_value"] + moveValue;
-    Toast.success(moveValueMessage, { key: "CHANGED_MOVE_VALUE" });
   }
 
   changeBrushSizeIfBrushIsActive(changeValue: number) {
     const isBrushActive = Utils.maybe(getVolumeTool)(this.props.tracing.volume)
       .map(tool => tool === VolumeToolEnum.BRUSH)
       .getOrElse(false);
     if (isBrushActive) {
-      const currentSize = Store.getState().userConfiguration.brushSize;
-      Store.dispatch(updateUserSettingAction("brushSize", currentSize + changeValue));
+      const brushSize = Store.getState().userConfiguration.brushSize + changeValue;
+      Store.dispatch(updateUserSettingAction("brushSize", brushSize));
     }
   }
 
@@ -480,9 +473,9 @@ class PlaneController extends React.PureComponent<Props> {
           .map(tool => tool === VolumeToolEnum.BRUSH)
           .getOrElse(false);
         if (isBrushActive) {
-          const currentSize = Store.getState().userConfiguration.brushSize;
           // Different browsers send different deltas, this way the behavior is comparable
-          Store.dispatch(updateUserSettingAction("brushSize", currentSize + (delta > 0 ? 5 : -5)));
+          const brushSize = Store.getState().userConfiguration.brushSize + (delta > 0 ? 5 : -5);
+          Store.dispatch(updateUserSettingAction("brushSize", brushSize));
         } else if (this.props.tracing.skeleton) {
           // Different browsers send different deltas, this way the behavior is comparable
           api.tracing.setNodeRadius(delta > 0 ? 5 : -5);

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

@@ -10,9 +10,9 @@ import { getBaseVoxelFactors } from "oxalis/model/scaleinfo";
 import { getMaxZoomValue } from "oxalis/model/accessors/flycam_accessor";
 import Dimensions from "oxalis/model/dimensions";
 import * as Utils from "libs/utils";
+import { userSettings } from "libs/user_settings.schema";
 
 export const ZOOM_STEP_INTERVAL = 1.1;
-const ZOOM_STEP_MIN = 0.005;
 
 function cloneMatrix(m: Matrix4x4): Matrix4x4 {
   return [
@@ -116,7 +116,7 @@ function moveReducer(state: OxalisState, vector: Vector3): OxalisState {
 export function zoomReducer(state: OxalisState, zoomStep: number): OxalisState {
   return update(state, {
     flycam: {
-      zoomStep: { $set: Utils.clamp(ZOOM_STEP_MIN, zoomStep, getMaxZoomValue(state)) },
+      zoomStep: { $set: Utils.clamp(userSettings.zoom.minimum, zoomStep, getMaxZoomValue(state)) },
     },
   });
 }

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

@@ -10,6 +10,8 @@ import {
   type StateShape1,
   type StateShape2,
 } from "oxalis/model/helpers/deep_update";
+import { clamp } from "libs/utils";
+import { userSettings } from "libs/user_settings.schema";
 
 //
 // Update helpers
@@ -38,8 +40,17 @@ const updateActiveMapping = (
 function SettingsReducer(state: OxalisState, action: Action): OxalisState {
   switch (action.type) {
     case "UPDATE_USER_SETTING": {
-      const { propertyName, value } = action;
+      const { propertyName } = action;
+      let { value } = action;
+
+      const settingSpec = userSettings[propertyName];
+      if (settingSpec != null && settingSpec.type === "number") {
+        const min = settingSpec.minimum != null ? settingSpec.minimum : -Infinity;
+        const max = settingSpec.maximum != null ? settingSpec.maximum : Infinity;
+        value = clamp(min, value, max);
+      }
 
+      // $FlowFixMe Flow doesn't check that only numbers will be clamped
       return updateUserConfig(state, { [propertyName]: value });
     }
 

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

@@ -39,6 +39,7 @@ import ColorGenerator from "libs/color_generator";
 import Constants from "oxalis/constants";
 import Toast from "libs/toast";
 import * as Utils from "libs/utils";
+import { userSettings } from "libs/user_settings.schema";
 
 function SkeletonTracingReducer(state: OxalisState, action: Action): OxalisState {
   const { restrictions } = state.tracing;
@@ -220,9 +221,9 @@ function SkeletonTracingReducer(state: OxalisState, action: Action): OxalisState
         case "SET_NODE_RADIUS": {
           const { radius, nodeId, treeId } = action;
           const clampedRadius = Utils.clamp(
-            Constants.MIN_NODE_RADIUS,
+            userSettings.nodeRadius.minimum,
             radius,
-            Constants.MAX_NODE_RADIUS,
+            userSettings.nodeRadius.maximum,
           );
           return getNodeAndTree(skeletonTracing, nodeId, treeId)
             .map(([tree, node]) => {

frontend/javascripts/oxalis/model/sagas/settings_saga.js

@@ -11,6 +11,8 @@ import {
 import { updateUserConfiguration, updateDatasetConfiguration } from "admin/admin_rest_api";
 import { trackAction } from "oxalis/model/helpers/analytics";
 import { type UpdateUserSettingAction } from "oxalis/model/actions/settings_actions";
+import messages from "messages";
+import Toast from "libs/toast";
 
 function* pushUserSettingsAsync(): Saga<void> {
   const activeUser = yield* select(state => state.activeUser);
@@ -35,12 +37,22 @@ function* trackUserSettingsAsync(action: UpdateUserSettingAction): Saga<void> {
   }
 }
 
+function* showUserSettingToast(action: UpdateUserSettingAction): Saga<void> {
+  const { propertyName } = action;
+  if (propertyName === "moveValue" || propertyName === "moveValue3d") {
+    const moveValue = yield* select(state => state.userConfiguration[propertyName]);
+    const moveValueMessage = messages["tracing.changed_move_value"] + moveValue;
+    Toast.success(moveValueMessage, { key: "CHANGED_MOVE_VALUE" });
+  }
+}
+
 export default function* watchPushSettingsAsync(): Saga<void> {
   yield* take("INITIALIZE_SETTINGS");
   yield _all([
     _throttle(500, "UPDATE_USER_SETTING", pushUserSettingsAsync),
     _throttle(500, "UPDATE_DATASET_SETTING", pushDatasetSettingsAsync),
     _throttle(500, "UPDATE_LAYER_SETTING", pushDatasetSettingsAsync),
     _takeEvery("UPDATE_USER_SETTING", trackUserSettingsAsync),
+    _takeEvery("UPDATE_USER_SETTING", showUserSettingToast),
   ]);
 }

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

@@ -32,6 +32,7 @@ import TreesTabView, { importNmls } from "oxalis/view/right-menu/trees_tab_view"
 import VersionView from "oxalis/view/version_view";
 import messages from "messages";
 import window, { document, location } from "libs/window";
+import ErrorHandling from "libs/error_handling";
 
 import { GoldenLayoutAdapter } from "./golden_layout_adapter";
 import { determineLayout, headerHeight } from "./default_layout_configs";
@@ -60,6 +61,7 @@ type Props = {| ...OwnProps, ...StateProps, ...DispatchProps |};
 type State = {
   isSettingsCollapsed: boolean,
   activeLayout: string,
+  hasError: boolean,
 };
 
 const canvasAndLayoutContainerID = "canvasAndLayoutContainer";
@@ -76,6 +78,12 @@ class TracingLayoutView extends React.PureComponent<Props, State> {
   currentLayoutConfig: Object;
   currentLayoutName: string;
 
+  static getDerivedStateFromError() {
+    // DO NOT set hasError back to false EVER as this will trigger a remount of the Controller
+    // with unforeseeable consequences
+    return { hasError: true };
+  }
+
   constructor(props: Props) {
     super(props);
     const layoutType = determineLayout(this.props.initialCommandType.type, this.props.viewMode);
@@ -93,10 +101,12 @@ class TracingLayoutView extends React.PureComponent<Props, State> {
     this.state = {
       isSettingsCollapsed: true,
       activeLayout: lastActiveLayout,
+      hasError: false,
     };
   }
 
-  componentDidCatch() {
+  componentDidCatch(error: Error) {
+    ErrorHandling.notify(error);
     Toast.error(messages["react.rendering_error"]);
   }
 
@@ -143,6 +153,14 @@ class TracingLayoutView extends React.PureComponent<Props, State> {
     this.props.storedLayouts[layoutKey] ? Object.keys(this.props.storedLayouts[layoutKey]) : [];
 
   render() {
+    if (this.state.hasError) {
+      return (
+        <div style={{ marginTop: 50, textAlign: "center" }}>
+          {messages["react.rendering_error"]}
+        </div>
+      );
+    }
+
     const layoutType = determineLayout(this.props.initialCommandType.type, this.props.viewMode);
     const currentLayoutNames = this.getLayoutNamesFromCurrentView(layoutType);
     const { displayScalebars, isDatasetOnScratchVolume, isUpdateTracingAllowed } = this.props;