Meshroom icon indicating copy to clipboard operation
Meshroom copied to clipboard

Show generated images in 2D viewer when double-clicking on node

Open mugulmd opened this issue 2 years ago • 17 comments

Description

When double-clicking on the PrepareDenseScene, DepthMap or DepthMapFilter node, the generated images (undistorted images, depth maps, sim maps) corresponding to the currently selected viewpoint will automatically appear in the 2D viewer.

This behaviour can be applied on other nodes that generate images with the following changes in the node description :

  • if it doesn't already exist, add an output File attribute
  • set its 'semantic' field to 'image'
  • set its value to a string describing the output image files, using the <VIEW_ID> macro to represent the corresponding viewId (see DepthMap.py for an example).

Implementation remarks

  • Viewer2D.qml: the ComboBox doesn't represent an image type anymore but rather the selected output attribute name (corresponding to the images to be visualized)
  • Viewer2D.qml: we now hold a reference to the node being displayed
  • onCurrentItemChanged in WorkspaceView.qml has been replaced with onSelectedViewIdChanged in Viewer2D.qml as it removes a dependency between 2 UI elements with a more manageable dependency between backend and frontend

mugulmd avatar Sep 21 '22 12:09 mugulmd

Need to adjust size for the combobox width: image

fabiencastan avatar Sep 22 '22 18:09 fabiencastan

Maybe add "panoramicImage" for PanoramaMerging output that can be loaded in 2D and 3D.

fabiencastan avatar Sep 22 '22 18:09 fabiencastan

Need to check that the node is computed to load it in the 2D viewer (or at least running?).

fabiencastan avatar Sep 22 '22 18:09 fabiencastan

If there is no param of image type to a node, it should not replace the last active node of the 2D Viewer. For instance, loading the Meshing node in the 3D Viewer should not unload the active node of the 2D Viewer, same for FeatureMatching, etc. But we also need a way to unload it to see the original images from the ImageGallery.

fabiencastan avatar Sep 22 '22 18:09 fabiencastan

Not specific to the PR, it's an historical problem: when changing the selection in the ImageGallery using the keyboard, it would be good to update the UI accordingly, as it's done when changing the selected image with the mouse.

fabiencastan avatar Sep 22 '22 18:09 fabiencastan

If there is no param of image type to a node, it should not replace the last active node of the 2D Viewer. For instance, loading the Meshing node in the 3D Viewer should not unload the active node of the 2D Viewer, same for FeatureMatching, etc. But we also need a way to unload it to see the original images from the ImageGallery.

I am not sure of how we could make such a behaviour user-friendly... Right now, if you want to switch to the images from the ImageGallery while something's already loaded, you have to unload the node (by "loading" another node that doesn't output images). If loading a node in the 3D viewer shouldn't unload the current 2D viewer node (unless it also has 2D ouputs), and we shouldn't replace it by a node that has no output, then what could unload it (and how)? Maybe this could be avoided by adding a classic "image" choice in the combo box for nodes that can be loaded in the 2D viewer:

  • if no node has been loaded in the 2D viewer yet, then images from the ImageGallery are displayed as usual
  • if a node has been loaded in the 2D viewer (let's say a DepthMap node), then the user can chose to see the depth, sim or classic image from the ImageGallery with the combo box
  • if a node is loaded in the 3D viewer (with no 2D outputs) while another is already loaded in the 2D viewer, then it is correctly loaded in the 3D viewer and nothing happens in the 2D viewer (and the "image" choice in the combo box is still available to display the regular images)
  • if a node is loaded in the 2D viewer and another node that has no output is double-clicked, then nothing happens

The issue is that there won't be any way to unload a node without replacing it by another.

cbentejac avatar Sep 23 '22 13:09 cbentejac

The issue is that there won't be any way to unload a node without replacing it by another.

Yes, but IMH that's not a problem as long as we can still display the source image as you suggested. BTW I have not double checked, if the behavior is fine if we remove the active node from the Graph.

fabiencastan avatar Sep 23 '22 22:09 fabiencastan

Detail: maybe good to add mask and weight to PanoramaWarping.

fabiencastan avatar Sep 28 '22 21:09 fabiencastan

We need to keep the capability to drag&drop external images into the 2D Viewer.

fabiencastan avatar Sep 28 '22 21:09 fabiencastan

In the combobox, use the Label instead of the technical name of the attribute to match the information displayed in the NodeEditor.

fabiencastan avatar Sep 28 '22 21:09 fabiencastan

Need to be able to load the node when we have chunks with mixed status: image

fabiencastan avatar Sep 28 '22 21:09 fabiencastan

Double click on a node when already on "Gallery" should switch to the first element of the combobox again. Currently it's doing if I am on DepthMap node, switch to gallery, then double clic on PrepareDenseScene. But it's not doing it if I'm on DepthMap node and switch to DepthMapFilter node (as the list of output is the same.

BUT it's a good think that I can be on "Sim Maps" and switch between DepthMap node and DepthMapFilter node.

fabiencastan avatar Sep 29 '22 17:09 fabiencastan

Maybe rename "Gallery" as "Image Gallery" in the combo box. And also update the label of the Image Gallery widget from "Images" to "Image Gallery".

fabiencastan avatar Sep 29 '22 17:09 fabiencastan

BTW could you update the labels of the output params of PanoramaMerging from "Output Folder" to "Panorama"? And remove the "Output" prefix from all the labels (mainly in the Panorama pipeline).

fabiencastan avatar Sep 29 '22 17:09 fabiencastan

Maybe good also to add a close button at the right of the node name to remove it from the 2D Viewer: image

fabiencastan avatar Sep 29 '22 17:09 fabiencastan

Double click on a node when already on "Gallery" should switch to the first element of the combobox again. Currently it's doing if I am on DepthMap node, switch to gallery, then double clic on PrepareDenseScene. But it's not doing it if I'm on DepthMap node and switch to DepthMapFilter node (as the list of output is the same.

BUT it's a good think that I can be on "Sim Maps" and switch between DepthMap node and DepthMapFilter node.

So should we keep the current behavior or change to always re-initialize the ComboBox index when we change node ?

It seems like the current behavior can be useful for comparing the output of DepthMap and DepthMapFilter without clicking too many things, and it doesn't impact anything else, so maybe keep it (for now at least) ?

mugulmd avatar Sep 30 '22 12:09 mugulmd

Maybe good also to add a close button at the right of the node name to remove it from the 2D Viewer: image

There's already a menu item in the image viewer header to toggle on/off the visibility of the node name and image filepath, so that would be a bit redundant don't you think ?

mugulmd avatar Sep 30 '22 12:09 mugulmd

There's already a menu item in the image viewer header to toggle on/off the visibility of the node name and image filepath, so that would be a bit redundant don't you think ?

I think the "close" button would unload the node from the viewer, not hide the node's name and path.

cbentejac avatar Oct 04 '22 07:10 cbentejac