Add mesh actions to 3d viewport context menu #6813

MichaelBuessemeyer · 2023-02-03T16:28:41Z

This pr adds some mesh-related items to the context menu in the 3d viewport.
Note that finding/opening the mesh/segment of the mesh in the segments tab is not included, as this requires #5792 to be implemented. IMO this pr shouldn't wait for these changes as the current items are already valuable enough to be merged .

Note, that this pr is affected by bug #6814. (The context menu needs to be opened twice under certain circumstances)

URL of deployed dev instance (used for testing):

https://addmeshactionstodcontextmenu.webknossos.xyz/

Steps to test:

Open a tracing with meshes. E.g. open a new hybrid tracing with an existing segmentation layer. Compute an ad-hoc mesh using the context menu in the e.g. xy viewport.
Hover the mesh in the 3d viewport and open the context menu (likely needed twice).
Play around with the new options (hide, remove, jump, and listing the name / segment id)
In case the name of the segment of the mesh is very long, the name will be abbreviated and fully available via a tooltip.

Issues:

fixes Context-menu for 3D viewport #5803

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

Updated (unreleased) changelog
~~- [ ] Updated (unreleased) migration guide if applicable~~
Updated documentation if applicable
~~- [ ] Adapted wk-connect if datastore API changes~~
~~- [ ] Adapted wk-libs python client if relevant API parts change~~
~~- [ ] Needs datastore update after deployment~~
Ready for review

MichaelBuessemeyer · 2023-02-06T10:58:11Z

frontend/javascripts/oxalis/view/context_menu.tsx

+        infoRows.push(
+          <InfoMenuItem key="copy-cell">
+            <div className="cell-context-icon" />
+            Segment Name: {segmentName}


I am unsure how to indicate that this menu field displays the segment's name. First, I wrote Segment Name: as a prefix. But this seems to be too long IMO. Therefore I replayed it with a name tag icon. Does this make sense? Do you have a better suggestion?

frontend/javascripts/oxalis/view/right-border-tabs/segments_tab/segment_list_item.tsx

MichaelBuessemeyer · 2023-02-06T11:54:13Z

I'll do the docs update and the changelog entry on Thursday (I think).

But it should be ready for the 1st code review :)

MichaelBuessemeyer · 2023-02-06T11:56:43Z

@philippotto If you think daniel has more time / is better suited for the review, please just assign him 🙏

philippotto

Code looks already quite good 👍 When the CI is green, I can test it on a dev instance :)

frontend/javascripts/libs/utils.ts

frontend/javascripts/oxalis/view/context_menu.tsx

philippotto · 2023-02-06T13:05:14Z

frontend/javascripts/oxalis/view/context_menu.tsx

@@ -1090,15 +1188,29 @@ function ContextMenuInner(propsWithInputRef: Props) {
      );
    }

-    if (segmentIdAtPosition > 0) {
+    if (segmentIdAtPosition > 0 || maybeClickedMeshId != null) {


Could it be that both are defined? Or is one case only for the data viewports and the second case only for the 3D viewport? If both were possible, the user wouldn't know which ID is shown, right?

It is not possible that both are defined. The segmentId is calculated based upon the calculated global position of where the context menu is opened. The function used for that is _calculateMaybeGlobalPos. Thus in case of the 3d viewport, the segmentId will never be defined.
And the maybeClickedMeshI can only be defined when the context menu is opened in the 3d viewport. Thus if on variable is defined / > 0 the other cannot. They exclude each other 🎉 .

I am personally not perfectly happy with my solution here, but other alternatives would me more code duplication. Thus I added a comment shortly explaining that only one of them can be defined.

It is not possible that both are defined. The segmentId is calculated based upon the calculated global position of where the context menu is opened. The function used for that is _calculateMaybeGlobalPos. Thus in case of the 3d viewport, the segmentId will never be defined.
And the maybeClickedMeshI can only be defined when the context menu is opened in the 3d viewport. Thus if on variable is defined / > 0 the other cannot. They exclude each other tada .

Great 👍 Thanks for the explanation!

I am personally not perfectly happy with my solution here, but other alternatives would me more code duplication. Thus I added a comment shortly explaining that only one of them can be defined.

Yeah, I think, it's a good trade-off.

…-mesh-actions-to-3d-context-menu

frontend/javascripts/oxalis/view/context_menu.tsx

philippotto · 2023-02-09T09:22:14Z

Just gave it a whirl and it worked quite well 👍 Great work!

Not, that this pr is affected by bug #6814. (The context menu needs to be opened twice under certain circumstances)

Do you know these circumstances? For me, it always worked on the first try 🤔

I am unsure how to indicate that this menu field displays the segment's name. First, I wrote Segment Name: as a prefix. But this seems to be too long IMO. Therefore I replayed it with a name tag icon. Does this make sense? Do you have a better suggestion?

Two ideas:

I like the name tag icon, but I'd give it a tooltip (only the icon) which says "Segment Name"
I'd only display the name if it really is defined. Otherwise, it's redundant (see screenshot).

One thing which caught my eye is this:

If I rightclick somewhere in the 3d viewport where no mesh or skeleton is, I get the following context menu:

At first, I thought: cool, now I can even load a mesh for a segment which is rendered onto one of the planes in the 3d viewports. However, a) this doesn't work (would be complicated to support probably) and b) these context options also show up when rightclicking a white area. So, I assume this case is just not handled properly somehow? I think, it should simply show "No actions available" or something like this (I think, this string already exists in the context menu module somewhere).

MichaelBuessemeyer · 2023-02-09T10:47:53Z

Do you know these circumstances? For me, it always worked on the first try 🤔

Yeah, it is actually hard to reproduce. For me, it always worked when I created an ad-hoc mesh in the zy viewport and then immediately hovered over a mesh in the 3d viewport without clicking anywhere and then opening the context menu. Then it says "no data" 🤷‍♂️.
The same bug already exists on the master (wk-demo):

create a tracing
create some nodes in the yz viewport
open the context menu in the yz viewport and close it by also clicking somewhere in the yz viewport
hover a skeleton node in the 3d viewport and right-click. The menu is empty although it shouldn't be. Then reponing the context menu shows options.

One thing which caught my eye is this:

This definitely shouldn't be the case. I'll investigate this :)

MichaelBuessemeyer · 2023-02-09T10:56:22Z

And also: Please note, that I added the change of words in the upload view where "uploaded" changed to "imported".

…-mesh-actions-to-3d-context-menu

…ableminds/webknossos into add-mesh-actions-to-3d-context-menu

MichaelBuessemeyer · 2023-02-09T15:11:34Z

I fixed the bug and mentioned the new mesh-related context menu items in the docs

philippotto

Awesome, works great 👍

…tarea * 'master' of github.com:scalableminds/webknossos: Add mesh actions to 3d viewport context menu (#6813)

* textarea: Add mesh actions to 3d viewport context menu (#6813)

WIP adding mesh actions to 3d context menu

39a3d50

MichaelBuessemeyer self-assigned this Feb 3, 2023

Add more actions and info context menu items

e00d5c4

MichaelBuessemeyer commented Feb 6, 2023

View reviewed changes

frontend/javascripts/oxalis/view/right-border-tabs/segments_tab/segment_list_item.tsx Outdated Show resolved Hide resolved

improve segment name menu item

709bdaa

MichaelBuessemeyer marked this pull request as ready for review February 6, 2023 11:54

Merge branch 'master' into add-mesh-actions-to-3d-context-menu

e540f81

MichaelBuessemeyer added enhancement frontend mesh labels Feb 6, 2023

MichaelBuessemeyer requested a review from philippotto February 6, 2023 11:56

MichaelBuessemeyer changed the title ~~[WIP] Add mesh actions to 3d viewport context menu~~ Add mesh actions to 3d viewport context menu Feb 6, 2023

philippotto reviewed Feb 6, 2023

View reviewed changes

MichaelBuessemeyer added 2 commits February 9, 2023 08:58

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

9a4d3b4

…-mesh-actions-to-3d-context-menu

fix typing and apply feedback

55bcd78

philippotto reviewed Feb 9, 2023

View reviewed changes

frontend/javascripts/oxalis/view/context_menu.tsx Outdated Show resolved Hide resolved

change text in dataset upload to "imported" for remote datasets

a0f9565

fix typo in comment

cc5799c

MichaelBuessemeyer and others added 6 commits February 9, 2023 11:56

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

38e7eb2

…-mesh-actions-to-3d-context-menu

Merge branch 'master' into add-mesh-actions-to-3d-context-menu

247a368

fix linting

eaad39b

Merge branch 'add-mesh-actions-to-3d-context-menu' of github.com:scal…

83811d5

…ableminds/webknossos into add-mesh-actions-to-3d-context-menu

add changelog entry

af631ec

mention new mesh context menu items in docs

afba250

MichaelBuessemeyer and others added 3 commits February 9, 2023 15:57

Merge branch 'master' into add-mesh-actions-to-3d-context-menu

9b845b0

don't show mesh loading menu items in 3d viewport

0364bb1

Merge branch 'add-mesh-actions-to-3d-context-menu' of github.com:scal…

5bf1fa5

…ableminds/webknossos into add-mesh-actions-to-3d-context-menu

philippotto approved these changes Feb 9, 2023

View reviewed changes

philippotto merged commit 38a942a into master Feb 9, 2023

philippotto deleted the add-mesh-actions-to-3d-context-menu branch February 9, 2023 16:31

hotzenklotz added a commit that referenced this pull request Feb 10, 2023

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

958585d

…tarea * 'master' of github.com:scalableminds/webknossos: Add mesh actions to 3d viewport context menu (#6813)

hotzenklotz added a commit that referenced this pull request Feb 10, 2023

Merge branch 'textarea' into search

1172537

* textarea: Add mesh actions to 3d viewport context menu (#6813)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add mesh actions to 3d viewport context menu #6813

Add mesh actions to 3d viewport context menu #6813

MichaelBuessemeyer commented Feb 3, 2023 •

edited by philippotto

MichaelBuessemeyer Feb 6, 2023

MichaelBuessemeyer commented Feb 6, 2023 •

edited

MichaelBuessemeyer commented Feb 6, 2023

philippotto left a comment

philippotto Feb 6, 2023

MichaelBuessemeyer Feb 9, 2023

philippotto Feb 9, 2023

philippotto commented Feb 9, 2023 •

edited by MichaelBuessemeyer

MichaelBuessemeyer commented Feb 9, 2023 •

edited

MichaelBuessemeyer commented Feb 9, 2023 •

edited

MichaelBuessemeyer commented Feb 9, 2023

philippotto left a comment

Add mesh actions to 3d viewport context menu #6813

Add mesh actions to 3d viewport context menu #6813

Conversation

MichaelBuessemeyer commented Feb 3, 2023 • edited by philippotto

URL of deployed dev instance (used for testing):

Steps to test:

Issues:

MichaelBuessemeyer Feb 6, 2023

Choose a reason for hiding this comment

MichaelBuessemeyer commented Feb 6, 2023 • edited

MichaelBuessemeyer commented Feb 6, 2023

philippotto left a comment

Choose a reason for hiding this comment

philippotto Feb 6, 2023

Choose a reason for hiding this comment

MichaelBuessemeyer Feb 9, 2023

Choose a reason for hiding this comment

philippotto Feb 9, 2023

Choose a reason for hiding this comment

philippotto commented Feb 9, 2023 • edited by MichaelBuessemeyer

MichaelBuessemeyer commented Feb 9, 2023 • edited

MichaelBuessemeyer commented Feb 9, 2023 • edited

MichaelBuessemeyer commented Feb 9, 2023

philippotto left a comment

Choose a reason for hiding this comment

MichaelBuessemeyer commented Feb 3, 2023 •

edited by philippotto

MichaelBuessemeyer commented Feb 6, 2023 •

edited

philippotto commented Feb 9, 2023 •

edited by MichaelBuessemeyer

MichaelBuessemeyer commented Feb 9, 2023 •

edited

MichaelBuessemeyer commented Feb 9, 2023 •

edited