Added Backdrop Node for Meshroom graph #2574

waaake · 2024-10-21T16:49:17Z

TCSR-1194

Feature Request

Added Backdrop Node
Notion of Region inside a Graph

Minor Fix

Added Key Press of Backspace to allow deletion in the graph.

cbentejac · 2024-10-24T09:53:34Z

meshroom/core/desc.py

+class Backdrop(InputNode):
+    """ A Backdrop for other nodes.
+    """
+
+    # The internal inputs' of Backdrop Node needs a Integer Field to determine the font size for the comment
+    internalInputs = [
+        StringParam(
+            name="invalidation",
+            label="Invalidation Message",
+            description="A message that will invalidate the node's output folder.\n"
+                        "This is useful for development, we can invalidate the output of the node when we modify the code.\n"
+                        "It is displayed in bold font in the invalidation/comment messages tooltip.",
+            value="",
+            semantic="multiline",
+            advanced=True,
+            uidIgnoreValue="",  # If the invalidation string is empty, it does not participate to the node's UID
+        ),
+        StringParam(
+            name="comment",
+            label="Comments",
+            description="User comments describing this specific node instance.\n"
+                        "It is displayed in regular font in the invalidation/comment messages tooltip.",
+            value="",
+            semantic="multiline",
+            invalidate=False,
+        ),
+        IntParam(
+            name="fontSize",
+            label="Font Size",
+            description="The Font size for the User Comment on the Backdrop.",
+            value=12,
+            range=(6, 100, 1),
+        ),
+        FloatParam(
+            name="nodeWidth",
+            label="Node Width",
+            description="The Backdrop Node's Width.",
+            value=600,
+            range=None,
+            enabled=False # Hidden always
+        ),
+        FloatParam(
+            name="nodeHeight",
+            label="Node Height",
+            description="The Backdrop Node's Height.",
+            value=400,
+            range=None,
+            enabled=False # Hidden always
+        ),
+        ColorParam(
+            name="color",
+            label="Color",
+            description="Custom color for the node (SVG name or hexadecimal code).",
+            value="",
+            invalidate=False,
+        )
+    ]
+


Is there a reason why we totally overwrite the internal inputs instead of using the ones from the parent class and adding "Font Size" to it?

With the current way, there's a lot of duplication and the "Node's Label" internal attribute is lost, which I think is a shame since we may want to name the backdrop itself, and not just use the "Comments" attribute. This would prevent having several backdrops named "Backdrop", "Backdrop2", "Backdrop3", and so on... if we want to name them. Similarly, the "Invalidation Message" is currently preserved although it has no impact anywhere so far.

cbentejac · 2024-10-24T14:28:38Z

meshroom/ui/qml/GraphEditor/GraphEditor.qml

@@ -135,7 +139,7 @@ Item {
    Keys.onPressed: {
        if (event.key === Qt.Key_F) {
            fit()
-        } else if (event.key === Qt.Key_Delete) {
+        } else if (event.key === Qt.Key_Delete || event.key === Qt.Key_Backspace) { // Backspace supports both Windows and MacOS Keyboards


Not directly related to this, but when a backdrop contains nodes, is there no way to remove the background only and not the background AND everything it contains? (that's a real question, not necessarily a change request)

codecov · 2024-12-18T07:14:38Z

Codecov Report

Attention: Patch coverage is 69.04762% with 26 lines in your changes missing coverage. Please review.

Project coverage is 69.90%. Comparing base (0586336) to head (6a97f48).
Report is 29 commits behind head on develop.

Files with missing lines	Patch %	Lines
meshroom/core/node.py	48.93%	24 Missing ⚠️
meshroom/core/desc/node.py	96.15%	1 Missing ⚠️
meshroom/core/graph.py	66.66%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #2574      +/-   ##
===========================================
- Coverage    69.93%   69.90%   -0.03%     
===========================================
  Files          121      122       +1     
  Lines         7088     7158      +70     
===========================================
+ Hits          4957     5004      +47     
- Misses        2131     2154      +23

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Attribute factory serves as a common place to initialize and group attributes and define an accessor to be able to retrieve the attributes

Backdrop Node serves as a Graphical Node allowing only internal attributes to be listed and can be serialized

…ines

Added Backdrop Node descriptor which is a derived Incomputable Node with Resizable Attribute Traits

The Loader allows loading Backdrop Component for backdrop nodes and Standard Node component for other nodes

…constructor type

…nstructor for Node Types

…at a given index With the change in nodeRepeater to hold the Loader instead of active delegate, fetching the item at a given index involves getting the loader at index and then it's item. This function allows easy access to the item directly.

…m rather than the loader delegate

…tem rather than the loader delegate

When dragging a backdrop node from the left side the node has to be moved and resized at the same time and the same needs to be done when an undo is performed, in order to do that with a single undo, a grouping is necessary for the two operations

…y when the drag of the backdrop is complete

…UI Graph The disableAnimation flag would be used in the UI to control when an Animation is to occur

The Grouped UI Modfications can be used to disabled overall animations for the UI Graph when commands get executed during both undo and redo operation.

…for qml elements Used graph's disabledAnimation flag to halt the animations when a backdrop node is resized during an undo.

fabiencastan · 2025-01-11T11:00:31Z

meshroom/core/desc/node.py

+    ]
+
+    @classmethod
+    def getInternalParameters(cls, traits: Traits) -> List[Union[StringParam, ColorParam, IntParam, FloatParam]]:


I don't see a reason to hardcode a list of specific param types. -> List[Attribute]

I was hesitant to import Attribute just for this purpose, but it's updated now.

fabiencastan · 2025-01-11T11:14:41Z

By default, when we move the backdrop, the elements within it are also moved. If we use Ctrl+Click, we only move the background, but when we stop dragging the backdrop, the elements in it are still moved!

fabiencastan · 2025-01-11T11:20:26Z

The brightness of the node's default color should remain low for the links to be visible.

fabiencastan · 2025-01-11T11:27:57Z

Minimal size for the backdrop is too large:

fabiencastan · 2025-01-11T11:41:29Z

Copy-paste does not work:

pcoip_client_cli_RbRMhBQYbj.mp4

fabiencastan · 2025-01-11T21:59:03Z

meshroom/core/graph.py

+        # Get the Node Constructor which should be initialized for the given node type
+        # Node or Backdrop...
+        NodeType = getPreferredNodeConstructor(nodeType)
+
+        n = self.addNode(NodeType(nodeType, position=position, **kwargs), uniqueName=name)


Directly use a function to create the node, instead of a function to return the node type.

Suggested change

# Get the Node Constructor which should be initialized for the given node type

# Node or Backdrop...

NodeType = getPreferredNodeConstructor(nodeType)

n = self.addNode(NodeType(nodeType, position=position, **kwargs), uniqueName=name)

n = self.addNode(createNode(nodeType, position=position, **kwargs), uniqueName=name)

This has been updated.

fabiencastan · 2025-01-12T10:45:23Z

meshroom/core/desc/builtins.py

@@ -0,0 +1,12 @@
+""" Defines the Built in Plugins.


I would expect the embedded nodes to be in a folder loaded automatically by Meshroom, in the same way than we are doing for external nodes:
meshroom/nodes/utils/Backdrop.py

Discussing with @yann-lty initially to have a way to register builtin plugins.
So have added this accordingly.
We can discuss and simplify this if needed 🙂

fabiencastan · 2025-01-12T11:04:17Z

meshroom/core/__init__.py

+def initBuiltinNodePlugins():
+    """ Registers the Builtin plugins for Meshroom.
+    """
+    registerNodeType(builtins.Backdrop)


We can remove that if we move it to the standard folder.

fabiencastan · 2025-01-12T11:48:12Z

meshroom/ui/qml/GraphEditor/Backdrop.qml

+
+        cursorShape: drag.active ? Qt.ClosedHandCursor : Qt.ArrowCursor
+
+        /// Backdrop Resize Controls ???


fabiencastan · 2025-01-12T11:48:41Z

meshroom/ui/qml/GraphEditor/Backdrop.qml

+        }
+
+        ///
+        /// Resize Bottom


Probably wants to be able to resize from the top too.

Resize Top has been added.

fabiencastan · 2025-01-12T11:51:22Z

meshroom/ui/qml/GraphEditor/Backdrop.qml

+                        if (root.height < 300) {
+                            root.height = 300;
+                        }


Probably want to declare this constant somewhere else.
This value is too big.

set as const value of 200, which seems more reasonable.

fabiencastan · 2025-01-12T11:53:08Z

meshroom/ui/qml/GraphEditor/Backdrop.qml

+        ///
+        /// Resize Bottom
+        ///
+        Rectangle {


It would be good to display a line on hover as we do between top widgets.

Would be used to save any node children for a given node, most likely a Backdrop node, could be extended in future for Group nodes or subGraph

[core] Graph: Updated addNode and pasteNode functions to refer to createNode method from core Node

…ode may have

…set to 200 by default Backdrop has a darker color compared to the base color to make the edges of nodes inside it visible

… halts animation while resizing operations

The node children are referenced later during the copy of the node and can be pasted to allow copying of the contents of the backdrop along with it

waaake · 2025-01-13T05:00:49Z

Minimal size for the backdrop is too large:

This is updated to be a const value set to 200 w and 200 h by default

waaake · 2025-01-13T05:03:45Z

Copy-paste does not work:

pcoip_client_cli_RbRMhBQYbj.mp4
Aah, I had missed to check this, my bad !!
This is updated to now copy-paste the backdrop along with the contents of it.

waaake · 2025-01-13T05:06:14Z

By default, when we move the backdrop, the elements within it are also moved. If we use Ctrl+Click, we only move the background, but when we stop dragging the backdrop, the elements in it are still moved!

Currently the standard Nodes also have this behaviour, can be fixed in a separate one if you say or in this ?

waaake added the feature request feature request from the community label Oct 21, 2024

waaake requested review from fabiencastan and cbentejac October 21, 2024 16:49

waaake self-assigned this Oct 21, 2024

fabiencastan added majorFeature and removed feature request feature request from the community labels Oct 22, 2024

waaake force-pushed the dev/BackdropNode branch from e5b1ba2 to 50d7003 Compare October 22, 2024 18:02

cbentejac added this to the Meshroom 2024.1.0 milestone Oct 23, 2024

cbentejac reviewed Oct 24, 2024

View reviewed changes

waaake marked this pull request as draft November 26, 2024 11:28

waaake force-pushed the dev/BackdropNode branch 2 times, most recently from 0a4bf9f to e73c4df Compare December 18, 2024 07:12

waaake marked this pull request as ready for review December 18, 2024 07:13

waaake requested a review from yann-lty December 18, 2024 07:13

waaake added 11 commits January 1, 2025 20:04

[desc] Node Internal Attributes: Added Attribute factory

868c6b9

Attribute factory serves as a common place to initialize and group attributes and define an accessor to be able to retrieve the attributes

[core] Backdrop Node: Added Notion of Backdrop as a Node

eb8f7ea

Backdrop Node serves as a Graphical Node allowing only internal attributes to be listed and can be serialized

[desc] Node Attribute Traits: Added Resizable Trait and attribute def…

2313ef3

…ines

[desc] Added Notion of Builtin Backdrop

0c01687

Added Backdrop Node descriptor which is a derived Incomputable Node with Resizable Attribute Traits

[ui] Geom2d: Added a way to detect full intersection between two rects

093ce7a

[ui][qml] Added Backdrop Node component in QML

f8c5ec5

[ui] Graph: Added Functionality to modify node position and dimensions

0c05baa

[ui] GraphEditor: Added Loader for Node Delegate

03e0133

The Loader allows loading Backdrop Component for backdrop nodes and Standard Node component for other nodes

[core] Builtins: Added registration mechanism for Builtin plugins

e33b22d

[core] Node: Added Constructor Preference to allow defining the Node …

1c65ada

…constructor type

[core] Graph: Updated Node Construction to rely on using preferred co…

bf6f765

…nstructor for Node Types

waaake force-pushed the dev/BackdropNode branch from e73c4df to cf16a9d Compare January 1, 2025 14:35

waaake added 3 commits January 2, 2025 09:08

[ui] GraphEditor: Updated bounding box to look at the actual node ite…

2532de4

…m rather than the loader delegate

[ui] DelegateSelectionBox: Updated selection to refer to the actual i…

e54d1a1

…tem rather than the loader delegate

waaake force-pushed the dev/BackdropNode branch from cf16a9d to e54d1a1 Compare January 2, 2025 03:46

waaake added 6 commits January 7, 2025 06:13

[ui] Backdrop: Updated the signal emission from QML to be invoked onl…

081a584

…y when the drag of the backdrop is complete

[ui] GraphEditor: Backdrop Resizing updates

0fdd044

[ui] GraphEditor: Added flag for holding the animation state for the …

d0b4626

…UI Graph The disableAnimation flag would be used in the UI to control when an Animation is to occur

[ui] Commands: Added Grouped UI Modification Command

54a569a

The Grouped UI Modfications can be used to disabled overall animations for the UI Graph when commands get executed during both undo and redo operation.

[ui] Graph: Using Grouped UI Graph Modification to disable animation …

68e1ba3

…for qml elements Used graph's disabledAnimation flag to halt the animations when a backdrop node is resized during an undo.

fabiencastan reviewed Jan 11, 2025

View reviewed changes

fabiencastan reviewed Jan 12, 2025

View reviewed changes

waaake added 8 commits January 13, 2025 10:04

[desc] Node: Updated return hint -> List[Attribute] on getInternalParams

8fb7d55

[core] Node: Added childNodes to BaseNode

4d39774

Would be used to save any node children for a given node, most likely a Backdrop node, could be extended in future for Group nodes or subGraph

[core] Node: Add method to create a node instance in memory

0447cd3

[core] Graph: Updated addNode and pasteNode functions to refer to createNode method from core Node

[ui] Graph: Copying Nodes also considers any child nodes the parent n…

f6853d7

…ode may have

[ui] Backdrop: Backdrop has a const defined minimum height and width …

ee4eb7f

…set to 200 by default Backdrop has a darker color compared to the base color to make the edges of nodes inside it visible

[ui] Backdrop: Backdrop can be resized from the top as well

87bf2e9

[ui] GraphEditor: Backdrop Node Animations considers reszing on Y and…

9b77310

… halts animation while resizing operations

[ui] Backdrop: Backdrop updates the node children when selected

6a97f48

The node children are referenced later during the copy of the node and can be pasted to allow copying of the contents of the backdrop along with it

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Backdrop Node for Meshroom graph #2574

Added Backdrop Node for Meshroom graph #2574

waaake commented Oct 21, 2024

cbentejac Oct 24, 2024

cbentejac Oct 24, 2024

codecov bot commented Dec 18, 2024 •

edited

Loading

fabiencastan Jan 11, 2025 •

edited

Loading

waaake Jan 13, 2025

fabiencastan commented Jan 11, 2025

fabiencastan commented Jan 11, 2025

fabiencastan commented Jan 11, 2025

fabiencastan commented Jan 11, 2025

fabiencastan Jan 11, 2025

waaake Jan 13, 2025

fabiencastan Jan 12, 2025 •

edited

Loading

waaake Jan 13, 2025

fabiencastan Jan 12, 2025

fabiencastan Jan 12, 2025

fabiencastan Jan 12, 2025

waaake Jan 13, 2025

fabiencastan Jan 12, 2025

waaake Jan 13, 2025

fabiencastan Jan 12, 2025

waaake commented Jan 13, 2025

waaake commented Jan 13, 2025

waaake commented Jan 13, 2025


		cursorShape: drag.active ? Qt.ClosedHandCursor : Qt.ArrowCursor

		/// Backdrop Resize Controls ???

Added Backdrop Node for Meshroom graph #2574

Are you sure you want to change the base?

Added Backdrop Node for Meshroom graph #2574

Conversation

waaake commented Oct 21, 2024

TCSR-1194

Feature Request

Minor Fix

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Dec 18, 2024 • edited Loading

Codecov Report

fabiencastan Jan 11, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabiencastan commented Jan 11, 2025

fabiencastan commented Jan 11, 2025

fabiencastan commented Jan 11, 2025

fabiencastan commented Jan 11, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabiencastan Jan 12, 2025 • 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

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

waaake commented Jan 13, 2025

waaake commented Jan 13, 2025

waaake commented Jan 13, 2025

codecov bot commented Dec 18, 2024 •

edited

Loading

fabiencastan Jan 11, 2025 •

edited

Loading

fabiencastan Jan 12, 2025 •

edited

Loading