Add measurement tools #7258

MichaelBuessemeyer · 2023-08-10T14:54:13Z

URL of deployed dev instance (used for testing):

https://addmeasurementtool.webknossos.xyz/

Steps to test:

Open an annotation of any dataset
Select the new measurement tool in the toolbar (the new tool should be the right most)
Try measuring a path. Setting path point can be done via left click. Setting the last point via double-/or right-click and terminating the measurement by hitting escape or another right click.
Try copying the measured length from the tooltip via the copy icon.
Start measuring and terminating it while still in the process using escape. This should not lead to any errors or unexpected behaviour. (if so, I have a regression bug 🙈)
Select the area measurement tool. Draw an area via right click. The area should always be closed by another highlighting line to highlight that the measured area is a closed area connecting the current end and start point.
Upon releasing the left mouse drag, the highlighted line should be gone and the connecting line should be in the normal color.
Potential bug: see below: When hitting escape while measuring an area, the area measurement tool is not correctly reset.
Please report any unexpected behavior.
Test the tracing tool. It should still work as expected as this PR fiddled around with the helper geometry used by the tracing tool.

TODOs / Bugs:

~~[ ] Add fill area for area measurement tool~~ We decided against that as this is quite the hurdle and thus went for always having a highlighted connection line between the start and end of the drawn area to highlight that the user is measuring an area.
When finished marking the area the tooltip can be moved around by left clicking.

Known issue / to discuss:

When canceling the area measurement via pressing escape while the mouse is still pressed down and then continuing dragging the mouse, the tooltip reappears and the measurement continues. Should this be tackled or is this misuse of the tool and the user can be expected to redo the measurement? In this scenario, no exception is thrown but the measured area is not correctly canceled upon hitting escape. So keeping it as it is right now, is acceptable imo.

Issues:

fixes Add Measurement Tools #3693

(Please delete unneeded items, merge only when none are left open)

Updated changelog
Updated documentation if applicable

…d value

…-measurement-tool

MichaelBuessemeyer · 2023-08-31T15:28:28Z

Just for information purposes:

The algorithm used for the calculation of the area of the polygon of the area measurement tool works correctly, when the point are exactly on a line: The algorithm was taken from: https://www.mathopenref.com/coordpolygonarea2.html

function polygonArea(X, Y, numPoints) 
{ 
area = 0;   // Accumulates area 
j = numPoints-1; 

for (i=0; i<numPoints; i++)
{ area +=  (X[j]+X[i]) * (Y[j]-Y[i]); 
  j = i;  //j is previous vertex to i
}
  return area/2;
}
// The triangle reaches from 1,1 to 4,1 to 1,3 and back to 1,1. Thus the area is h*w/2 = 2*3/2 = 3
X = [1,2,3,4,2.5,1,1]
Y = [1,1,1,1,2,3,2]
polygonArea(X,Y,7)
=> -3
// The result is correct (and negative due to the points being in counter clockwise order). 
// The implementation of the algorithm in this pr is secure against this flaw by using the absolute value of the calculated area.

- add double click to linemeasurement tool, - WIP adding filled area to area measurement tool

…end of the area

…-measurement-tool

MichaelBuessemeyer · 2023-09-01T08:54:44Z

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

+    const guardFromResettedTool = (func: Function) => {
+      // TODO: Problem typing is lost
+      return (...args: any) => {
+        if (lineMeasurementGeometry.isResetted && isMeasuring) {
+          isMeasuring = false;
+          return;
+        }
+        func(...args);
+      };
+    };


All of the following three functions need to be guarded against the user having used escape while still in the process of measuring, to interrupt the measurement. When escape was pressed, the lineMeasurementGeometry carries that information in the field isResetted.

The guarding function takes care of guarding these three functions against the user using escape while still measuring. But the typing is lost (as far as I would guess) and the code ready quite quirky IMO. Maybe you have a better idea?

MichaelBuessemeyer · 2023-09-01T09:37:31Z

frontend/javascripts/oxalis/view/input_catcher.tsx

+  LINE_MEASUREMENT: "pointer",
+  AREA_MEASUREMENT: "pointer",


Can you think of a better suitable mouse pointer attribute for the new tools?

MichaelBuessemeyer · 2023-09-01T09:58:49Z

@philippotto This pr should now be ready for the first review. After the CI is done, I will also ask for further UX feedback in slack.

…-measurement-tool

- allow panning in line measurement tool - add custom mouse cursors - use area unit for area measurement tool

philippotto

Great stuff :) Will test now!

frontend/javascripts/libs/format_utils.ts

philippotto · 2023-09-14T14:35:10Z

frontend/javascripts/oxalis/view/distance_measurement_tooltip.tsx

+function mapStateToProps(state: OxalisState): Props {
+  return {
+    position: state.uiInformation.measurementTooltipPosition,
+    flycam: state.flycam,
+    activeTool: state.uiInformation.activeTool,
+    datasetScale: state.dataset.dataSource.scale,
+  };
+}
+
+const connector = connect(mapStateToProps);


since this is a function components, I'd rather use useSelector to access the store.

philippotto · 2023-09-15T07:24:50Z

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

@@ -780,6 +784,171 @@ export class QuickSelectTool {
  static onToolDeselected() {}
 }

+export class LineMeasurementTool {
+  static DOUBLE_CLICK_TIME_THRESHOLD = 600;
+  static getPlaneMouseControls(_planeId: OrthoView): any {


getPlaneMouseControls of the area measurement tool doesn't accept that parameter. maybe make these interfaces consistent?

philippotto · 2023-09-15T07:28:35Z

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

+        return;
+      }
+      const currentTime = Date.now();
+      if (currentTime - lastLeftClickTime <= this.DOUBLE_CLICK_TIME_THRESHOLD) {


wouldn't it make sense to check that the two clicks were registered roughly at the same spot?

philippotto · 2023-09-15T07:30:35Z

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

+    lineMeasurementGeometry.reset();
+    lineMeasurementGeometry.hide();


There are several places where reset and hide is called. How about adding a resetAndHide helper method?

philippotto · 2023-09-15T07:32:43Z

frontend/javascripts/oxalis/geometries/helper_geometries.ts

@@ -61,6 +107,43 @@ export class ContourGeometry {
    mesh.geometry.attributes.position.needsUpdate = true;
    mesh.geometry.setDrawRange(0, this.vertexBuffer.getLength());
    mesh.geometry.computeBoundingSphere();
+    this.connectingLine.geometry.attributes.position.needsUpdate = true;
+    // this.connectingLine.geometry.setDrawRange(0, 2);


philippotto · 2023-09-15T07:34:20Z

frontend/javascripts/oxalis/geometries/helper_geometries.ts

+    this.finalizeMesh();
+  }
+
+  setTopPoint(pos: Vector3) {


I think I don't really understand what a "top point" is. maybe add a comment?

I added a comment and renamed the method to updateLatestPointPosition.

By top point I meant the latest added point to the array of points (like the latest element on a stack is called top element).

philippotto · 2023-09-15T07:36:11Z

frontend/javascripts/oxalis/geometries/helper_geometries.ts

@@ -231,3 +314,113 @@ export class QuickSelectGeometry {
    rectangle.material.alphaMap = null;
  }
 }
+
+export class LineMeasurementGeometry {
+  color: THREE.Color;


maybe add a class comment explaining that this is used for displaying "line segments" (not sure whether this is the right term?).

I called it connected line segments 🤷

philippotto · 2023-09-15T07:37:19Z

frontend/javascripts/oxalis/geometries/helper_geometries.ts

+  }
+
+  setStartPoint(pos: Vector3, initialOrthoView: OrthoView) {
+    this.wasReset = false;


I'd suggest to either add a comment explaining that this method expects an empty vertexBuffer or add this.vertexBuffer.clear() to this method.

Good catch 👍, I went for the 2nd suggestion, as this is more bullet proof against potential bugs imo.

philippotto · 2023-09-15T08:18:55Z

Overall it feels pretty good 👍 My testing results:

line tool: fast clicking is sometimes registered as double click (see my other suggestion to compare the clicked positions)
area tool shows units without ² and the line tool shows them with ²
the measured area seems wrong (see my other code comment)
I'd change "Finish Measurement and reset" in the status bar to "Finish Measurement" since it's not clear what reset actually means

Please remind me again why the measurement geometries are hidden as soon as the user moves? Is it because it would be hard to stick the tooltip to the geometry? If this is the case, one could also think about moving the tooltip content simply into the toolbar 🤔 But I think we'd need actual user feedback for that. Right now, I imagine it to be confusing potentially (could be percepted as bug, too).
@normanrz do you have an opinion about the second paragraph?

…-measurement-tool

- Add double click to area measurement to cancel the tool

MichaelBuessemeyer

Thanks for your feedback @philippotto: I think I addressed everything now.

Please remind me again why the measurement geometries are hidden as soon as the user moves?

you mentioned hiding the tool as soon as the camera moves here and here

Is it because it would be hard to stick the tooltip to the geometry?

Jup, that is the case 👍

Let's wait for @normanrz answer 😺

frontend/javascripts/libs/format_utils.ts

MichaelBuessemeyer · 2023-09-21T12:34:42Z

frontend/javascripts/oxalis/geometries/helper_geometries.ts

@@ -231,3 +314,113 @@ export class QuickSelectGeometry {
    rectangle.material.alphaMap = null;
  }
 }
+
+export class LineMeasurementGeometry {
+  color: THREE.Color;


I called it connected line segments 🤷

MichaelBuessemeyer · 2023-09-21T12:37:21Z

frontend/javascripts/oxalis/geometries/helper_geometries.ts

+  }
+
+  setStartPoint(pos: Vector3, initialOrthoView: OrthoView) {
+    this.wasReset = false;


Good catch 👍, I went for the 2nd suggestion, as this is more bullet proof against potential bugs imo.

MichaelBuessemeyer · 2023-09-21T12:42:07Z

frontend/javascripts/oxalis/geometries/helper_geometries.ts

+    this.finalizeMesh();
+  }
+
+  setTopPoint(pos: Vector3) {


I added a comment and renamed the method to updateLatestPointPosition.

By top point I meant the latest added point to the array of points (like the latest element on a stack is called top element).

…-measurement-tool

normanrz · 2023-09-21T13:11:52Z

Please remind me again why the measurement geometries are hidden as soon as the user moves? Is it because it would be hard to stick the tooltip to the geometry? If this is the case, one could also think about moving the tooltip content simply into the toolbar 🤔 But I think we'd need actual user feedback for that. Right now, I imagine it to be confusing potentially (could be percepted as bug, too).
@normanrz do you have an opinion about the second paragraph?

I think keeping the lines on move would be nice. But how would a user remove them again?

Also, Alt+Mouse Move doesn't work for me in this tool mode. That feels like a bug.

MichaelBuessemeyer · 2023-09-21T17:35:01Z

I think keeping the lines on move would be nice.

I misunderstood your statement a little 😅 Now the lines are kept (unless the user moves in the z direction of the viewport) and the tooltip follows the line / area mesurement

MichaelBuessemeyer · 2023-09-21T17:36:17Z

But how would a user remove them again?

Via right or double left click.

Also, Alt+Mouse Move doesn't work for me in this tool mode. That feels like a bug.

For me this works. Maybe the viewports weren't the active part of the wk webpage when you tested? 🤔

philippotto · 2023-09-22T08:30:15Z

The sticky tooltip is quite cool 👍 However, I ran into some bugs:

like @normanrz, alt+mousemove doesn't work for me while using the line tool
alt+mousemove for the area tool works sort of. it changes to the proper tool and does the moving, but the tool still registers new points around the mouse.
I got into a weird state where the measurement tools got broken. I can reproduce this by starting measuring in XY and then adding points in YZ. see video.

minor problems:

when moving the mouse fast, I can move it into the tooltip which is a bit weird since I cannot pinpoint the next click in the area of the tooltip. not sure, how to solve this. as a user I can always leave the tooltip and then hidden space is visible again. so, it's not a big issue, but I ran into this quite quickly, which is why I'm writing this.

I recorded a video for these issues:

the video starts with the broken state where I cannot measure anything. I recover this be reloading the page
then I show that alt+mousemove doesnt work for the line tool
then I show that the mouse can get caught up in the tooltip
then I show alt+mousemove for the area tool
in the end, I try to reproduce the weird state again. it was a bit difficult, but at 1:40 I managed to break it. afterwards, I cannot fix it again except for reloading the page. I would expect that switching the tools, resets all the relevant internal state.

measurement-bugs.mp4

…-measurement-tool

MichaelBuessemeyer

like @normanrz, alt+mousemove doesn't work for me while using the line tool

Ahh, ok now I understand what you mean: It is impossible to move around while measuring with a line 👍. Previously I only tested moving with alt when I was not actively measuring a line.
The alt moving should now be fixed for both measurement tools during the measurement and also while not measuring.

alt+mousemove for the area tool works sort of. it changes to the proper tool and does the moving, but the tool still registers new points around the mouse.

This should be fixed now. The tool should register new points while alt is pressed.

I got into a weird state where the measurement tools got broken. I can reproduce this by starting measuring in XY and then adding points in YZ. see video.

I think this was due to an instant reset of the measurement caused by the distance_measurement_tooltip. See my comment :)

when moving the mouse fast, I can move it into the tooltip which is a bit weird since I cannot pinpoint the next click in the area of the tooltip. not sure, how to solve this. as a user I can always leave the tooltip and then hidden space is visible again. so, it's not a big issue, but I ran into this quite quickly, which is why I'm writing this.

While measuring, the tooltip now ignores all pointer event, which mitigates this problem as the inputcatcher get the pointer event, even when the mouse hits the tooltip. Thanks to @fm3 for the solution.
This is done by saving whether the measurement tool is measuring at the moment in the store.

I added a tooltip to the distance option for the context menu as the entry could be misunderstood. See for reference https://scm.slack.com/archives/C5AKLAV0B/p1695894445554669

MichaelBuessemeyer · 2023-09-28T15:03:15Z

frontend/javascripts/oxalis/view/distance_measurement_tooltip.tsx

+  useEffect(() => {
+    if (
+      position != null &&
+      Math.floor(currentPosition[thirdDim]) !== Math.floor(position[thirdDim])


Here is the "trick" hopefully fixing the bug you described. In case the flycam is not at an exact numeric position, the fraction of it causes the !== to be false. This is due to the value stored in position being already floored by the measurement tool (which sets the value state.uiInformation.measurementToolInfo.lastMeasuredPosition)

Thus flooring the value should fix that the tooltip auto resets the measurement all the time and only triggers the reset in case the third coordinate actually changes by a full unit.

MichaelBuessemeyer · 2023-09-28T15:04:39Z

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

+        Store.dispatch(setIsMeasuringAction(false));
+      }
+      const isAltPressed = evt.altKey;
+      if (isAltPressed || plane !== this.initialPlane || !this.isMeasuring) {


Adding the isAltPressed enabled moving with alt while measuring

philippotto · 2023-10-11T12:58:59Z

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

+    const currentTime = Date.now();
+    if (
+      currentTime - lastLeftClickTime <= DOUBLE_CLICK_TIME_THRESHOLD &&
+      Math.abs(pos.x - lastClickPosition.x) + Math.abs(pos.y - lastClickPosition.y) < 6


extract the 6 into a constant?

philippotto · 2023-10-11T12:59:47Z

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

@@ -784,12 +784,32 @@ export class QuickSelectTool {
  static onToolDeselected() {}
 }

+function getDoubleClickGuard() {


cool abstraction 👍

philippotto

awesome! works very well :)

there are several PR comments where the code is marked as outdated. I assume these comments can be resolved? other than that, only see my one comment about a magic number.

MichaelBuessemeyer · 2023-10-11T15:10:44Z

I assume these comments can be resolved?

I skimmed through them and nothing of these comments seemed to be important 👍

…-measurement-tool

WIP: add measurement tool

7e9cca9

MichaelBuessemeyer self-assigned this Aug 10, 2023

MichaelBuessemeyer added frontend new feature labels Aug 10, 2023

MichaelBuessemeyer added 7 commits August 10, 2023 18:53

Make line mesh visible

7b66507

add ui to show measured length

feffad5

fix linting and tests

a833b93

only show tooltip, add option to switch units and to copy the measure…

aa0b825

…d value

Merge branch 'master' of github.com:scalableminds/webknossos into add…

0e40634

…-measurement-tool

Merge branch 'master' of github.com:scalableminds/webknossos into add…

59dc2cd

…-measurement-tool

add area measurement tool

d2187c2

MichaelBuessemeyer changed the title ~~WIP: Add measurement tool~~ WIP: Add measurement tools Aug 24, 2023

MichaelBuessemeyer added 3 commits August 24, 2023 20:55

fix area calculation for all viewports

ba61b1a

Merge branch 'master' of github.com:scalableminds/webknossos into add…

a96589b

…-measurement-tool

fix the tooltip position after drawing a measured area

bb1e52c

MichaelBuessemeyer added 6 commits August 31, 2023 20:26

- bug fixes,

ac930f2

- add double click to linemeasurement tool, - WIP adding filled area to area measurement tool

remove filling area and add highlighted connection between start and …

725b966

…end of the area

clean up for review & add comments

1f64636

adjust unit string for area measurement

74f4c5f

add changelog entry

fc5a064

Merge branch 'master' of github.com:scalableminds/webknossos into add…

9fdb2f7

…-measurement-tool

MichaelBuessemeyer marked this pull request as ready for review September 1, 2023 09:55

MichaelBuessemeyer commented Sep 1, 2023

View reviewed changes

MichaelBuessemeyer requested a review from philippotto September 1, 2023 09:56

MichaelBuessemeyer changed the title ~~WIP: Add measurement tools~~ Add measurement tools Sep 1, 2023

MichaelBuessemeyer added 3 commits September 7, 2023 14:59

Merge branch 'master' of github.com:scalableminds/webknossos into add…

7fc447c

…-measurement-tool

Merge branch 'master' of github.com:scalableminds/webknossos into add…

5fbc107

…-measurement-tool

apply feedback

f824392

- allow panning in line measurement tool - add custom mouse cursors - use area unit for area measurement tool

philippotto reviewed Sep 15, 2023

View reviewed changes

MichaelBuessemeyer added 2 commits September 21, 2023 13:57

Merge branch 'master' of github.com:scalableminds/webknossos into add…

d2ccb2c

…-measurement-tool

properly check for double clicks in measurement tools

6348abc

- Add double click to area measurement to cancel the tool

MichaelBuessemeyer commented Sep 21, 2023

View reviewed changes

Merge branch 'master' of github.com:scalableminds/webknossos into add…

0ea1ee7

…-measurement-tool

MichaelBuessemeyer added 2 commits September 21, 2023 15:48

apply code review feedback

12a348a

make tooltip follow measurement when camera is moved

3b1791d

MichaelBuessemeyer requested a review from philippotto September 21, 2023 17:36

MichaelBuessemeyer added 6 commits September 28, 2023 11:21

Merge branch 'master' of github.com:scalableminds/webknossos into add…

49db897

…-measurement-tool

fix alt movement in measurement tools

3d4a5fa

Add tooltip to distance entry in context menu to explain it better

2979b81

reset measurement tools on deselect

50e92e5

don't let measurement tooltip get pointer events while measuring

61d0ab1

fix bug where the measurement was directly reset after starting

1b66fa3

MichaelBuessemeyer commented Sep 28, 2023

View reviewed changes

Merge branch 'master' into add-measurement-tool

a4c9bf6

philippotto reviewed Oct 11, 2023

View reviewed changes

philippotto approved these changes Oct 11, 2023

View reviewed changes

MichaelBuessemeyer added 2 commits October 11, 2023 17:38

Merge branch 'master' of github.com:scalableminds/webknossos into add…

197a7dc

…-measurement-tool

extract magic value into constant

ead52f7

philippotto merged commit 8e809a2 into master Oct 12, 2023
1 check passed

philippotto deleted the add-measurement-tool branch October 12, 2023 08:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add measurement tools #7258

Add measurement tools #7258

MichaelBuessemeyer commented Aug 10, 2023 •

edited

Loading

MichaelBuessemeyer commented Aug 31, 2023

MichaelBuessemeyer Sep 1, 2023

MichaelBuessemeyer Sep 1, 2023

MichaelBuessemeyer commented Sep 1, 2023

philippotto left a comment

philippotto Sep 14, 2023

philippotto Sep 15, 2023

philippotto Sep 15, 2023

philippotto Sep 15, 2023

philippotto Sep 15, 2023

philippotto Sep 15, 2023

MichaelBuessemeyer Sep 21, 2023

philippotto Sep 15, 2023

MichaelBuessemeyer Sep 21, 2023

philippotto Sep 15, 2023

MichaelBuessemeyer Sep 21, 2023

philippotto commented Sep 15, 2023

MichaelBuessemeyer left a comment •

edited

Loading

MichaelBuessemeyer Sep 21, 2023

MichaelBuessemeyer Sep 21, 2023

MichaelBuessemeyer Sep 21, 2023

normanrz commented Sep 21, 2023

MichaelBuessemeyer commented Sep 21, 2023

MichaelBuessemeyer commented Sep 21, 2023

philippotto commented Sep 22, 2023

MichaelBuessemeyer left a comment •

edited

Loading

MichaelBuessemeyer Sep 28, 2023

MichaelBuessemeyer Sep 28, 2023

philippotto Oct 11, 2023

philippotto Oct 11, 2023

philippotto left a comment

MichaelBuessemeyer commented Oct 11, 2023

		lineMeasurementGeometry.reset();
		lineMeasurementGeometry.hide();

Add measurement tools #7258

Add measurement tools #7258

Conversation

MichaelBuessemeyer commented Aug 10, 2023 • edited Loading

URL of deployed dev instance (used for testing):

Steps to test:

TODOs / Bugs:

Known issue / to discuss:

Issues:

MichaelBuessemeyer commented Aug 31, 2023

Just for information purposes:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MichaelBuessemeyer commented Sep 1, 2023

philippotto left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

philippotto commented Sep 15, 2023

MichaelBuessemeyer left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

normanrz commented Sep 21, 2023

MichaelBuessemeyer commented Sep 21, 2023

MichaelBuessemeyer commented Sep 21, 2023

philippotto commented Sep 22, 2023

MichaelBuessemeyer left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

philippotto left a comment

Choose a reason for hiding this comment

MichaelBuessemeyer commented Oct 11, 2023

MichaelBuessemeyer commented Aug 10, 2023 •

edited

Loading

MichaelBuessemeyer left a comment •

edited

Loading

MichaelBuessemeyer left a comment •

edited

Loading