WIP: Node Editor: Improve working with frame nodes #108358

Leon Schittek · 2023-05-27T23:57:58+02:00

Leon Schittek commented

2023-05-27 23:57:58 +02:00

Improvements to the workflow of adding/removing nodes inside frames:

Always attach all nodes to frames that include them after each
transform
Allow detaching nodes from their parent frame during transform by
pressing the Shift key
Nodes can be added to/removed from a frame by resizing it

This is implementing the base functionality outlined in #106956.

Showcase

Improvements to the workflow of adding/removing nodes inside frames: * Always attach all nodes to frames that include them after each transform * Allow detaching nodes from their parent frame during transform by pressing the `Shift` key * Nodes can be added to/removed from a frame by resizing it This is implementing the base functionality outlined in #106956. ---- **Showcase** ![](https://projects.blender.org/attachments/73089456-9ac4-429b-9529-c4522fd97f26)

frame-joining_showcase.gif

423 KiB

frame-joining_showcase.mp4

332 KiB

👍 6 ❤️ 1

Leon Schittek added the

Module

Nodes & Physics

label 2023-05-27 23:57:58 +02:00

Leon Schittek added this to the Nodes & Physics project 2023-05-27 23:58:29 +02:00

Hans Goudey commented

2023-05-31 22:35:25 +02:00

@blender-bot package

Blender Bot commented

2023-05-31 22:35:28 +02:00

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR108358) when ready.

Leon Schittek force-pushed frame-joining-base-functionality from 5f8cec4891 to 84b46d07d8

2023-06-03 17:11:13 +02:00

Compare

Hans Goudey requested review from Simon Thommes 2023-06-03 17:23:30 +02:00

Hans Goudey requested review from Hans Goudey 2023-06-03 17:23:31 +02:00

Simon Thommes requested changes 2023-06-09 14:50:21 +02:00

Simon Thommes left a comment

I tested the patch, first up I like the changes overall and it feels quite natural and overall less error prone. That said, this system also has its issues and potential pitfalls and feels more tedious in a lot of cases (at least right now imo).
Personally, the way that it is right now I wouldn't feel comfortable merging it instead of how things currently work. But I'm quite confident that it can be pushed to be plain better and worth replacing the current system.

I do like how this is inherently enforcing good node-tree organization, as it is putting a bigger emphasis on node placement. (Something I haven't tried yet and I am wondering about though is how this is affected by, for example the auto-offset and versioning code that change the structure of node trees implicitly.)

Bugs:

Looks like this patch is breaking the behavior of inserting a node on an existing link. That should be fixed before merging.
I think that resizing a frame is missing an undo step (?) Since this now also has an impact on what frame a node belongs to you should probably be able to undo individual scaling operations.

I also noticed that the behavior of using the shift modifier key is inconsistent with the holding the Alt modifier key to extract a node from a link in terms of pressing the modifier key before or during dragging the mouse. This is already inconsistent with holding alt to prevent inserting a node on a link, so I'm not really sure what to make of it. For this patch by itself, I think the behavior of toggling during dragging (as it is now) is better.

I'm not really sure how to feel about putting a bigger emphasis on changing the frame scale to add multiple nodes. Right now it's very easy to select a group of nodes and making them part of a frame. If that frame is relatively small that results in having to shuffle things around to before you're able to contain them in the frame within one action.
In any case I find it quite tedious to find the exact spot where I can scale the frame when I have to rely on that frequently. I think that the interactive area for scaling the frame should be a lot larger, like at least 2x, other operations that compete with that real estate (frame dragging and scaling on one axis), still have the majority of area either way.

Right now the context of the selection is completely disregarded when joining nodes to a frame. I think that this is a major downside versus how it currently works in Blender.
When I want to add multiple nodes to an existing frame, working with the selection makes it very explicit and clear what the context for my operation is. Imo, relying on the position (and even bounding box) of every individual node becomes more of a hassle than actually being useful.
I personally don't see the benefit of being able to select multiple nodes at the same time to move them into positions where they will be part of different frames. Those should be separate operations anyways imo.

The same issue already exists also for single nodes that are larger than the frame you want to add them to. You end up having to do multiple actions to add a single node, resulting in intermediate steps you have to take. That is currently not an issue in Blender.

Those are my concerns that I have when trying to apply this functionality in practical workflow.
I don't have the perfect solution to everything but here are some ideas, that are totally up for discussion:

the entire selection could be treated as one element to leverage the selection as a very integral functionality for adding nodes to frames as is possible currently in Blender
to compensate for large elements/multiple nodes the sensitivity should not rely on the actual bounding box of the element. Using the bounding box center/a small area around it would be more
similarly to how you see the effect of toggling shift immediatly, maybe it could make sense to do the same while moving nodes, to give intermediate feedback what the result would be as a node is being placed (kind of like snapping). Alternatively the point/area used for snapping to a frame could be shown as an overlay.

Sorry for the large dump of feedback, I don't have time to make it more concise.

I tested the patch, first up I like the changes overall and it feels quite natural and overall less error prone. That said, this system also has its issues and potential pitfalls and feels more tedious in a lot of cases (at least right now imo). Personally, the way that it is right now I wouldn't feel comfortable merging it instead of how things currently work. But I'm quite confident that it can be pushed to be plain better and worth replacing the current system. I do like how this is inherently enforcing good node-tree organization, as it is putting a bigger emphasis on node placement. (Something I haven't tried yet and I am wondering about though is how this is affected by, for example the auto-offset and versioning code that change the structure of node trees implicitly.) Bugs: - Looks like this patch is breaking the behavior of inserting a node on an existing link. That should be fixed before merging. - I think that resizing a frame is missing an undo step (?) Since this now also has an impact on what frame a node belongs to you should probably be able to undo individual scaling operations. I also noticed that the behavior of using the shift modifier key is inconsistent with the holding the Alt modifier key to extract a node from a link in terms of pressing the modifier key before or during dragging the mouse. This is already inconsistent with holding alt to prevent inserting a node on a link, so I'm not really sure what to make of it. For this patch by itself, I think the behavior of toggling during dragging (as it is now) is better. I'm not really sure how to feel about putting a bigger emphasis on changing the frame scale to add multiple nodes. Right now it's very easy to select a group of nodes and making them part of a frame. If that frame is relatively small that results in having to shuffle things around to before you're able to contain them in the frame within one action. In any case I find it quite tedious to find the exact spot where I can scale the frame when I have to rely on that frequently. I think that the interactive area for scaling the frame should be a lot larger, like at least 2x, other operations that compete with that real estate (frame dragging and scaling on one axis), still have the majority of area either way. Right now the context of the selection is completely disregarded when joining nodes to a frame. I think that this is a major downside versus how it currently works in Blender. When I want to add multiple nodes to an existing frame, working with the selection makes it very explicit and clear what the context for my operation is. Imo, relying on the position (and even bounding box) of every individual node becomes more of a hassle than actually being useful. I personally don't see the benefit of being able to select multiple nodes at the same time to move them into positions where they will be part of different frames. Those should be separate operations anyways imo. The same issue already exists also for single nodes that are larger than the frame you want to add them to. You end up having to do multiple actions to add a single node, resulting in intermediate steps you have to take. That is currently not an issue in Blender. Those are my concerns that I have when trying to apply this functionality in practical workflow. I don't have the perfect solution to everything but here are some ideas, that are totally up for discussion: - the entire selection could be treated as one element to leverage the selection as a very integral functionality for adding nodes to frames as is possible currently in Blender - to compensate for large elements/multiple nodes the sensitivity should not rely on the actual bounding box of the element. Using the bounding box center/a small area around it would be more - similarly to how you see the effect of toggling shift immediatly, maybe it could make sense to do the same while moving nodes, to give intermediate feedback what the result would be as a node is being placed (kind of like snapping). Alternatively the point/area used for snapping to a frame could be shown as an overlay. Sorry for the large dump of feedback, I don't have time to make it more concise.

Leon Schittek added 4 commits 2023-06-18 10:32:11 +02:00

f8e08a370a Merge branch 'main' into frame-joining-base-functionality

30ccfbe006 Fix link insertion flag not being properly set when starting transform.

6dbe863e3e Merge branch 'main' into frame-joining-base-functionality

6c85b27b94 Remove unnecessary check.

Leon Schittek commented

2023-06-18 10:45:13 +02:00

Thanks for testing and the comprehensive feedback. I'm still trying to organize my reply to the points about the design, so for now I'll just address the functional issues with the PR.

Bugs:

Looks like this patch is breaking the behavior of inserting a node on an existing link. That should be fixed before merging.

Done!

I think that resizing a frame is missing an undo step (?) Since this now also has an impact on what frame a node belongs to you should probably be able to undo individual scaling operations.

I'm not sure I can reproduce this. Maybe I'm misunderstanding, but for me resizing nodes can already be undone step-by-step, which also undoes the parenting. (Perhaps this was a case of undo in the node editor not working when in edit mode?)

I think that the interactive area for scaling the frame should be a lot larger, like at least 2x, other operations that compete with that real estate (frame dragging and scaling on one axis), still have the majority of area either way.

I think the build on the bot above was from before #108359 got merged, which made the area for resizing bigger in most cases. Maybe that's already enough.

Thanks for testing and the comprehensive feedback. I'm still trying to organize my reply to the points about the design, so for now I'll just address the functional issues with the PR. > Bugs: > - Looks like this patch is breaking the behavior of inserting a node on an existing link. That should be fixed before merging. Done! > - I think that resizing a frame is missing an undo step (?) Since this now also has an impact on what frame a node belongs to you should probably be able to undo individual scaling operations. I'm not sure I can reproduce this. Maybe I'm misunderstanding, but for me resizing nodes can already be undone step-by-step, which also undoes the parenting. (Perhaps this was a case of undo in the node editor not working when in edit mode?) > I think that the interactive area for scaling the frame should be a lot larger, like at least 2x, other operations that compete with that real estate (frame dragging and scaling on one axis), still have the majority of area either way. I think the build on the bot above was from before https://projects.blender.org/blender/blender/pulls/108359 got merged, which made the area for resizing bigger in most cases. Maybe that's already enough.

Leon Schittek commented

2023-06-28 13:49:18 +02:00

Sorry for taking so long to get back at you.I found it hard to keep things short, so if you guys feel that there is a better place for discussion, we can move this to a different channel!

Overall, I think the most essential (and least controversial) improvement this patch brings is the ability to easily unparent nodes by pressing Shift while moving them.

I still see value in tying the content of frames strongly to what is inside them, since it’s just so conceptually simple and removes some potential sources errors. But, I think you've made fair points, and I'll trust your instincts, if you think it's too much of a hassle. :)

Simon's ideas

What you describe, doesn’t sound too different from my inital proposal.
Some notes on the individual points below.

the entire selection could be treated as one element to leverage the selection as a very integral functionality for adding nodes to frames as is possible currently in Blender

I think this can work as a premise. But the devil is in the details.

Even right now Blender doesn't always join all selected nodes into frames.
E.g. nodes that are already parented to another frame won't be added to a frame when dragged (see attached video).
I think that behavior makes some sense intuitively, but it is what prompted me to only enable adding nodes to frames after unparenting them by pressing Shift -that way you’ll always join all selected nodes into the frame.

The big drawback of always having to press Shift to add nodes into frames is, that you are much more likely to accidentally not add them to the frame, which is also annoying and leads to a lot of cases where nodes are on top of frames without being part of them.

Maybe it's fine to keep the current joining behavior we have now and only add Shift to unparent them during moving.

to compensate for large elements/multiple nodes the sensitivity should not rely on the actual bounding box of the element. Using the bounding box center/a small area around it would be more [convenient?]

In my initial prototype I went for using the center of the selection - and this can work alright.

I've since come around on that, though, and think using the cursor is better.
When tweaking nodes the cursor is usually the user's point of focus anyway and making them refocus on another point seems odd.
It also avoids some edge cases that arise e.g. when the selection is much larger that the view, which I found hard to make feel good when I experimented with that.

similarly to how you see the effect of toggling shift immediatly, maybe it could make sense to do the same while moving nodes, to give intermediate feedback what the result would be as a node is being placed (kind of like snapping). Alternatively the point/area used for snapping to a frame could be shown as an overlay.

Do you mean something like previewing the frames as if the nodes were joined? I feel like that could be a bit confusing.

Some form of immediate feedback to telegraph what is about to happen would be good, though. I originally proposed highlighting the frame, but a small icon next to the cursor could work could also work. Highlighting the frame is very clear, but adds a lot of visual noise.

I also updated the proptotype patch (#105457) so you could compare it to this approach. There it works like this:

pressing Shift during transform detaches nodes from frames
Not pressing Shift won't add nodes to frames
Add a highlight to frames when nodes are about to be added to them

Sorry for taking so long to get back at you.I found it hard to keep things short, so if you guys feel that there is a better place for discussion, we can move this to a different channel! --- Overall, I think the most essential (and least controversial) improvement this patch brings is the ability to easily unparent nodes by pressing `Shift` while moving them. I still see value in tying the content of frames strongly to what is inside them, since it’s just so conceptually simple and removes some potential sources errors. But, I think you've made fair points, and I'll trust your instincts, if you think it's too much of a hassle. :) #### Simon's ideas What you describe, doesn’t sound too different from [my inital proposal](https://devtalk.blender.org/t/testing-wanted-for-joining-nodes-into-frames/28241#proposal-1). Some notes on the individual points below. > - the entire selection could be treated as one element to leverage the selection as a very integral functionality for adding nodes to frames as is possible currently in Blender I think this can work as a premise. But the devil is in the details. Even right now Blender doesn't always join _all_ selected nodes into frames. E.g. nodes that are already parented to another frame won't be added to a frame when dragged (see attached video). I think that behavior makes some sense intuitively, but it is what prompted me to only enable adding nodes to frames after unparenting them by pressing `Shift` -that way you’ll always join all selected nodes into the frame. The big drawback of always having to press `Shift` to add nodes into frames is, that you are much more likely to accidentally _not_ add them to the frame, which is also annoying and leads to a lot of cases where nodes are on top of frames without being part of them. Maybe it's fine to keep the current joining behavior we have now and only add `Shift` to unparent them during moving. > - to compensate for large elements/multiple nodes the sensitivity should not rely on the actual bounding box of the element. Using the bounding box center/a small area around it would be more [convenient?] In my initial prototype I went for using the center of the selection - and this can work alright. I've since come around on that, though, and think using the cursor is better. When tweaking nodes the cursor is usually the user's point of focus anyway and making them refocus on another point seems odd. It also avoids some edge cases that arise e.g. when the selection is much larger that the view, which I found hard to make feel good when I experimented with that. > - similarly to how you see the effect of toggling shift immediatly, maybe it could make sense to do the same while moving nodes, to give intermediate feedback what the result would be as a node is being placed (kind of like snapping). Alternatively the point/area used for snapping to a frame could be shown as an overlay. Do you mean something like previewing the frames as if the nodes were joined? I feel like that could be a bit confusing. Some form of immediate feedback to telegraph what is about to happen would be good, though. I originally proposed highlighting the frame, but a small icon next to the cursor could work could also work. Highlighting the frame is very clear, but adds a lot of visual noise. --- I also updated the proptotype patch (https://projects.blender.org/blender/blender/pulls/105457) so you could compare it to this approach. There it works like this: * pressing `Shift` during transform detaches nodes from frames * Not pressing `Shift` won't add nodes to frames * Add a highlight to frames when nodes are about to be added to them

frame-joining-exception.mp4

161 KiB

Simon Thommes commented

2023-09-04 12:36:35 +02:00

I haven't tested the adjusted patch yet, will do so in the coming days hopefully, now that I have some time between projects.

I just wanted to respond to some of the points.

Overall, I think the most essential (and least controversial) improvement this patch brings is the ability to easily unparent nodes by pressing Shift while moving them.

Totally agree

Even right now Blender doesn't always join all selected nodes into frames.
E.g. nodes that are already parented to another frame won't be added to a frame when dragged (see attached video).
I think that behavior makes some sense intuitively, but it is what prompted me to only enable adding nodes to frames after unparenting them by pressing Shift -that way you’ll always join all selected nodes into the frame.

I'm not a fan of adding a modifier key to this operation for workflow reasons. The current behavior is to me a totally acceptable compromise. You could still use the shift key to first unparent and basically override the frame parenting. That sounds logical to me. It would only be necessary in edge cases.

Highlighting the frame is very clear, but adds a lot of visual noise.

I think highlighting the frame a node would be added to would be great. If this only happens whenever the parent frame would change, not when moving a node within the same frame, this would be the right level of visual noise to notify the user imo.

When tweaking nodes the cursor is usually the user's point of focus anyway and making them refocus on another point seems odd.
It also avoids some edge cases that arise e.g. when the selection is much larger that the view, which I found hard to make feel good when I experimented with that.

The main problem with the cursor is when you move the nodes not by dragging but by pressing G. Then the position of the cursor is entirely arbitrary. I agree that the cursor is a clear point of reference that eliminates some issues ambiguity, but it isn't intuitive in all cases. That's why I was proposing to base the point of reference on the intuitive bounding box center of the selection and indicating that point in the UI during the operation.

I haven't tested the adjusted patch yet, will do so in the coming days hopefully, now that I have some time between projects. I just wanted to respond to some of the points. > Overall, I think the most essential (and least controversial) improvement this patch brings is the ability to easily unparent nodes by pressing `Shift` while moving them. Totally agree > Even right now Blender doesn't always join all selected nodes into frames. > E.g. nodes that are already parented to another frame won't be added to a frame when dragged (see attached video). > I think that behavior makes some sense intuitively, but it is what prompted me to only enable adding nodes to frames after unparenting them by pressing Shift -that way you’ll always join all selected nodes into the frame. I'm not a fan of adding a modifier key to this operation for workflow reasons. The current behavior is to me a totally acceptable compromise. You could still use the shift key to first unparent and basically override the frame parenting. That sounds logical to me. It would only be necessary in edge cases. > Highlighting the frame is very clear, but adds a lot of visual noise. I think highlighting the frame a node would be added to would be great. If this only happens whenever the parent frame would change, not when moving a node within the same frame, this would be the right level of visual noise to notify the user imo. > When tweaking nodes the cursor is usually the user's point of focus anyway and making them refocus on another point seems odd. > It also avoids some edge cases that arise e.g. when the selection is much larger that the view, which I found hard to make feel good when I experimented with that. The main problem with the cursor is when you move the nodes not by dragging but by pressing `G`. Then the position of the cursor is entirely arbitrary. I agree that the cursor is a clear point of reference that eliminates some issues ambiguity, but it isn't intuitive in all cases. That's why I was proposing to base the point of reference on the intuitive bounding box center of the selection and indicating that point in the UI during the operation.

👍 1

Leon Schittek changed title from ~~Node Editor: Improve working with frame nodes~~ to WIP: Node Editor: Improve working with frame nodes

2023-09-22 13:58:32 +02:00

Leon Schittek commented

2023-09-22 14:11:50 +02:00

@SimonThommes I think that all sounds good and I updated the patch #105457 accordingly, including a basic indicator for the selection bounding box center.

The main problem with the cursor is when you move the nodes not by dragging but by pressing G. Then the position of the cursor is entirely arbitrary. I agree that the cursor is a clear point of reference that eliminates some issues ambiguity, but it isn't intuitive in all cases.

Yeah, this is an odd quirk. That being said, the ability to position your mouse strategically before moving nodes so you can join them into a frame without having the added nodes overlap any on the nodes already inside the frame can be nice.

But I honestly don't feel very strongly either way, so let's go with the bounding box center!

I think we also can move review over to #105457 then. If that moves forward I'll close this PR but I think it's nice to have this design still somewhat documented here.

@SimonThommes I think that all sounds good and I updated the patch #105457 accordingly, including a basic indicator for the selection bounding box center. > The main problem with the cursor is when you move the nodes not by dragging but by pressing `G`. Then the position of the cursor is entirely arbitrary. I agree that the cursor is a clear point of reference that eliminates some issues ambiguity, but it isn't intuitive in all cases. Yeah, this is an odd quirk. That being said, the ability to position your mouse strategically before moving nodes so you can join them into a frame without having the added nodes overlap any on the nodes already inside the frame can be nice. But I honestly don't feel very strongly either way, so let's go with the bounding box center! --- I think we also can move review over to #105457 then. If that moves forward I'll close this PR but I think it's nice to have this design still somewhat documented here.

Leon Schittek referenced this pull request

2023-09-22 14:24:29 +02:00

WIP: Node Editor: Improve frame joining during transform #105457

Hadrien commented

2023-09-22 14:30:35 +02:00

First-time contributor

The main problem with the cursor is when you move the nodes not by dragging but by pressing G. Then the position of the cursor is entirely arbitrary. I agree that the cursor is a clear point of reference that eliminates some issues ambiguity, but it isn't intuitive in all cases.

Perhaps we could make this behaviour more obvious by highlighting hovered frames when transforming nodes. For someone who doesn't know about it, I imagine it can look as if nodes are sometimes parented to frames randomly.

> The main problem with the cursor is when you move the nodes not by dragging but by pressing G. Then the position of the cursor is entirely arbitrary. I agree that the cursor is a clear point of reference that eliminates some issues ambiguity, but it isn't intuitive in all cases. Perhaps we could make this behaviour more obvious by highlighting hovered frames when transforming nodes. For someone who doesn't know about it, I imagine it can look as if nodes are sometimes parented to frames randomly.

Simon Thommes commented

2023-09-25 12:30:08 +02:00

@lone_noel when you get the chance, could you update this with current main?
I tried building it based on the old main, but it seems to not have worked properly.

@lone_noel when you get the chance, could you update this with current main? I tried building it based on the old main, but it seems to not have worked properly.

Simon Thommes commented

2023-09-25 12:34:39 +02:00

On the behavior of the unparenting: It would be good to have the same behavior when Shift is held before the nodes are dragged. Having to do the strict order of drag, shift, drop is a bit awkward to keep up in the workflow.
That's consistent for example with shift dragging objects in the outliner.

On the behavior of the unparenting: It would be good to have the same behavior when Shift is held before the nodes are dragged. Having to do the strict order of drag, shift, drop is a bit awkward to keep up in the workflow. That's consistent for example with shift dragging objects in the outliner.

👍 1

Leon Schittek added 2 commits 2023-09-25 15:25:48 +02:00

01b02ef80c Merge branch 'main' into frame-joining-base-functionality

021c584553 Merge branch 'main' into frame-joining-base-functionality

Simon Thommes commented

2023-09-25 19:49:38 +02:00

Thank you for updating the patch! But it looks like this was a misunderstanding, sorry. I didn't realize when you were referring to another patch #105457 . That's my bad, I will test that one to compare. I see you also updated that one 👍

Simon Thommes referenced this pull request

2023-09-25 20:14:40 +02:00

WIP: Node Editor: Improve frame joining during transform #105457

This pull request has changes conflicting with the target branch.

source/blender/editors/space_node/node_relationships.cc
source/blender/editors/transform/transform.hh
source/blender/editors/transform/transform_convert_node.cc

View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u frame-joining-base-functionality:lone_noel-frame-joining-base-functionality

git checkout lone_noel-frame-joining-base-functionality

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

WIP: Node Editor: Improve working with frame nodes #108358

Simon's ideas

Checkout