Fix offset when creating node with mouse (#7639)

* proof of concept fix of mouse-offset by putting viewport borders into separate div * fix NaN in ishit check * rename css class to snake case; remove unused box-sizing rule * fix scalebar size (due to box-sizing content-box, the padding should not be incorporated) * misc simplification of early-out * remove all usages and the definition of OUTER_CSS_BORDER since its value is 0 now * update changelog
scalableminds · Feb 21, 2024 · c32bc1b · c32bc1b
1 parent 8f32b07
commit c32bc1b
Show file tree

Hide file tree

Showing 11 changed files with 53 additions and 70 deletions.
diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md
@@ -36,6 +36,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released
 - Fixed deprecation warnings caused by Antd \<Collapse\> components. [#7610](https://github.com/scalableminds/webknossos/pull/7610)
 - Fixed small styling error with a welcome notification for new users on webknossos.org. [#7623](https://github.com/scalableminds/webknossos/pull/7623)
 - Fixed that filtering by tags could produce false positives. [#7640](https://github.com/scalableminds/webknossos/pull/7640)
+- Fixed a slight offset when creating a node with the mouse. [#7639](https://github.com/scalableminds/webknossos/pull/7639)
 
 ### Removed
 

diff --git a/frontend/javascripts/libs/input.ts b/frontend/javascripts/libs/input.ts
@@ -676,6 +676,8 @@ export class InputMouse {
       width: 0,
       height: 0,
     };
+    // Don't use {...boundingRect, }, because boundingRect is a DOMRect
+    // which isn't compatible with the spreading, apparently.
     return _.extend({}, boundingRect, {
       left: boundingRect.left + window.scrollX,
       top: boundingRect.top + window.scrollY,

diff --git a/frontend/javascripts/oxalis/constants.ts b/frontend/javascripts/oxalis/constants.ts
@@ -264,10 +264,6 @@ export enum TreeTypeEnum {
 export type TreeType = keyof typeof TreeTypeEnum;
 export const NODE_ID_REF_REGEX = /#([0-9]+)/g;
 export const POSITION_REF_REGEX = /#\(([0-9]+,[0-9]+,[0-9]+)\)/g;
-// The plane in orthogonal mode is a little smaller than the viewport
-// There is an outer yellow CSS border and an inner (red/green/blue) border
-// that is a result of the plane being smaller than the renderer viewport
-export const OUTER_CSS_BORDER = 2;
 const VIEWPORT_WIDTH = 376;
 export const ensureSmallerEdge = false;
 export const Unicode = {

diff --git a/frontend/javascripts/oxalis/controller/combinations/skeleton_handlers.ts b/frontend/javascripts/oxalis/controller/combinations/skeleton_handlers.ts
@@ -6,7 +6,7 @@ import type {
   Vector3,
   ShowContextMenuFunction,
 } from "oxalis/constants";
-import { OUTER_CSS_BORDER, OrthoViews } from "oxalis/constants";
+import { OrthoViews } from "oxalis/constants";
 import { V3 } from "libs/mjs";
 import _ from "lodash";
 import { enforce, values } from "libs/utils";
@@ -345,9 +345,7 @@ export function maybeGetNodeIdFromPosition(
   height = Math.round(height);
   const buffer = renderToTexture(plane, pickingScene, camera, true);
   // Beware of the fact that new browsers yield float numbers for the mouse position
-  // Subtract the CSS border as the renderer viewport is smaller than the inputcatcher
-  const borderWidth = OUTER_CSS_BORDER;
-  const [x, y] = [Math.round(position.x) - borderWidth, Math.round(position.y) - borderWidth];
+  const [x, y] = [Math.round(position.x), Math.round(position.y)];
   // compute the index of the pixel under the cursor,
   // while inverting along the y-axis, because WebGL has its origin bottom-left :/
   const index = (x + (height - y) * width) * 4;

diff --git a/frontend/javascripts/oxalis/geometries/plane.ts b/frontend/javascripts/oxalis/geometries/plane.ts
@@ -140,12 +140,11 @@ class Plane {
   };
 
   setScale(xFactor: number, yFactor: number): void {
-    if (this.lastScaleFactors[0] !== xFactor || this.lastScaleFactors[1] !== yFactor) {
-      this.lastScaleFactors[0] = xFactor;
-      this.lastScaleFactors[1] = yFactor;
-    } else {
+    if (this.lastScaleFactors[0] === xFactor && this.lastScaleFactors[1] === yFactor) {
       return;
     }
+    this.lastScaleFactors[0] = xFactor;
+    this.lastScaleFactors[1] = yFactor;
 
     const scaleVec = new THREE.Vector3().multiplyVectors(
       new THREE.Vector3(xFactor, yFactor, 1),

diff --git a/frontend/javascripts/oxalis/model/accessors/view_mode_accessor.ts b/frontend/javascripts/oxalis/model/accessors/view_mode_accessor.ts
@@ -13,7 +13,6 @@ import type {
 } from "oxalis/constants";
 import constants, {
   ArbitraryViewport,
-  OUTER_CSS_BORDER,
   OrthoViews,
   OrthoViewValuesWithoutTDView,
 } from "oxalis/constants";
@@ -86,11 +85,8 @@ export function getInputCatcherAspectRatio(state: OxalisState, viewport: Viewpor
 // Returns the ratio between VIEWPORT_WIDTH and the actual extent of the viewport for width and height
 export function getViewportScale(state: OxalisState, viewport: Viewport): [number, number] {
   const { width, height } = getInputCatcherRect(state, viewport);
-  // For the orthogonal views the CSS border width was subtracted before, so we'll need to
-  // add it back again to get an accurate scale
-  const borderWidth = viewport === ArbitraryViewport ? 0 : OUTER_CSS_BORDER;
-  const xScale = (width + 2 * borderWidth) / constants.VIEWPORT_WIDTH;
-  const yScale = (height + 2 * borderWidth) / constants.VIEWPORT_WIDTH;
+  const xScale = width / constants.VIEWPORT_WIDTH;
+  const yScale = height / constants.VIEWPORT_WIDTH;
   return [xScale, yScale];
 }
 

diff --git a/frontend/javascripts/oxalis/view/input_catcher.tsx b/frontend/javascripts/oxalis/view/input_catcher.tsx
@@ -146,30 +146,32 @@ function InputCatcher({
   );
 
   return (
-    <div
-      className="flexlayout-dont-overflow"
-      onContextMenu={ignoreContextMenu}
-      style={{ cursor: busyBlockingInfo.isBusy ? "wait" : cursorForTool[adaptedTool] }}
-    >
+    <div className={`inputcatcher-border ${viewportID}`}>
       <div
-        id={`inputcatcher_${viewportID}`}
-        ref={(domElement) => {
-          domElementRef.current = domElement;
-        }}
-        data-value={viewportID}
-        className={`inputcatcher ${viewportID}`}
-        style={{
-          position: "relative",
-          // Disable inputs while wk is busy. However, keep the custom cursor and the ignoreContextMenu handler
-          // which is why those are defined at the outer element.
-          pointerEvents: busyBlockingInfo.isBusy ? "none" : "auto",
-        }}
+        className="flexlayout-dont-overflow"
+        onContextMenu={ignoreContextMenu}
+        style={{ cursor: busyBlockingInfo.isBusy ? "wait" : cursorForTool[adaptedTool] }}
       >
-        <ViewportStatusIndicator />
-        {displayScalebars && viewportID !== "arbitraryViewport" ? (
-          <Scalebar viewportID={viewportID} />
-        ) : null}
-        {children}
+        <div
+          id={`inputcatcher_${viewportID}`}
+          ref={(domElement) => {
+            domElementRef.current = domElement;
+          }}
+          data-value={viewportID}
+          className={`inputcatcher ${viewportID}`}
+          style={{
+            position: "relative",
+            // Disable inputs while wk is busy. However, keep the custom cursor and the ignoreContextMenu handler
+            // which is why those are defined at the outer element.
+            pointerEvents: busyBlockingInfo.isBusy ? "none" : "auto",
+          }}
+        >
+          <ViewportStatusIndicator />
+          {displayScalebars && viewportID !== "arbitraryViewport" ? (
+            <Scalebar viewportID={viewportID} />
+          ) : null}
+          {children}
+        </div>
       </div>
     </div>
   );

diff --git a/frontend/javascripts/oxalis/view/layouting/layout_canvas_adapter.ts b/frontend/javascripts/oxalis/view/layouting/layout_canvas_adapter.ts
@@ -1,5 +1,4 @@
 import type { Rect } from "oxalis/constants";
-import { OUTER_CSS_BORDER } from "oxalis/constants";
 import { document } from "libs/window";
 
 export default function makeRectRelativeToCanvas(rect: Rect): Rect {
@@ -10,15 +9,13 @@ export default function makeRectRelativeToCanvas(rect: Rect): Rect {
   }
 
   const { left: containerX, top: containerY } = layoutContainerDOM.getBoundingClientRect();
-  const borderWidth = OUTER_CSS_BORDER;
-
   const minNull = (el: number) => Math.max(el, 0);
 
   // Since we want to paint inside the InputCatcher we have to subtract the border
   return {
-    left: minNull(rect.left - containerX + borderWidth),
-    top: minNull(rect.top - containerY + borderWidth),
-    width: minNull(rect.width - 2 * borderWidth),
-    height: minNull(rect.height - 2 * borderWidth),
+    left: minNull(rect.left - containerX),
+    top: minNull(rect.top - containerY),
+    width: minNull(rect.width),
+    height: minNull(rect.height),
   };
 }
diff --git a/frontend/javascripts/oxalis/view/rendering_utils.ts b/frontend/javascripts/oxalis/view/rendering_utils.ts
@@ -1,7 +1,7 @@
 import * as THREE from "three";
 import { saveAs } from "file-saver";
 import Store from "oxalis/store";
-import { OUTER_CSS_BORDER, OrthoView } from "oxalis/constants";
+import { OrthoView } from "oxalis/constants";
 import constants, {
   ArbitraryViewport,
   OrthoViewColors,
@@ -121,10 +121,7 @@ export async function downloadScreenshot() {
         ? (ctx: CanvasRenderingContext2D) => {
             const scalebarDistanceToRightBorder = constants.SCALEBAR_OFFSET;
             const scalebarDistanceToTopBorder =
-              ctx.canvas.height +
-              OUTER_CSS_BORDER -
-              constants.SCALEBAR_OFFSET -
-              constants.SCALEBAR_HEIGHT;
+              ctx.canvas.height + constants.SCALEBAR_OFFSET - constants.SCALEBAR_HEIGHT;
             const logoHeight = constants.SCALEBAR_HEIGHT;
             const logoWidth = (logoHeight / logo.height) * logo.width;
             ctx.drawImage(

diff --git a/frontend/javascripts/oxalis/view/scalebar.tsx b/frontend/javascripts/oxalis/view/scalebar.tsx
@@ -8,7 +8,7 @@ import { formatNumberToLength } from "libs/format_utils";
 import { getViewportExtents, getTDViewZoom } from "oxalis/model/accessors/view_mode_accessor";
 import { getZoomValue } from "oxalis/model/accessors/flycam_accessor";
 import type { OrthoView } from "oxalis/constants";
-import constants, { Unicode, OUTER_CSS_BORDER, OrthoViews } from "oxalis/constants";
+import constants, { Unicode, OrthoViews } from "oxalis/constants";
 const { ThinSpace, MultiplicationSymbol } = Unicode;
 type OwnProps = {
   viewportID: OrthoView;
@@ -76,11 +76,7 @@ function Scalebar({ zoomValue, dataset, viewportWidthInPixels, viewportHeightInP
           position: "absolute",
           bottom: constants.SCALEBAR_OFFSET,
           right: constants.SCALEBAR_OFFSET,
-          width: collapseScalebar
-            ? 16
-            : `calc(${scaleBarWidthFactor * 100}% - ${Math.round(
-                ((2 * OUTER_CSS_BORDER) / constants.VIEWPORT_WIDTH) * 100,
-              )}% + ${2 * padding}px)`,
+          width: collapseScalebar ? 16 : `${scaleBarWidthFactor * 100}%`,
           height: constants.SCALEBAR_HEIGHT - padding * 2,
           background: "rgba(0, 0, 0, .3)",
           color: "white",

diff --git a/frontend/stylesheets/trace_view/_tracing_view.less b/frontend/stylesheets/trace_view/_tracing_view.less
@@ -18,18 +18,15 @@
   position: absolute;
   width: 100%;
 }
-.inputcatcher {
+.inputcatcher-border {
   border-style: solid;
   border-width: 2px;
-  z-index: 20;
-  box-sizing: border-box;
-  -moz-box-sizing: border-box;
-  float: left;
-  margin: 0;
   opacity: 0.85;
   width: 100%;
   height: 100%;
-
+  &:hover {
+    opacity: 1;
+  }
   &.PLANE_XY {
     border-color: var(--color-xy-viewport);
   }
@@ -42,10 +39,13 @@
   &.TDView {
     border-color: white;
   }
-
-  &:hover {
-    opacity: 1;
-  }
+}
+.inputcatcher {
+  z-index: 20;
+  float: left;
+  margin: 0;
+  width: 100%;
+  height: 100%;
 }
 
 #inputcatcher_TDView {
@@ -549,7 +549,6 @@ img.keyboard-mouse-icon:first-child {
   opacity: 0.65;
 }
 
-
 .segment-context-icon {
   margin-inline-end: var(--ant-margin-xs);
   height: 12px;