Nodes: automatically move viewer node to current node #121951

Jacques Lucke · 2024-05-18T18:28:11+02:00

Jacques Lucke commented

2024-05-18 18:28:11 +02:00

This improves working with viewer nodes in geometry and compositor nodes. Previously, the viewer node would typically stay at the position where it was first inserted which leads to very long links in many cases. Now the viewer node automatically moves to the place where the user ctrl+shift+clicked to view data.

The viewer is placed slightly to the right and top of the current node. It is moved up a bit, so that it does not get in the way as quickly when the user wants to add another new node. Furthermore, the viewer node position is chosen so that it does not intersect with other nodes.

In the future we could implement animating the node position so that it slowly transitions to it's new places.

This improves working with viewer nodes in geometry and compositor nodes. Previously, the viewer node would typically stay at the position where it was first inserted which leads to very long links in many cases. Now the viewer node automatically moves to the place where the user ctrl+shift+clicked to view data. The viewer is placed slightly to the right and top of the current node. It is moved up a bit, so that it does not get in the way as quickly when the user wants to add another new node. Furthermore, the viewer node position is chosen so that it does not intersect with other nodes. In the future we could implement animating the node position so that it slowly transitions to it's new places. ![image](/attachments/2a91c01c-2bd1-427b-ab5a-e7be72503e01)

image.png

24 KiB

❤️ 10 🚀 2

Jacques Lucke added 3 commits 2024-05-18 18:28:23 +02:00

9e1f902059 initial viewer node positioning

b02bd0603c better viewer positioning

34f8be0341 add comment

Jacques Lucke added 5 commits 2024-05-18 19:02:37 +02:00

c07614bf18 use ui scale

1b6a16cf4b reposition node even if the socket is linked to the viewer already

84fe45764b Merge branch 'main' into viewer-node-position

29e17d8a58 fix

4c99edbe69 improve handling of frames

Jacques Lucke requested review from Simon Thommes 2024-05-18 19:03:04 +02:00

Jacques Lucke requested review from Hans Goudey 2024-05-18 19:03:04 +02:00

Hans Goudey reviewed 2024-05-20 01:36:08 +02:00

Hans Goudey left a comment

I didn't think too hard about the details of the algorithm. It didn't seem worth it since it works really nicely in my testing, like night and day with the crazy long links we had before.

The only think I can think of that would make this even cooler is if the viewer node animated to the new position. That might make things a little less disorienting? Actually seems pretty simple to do that by generalizing NodeStackAnimationData or so, but it's also not necessary.

I didn't think too hard about the details of the algorithm. It didn't seem worth it since it works really nicely in my testing, like night and day with the crazy long links we had before. The only think I can think of that would make this even cooler is if the viewer node animated to the new position. That might make things a little less disorienting? Actually seems pretty simple to do that by generalizing `NodeStackAnimationData` or so, but it's also not necessary.

👍 1

source/blender/editors/space_node/node_relationships.cc Outdated

						
				@ -590,0 +599,4 @@

				 * If possible, the viewer is aligned to another node that is close by to result in a better

				 * looking node position.

				 */

				static void position_viewer_node(bNodeTree &tree,

Hans Goudey commented

2024-05-20 01:29:34 +02:00

Looks like all the arguments could be const besides the viewer node

JacquesLucke marked this conversation as resolved

source/blender/editors/space_node/node_relationships.cc Outdated

						
				@ -590,0 +625,4 @@

				    BLI_rctf_pad(&main_candidate_rect, default_padding, default_padding);

				    const bNode *collided_node = nullptr;

				    LISTBASE_FOREACH (const bNode *, node, &tree.nodes) {

Hans Goudey commented

2024-05-20 01:29:18 +02:00

for (const bNode *node : tree.all_nodes())

`for (const bNode *node : tree.all_nodes())`

JacquesLucke marked this conversation as resolved

source/blender/editors/space_node/node_relationships.cc Outdated

						
				@ -590,0 +661,4 @@

				    bool found_collision_for_aligned_rect = false;

				    LISTBASE_FOREACH (const bNode *, node, &tree.nodes) {

				      if (node->type == NODE_FRAME) {

Hans Goudey commented

2024-05-20 01:33:12 +02:00

node->is_frame()?

`node->is_frame()`?

JacquesLucke marked this conversation as resolved

Jacques Lucke added 4 commits 2024-05-20 19:05:15 +02:00

3333c25320 Merge branch 'main' into viewer-node-position

b05ddafa66 use const args

e1b20c29c9 use tree.all_nodes()

6817aa3ea3 use node.is_frame()

Jacques Lucke commented

2024-05-20 19:06:08 +02:00

Animating the viewer movement seems nice, but won't do it as part of this patch, especially because it also requires this other refactor to be done first.

Hans Goudey approved these changes 2024-05-20 19:07:59 +02:00

Hans Goudey commented

2024-05-20 19:08:29 +02:00

Sounds good. Maybe animation could be mentioned as future work in the description?

Simon Thommes requested changes 2024-05-22 18:07:23 +02:00

Dismissed

Simon Thommes left a comment

This feels quite good for a start 👍

I did run into some weird behavior here and there, particularly when there is no free space towards the top, where the viewer gets stacked quite far away from the target node.

Some ideas:

Right now it seems quite clear that the node checks incrementally upwards for free space. It would make sense to me to alternate between incrementing up and down to also catch free space just below, if there are a lot of nodes above. (Alternatively even searching in an arch pattern to include free spaces further to the right.)
I think a simple heuristic that would help would be to check if the new chosen location is further from the target node than the previous one and just leave it in place if so (probably only when the viewer is to the right of the target)

This feels quite good for a start 👍 I did run into some weird behavior here and there, particularly when there is no free space towards the top, where the viewer gets stacked quite far away from the target node. Some ideas: - Right now it seems quite clear that the node checks incrementally upwards for free space. It would make sense to me to alternate between incrementing up and down to also catch free space just below, if there are a lot of nodes above. (Alternatively even searching in an arch pattern to include free spaces further to the right.) - I think a simple heuristic that would help would be to check if the new chosen location is further from the target node than the previous one and just leave it in place if so (probably only when the viewer is to the right of the target)

Simon Thommes commented

2024-05-22 18:10:19 +02:00

Another thought:

Could it make sense to make the viewer node active when using the CTRL+SHIFT+LMB functionality rather than the node that is clicked on? That would help highlighting it and make it easy to move manually right after.
I'm not sure what purpose the current functionality is serving regarding that, but it would feel pretty simple to me to just make the node active again that I just clicked on for the viewer and still have under my cursor.
But maybe I'm missing something.

Another thought: Could it make sense to make the viewer node active when using the CTRL+SHIFT+LMB functionality rather than the node that is clicked on? That would help highlighting it and make it easy to move manually right after. I'm not sure what purpose the current functionality is serving regarding that, but it would feel pretty simple to me to just make the node active again that I just clicked on for the viewer and still have under my cursor. But maybe I'm missing something.

👍 2

Simon Thommes commented

2024-05-22 18:28:07 +02:00

I forgot one more small note:
When there is no near node found for horizontal alignment the distance should be more than it is now/potentially just the same as the margin for auto offset, where we just set the default to 40. Looks like right now it's 20.

I forgot one more small note: When there is no near node found for horizontal alignment the distance should be more than it is now/potentially just the same as the margin for auto offset, where we just set the default to 40. Looks like right now it's 20.

Jacques Lucke added 5 commits 2024-05-29 13:02:30 +02:00

d6be462c88 Merge branch 'main' into viewer-node-position

2008134029 Merge branch 'main' into viewer-node-position

1f2fa4a9d7 extract function to find overlapping node

ff0d500a59 cleanup

3b529bac45 try different search pattern

Simon Thommes commented

2024-05-29 14:16:53 +02:00

The new search pattern works a lot better for me!

It's still trying quite hard not to put it below the target node. But that's probably fine to avoid putting it in the way of new nodes.

For my testing so far I only used it for inspecting existing node-trees rather than in an actual workflow. For the actual fine-tuning I have the feeling it might be easier to do when the initial version is in main.

2 points that I have left:

could it make sense to use the currently visible area of the node-editor and interpret points that are out of view as a collision? To avoid placing the viewer out of view
I still do like the idea of leaving the viewer where it is in certain cases. I do think in cases where the user moved the viewer manually into a more convenient place and then keeps inspecting the same general area of nodes, it might be more annoying than useful to have it jump around. One case especially that I'd like to avoid for example is moving it back when cycling through outputs of the same node it is already connected to, after the user moved it manually.
So I would propose to try two things:
- if the viewer is already connected to the targeted node: don't move it, just cycle through the outputs
- if the viewer is already to the right of the target node, it is currently not colliding (+in view) and the new found position is further from the target than the current one: don't move it, just connect

I think that logic could work well. Not sure if you would try these things in a separate PR.

The new search pattern works a lot better for me! It's still trying quite hard not to put it below the target node. But that's probably fine to avoid putting it in the way of new nodes. For my testing so far I only used it for inspecting existing node-trees rather than in an actual workflow. For the actual fine-tuning I have the feeling it might be easier to do when the initial version is in main. 2 points that I have left: - could it make sense to use the currently visible area of the node-editor and interpret points that are out of view as a collision? To avoid placing the viewer out of view - I still do like the idea of leaving the viewer where it is in certain cases. I do think in cases where the user moved the viewer manually into a more convenient place and then keeps inspecting the same general area of nodes, it might be more annoying than useful to have it jump around. One case especially that I'd like to avoid for example is moving it back when cycling through outputs of the same node it is already connected to, after the user moved it manually. So I would propose to try two things: - if the viewer is already connected to the targeted node: don't move it, just cycle through the outputs - if the viewer is already to the right of the target node, it is currently not colliding (+in view) and the new found position is further from the target than the current one: don't move it, just connect I think that logic could work well. Not sure if you would try these things in a separate PR.

👍 1

Jacques Lucke added 5 commits 2024-05-29 15:34:41 +02:00

d7b2d9a132 cleanup

7cc14ce7ee take region bounds into account

12a296895b Merge branch 'main' into viewer-node-position

a311760117 remove reliance on socket position

7773b4ed04 avoid moving node if the new location is further away

Jacques Lucke added 1 commit 2024-05-29 15:36:53 +02:00

072f499734 fix description

Jacques Lucke commented

2024-05-29 15:39:50 +02:00

I implemented some of the suggested improvements. Please have another look.

I don't have a good example where I moved the viewer manually where I would want it to stay at a specific location when viewer another socket of the same node again. So I did not implement that part yet. The viewer generally shouldn't end up further than it was anymore though.

When not moving the viewer manually, cycling through the sockets of a single node should not move the viewer.

I implemented some of the suggested improvements. Please have another look. I don't have a good example where I moved the viewer manually where I would want it to stay at a specific location when viewer another socket of the same node again. So I did not implement that part yet. The viewer generally shouldn't end up further than it was anymore though. When not moving the viewer manually, cycling through the sockets of a single node should not move the viewer.

Simon Thommes approved these changes 2024-05-29 16:11:18 +02:00

Simon Thommes left a comment

This is pretty good to me now!
If we identify certain cases where it doesn't perform we can still tweak that later, but this is good to go into main imo.

Looking into animating the position as a transition could still be interesting, but it's not required

This is pretty good to me now! If we identify certain cases where it doesn't perform we can still tweak that later, but this is good to go into main imo. Looking into animating the position as a transition could still be interesting, but it's not required

Jacques Lucke merged commit 7be4d4f443 into main

2024-05-29 16:49:47 +02:00

Jacques Lucke referenced this issue from a commit

2024-05-29 16:49:47 +02:00

Nodes: automatically move viewer node to current node

Jacques Lucke deleted branch viewer-node-position

2024-05-29 16:49:50 +02:00

John Kiril Swenson referenced this issue from a commit

2024-06-05 05:27:05 +02:00

Nodes: automatically move viewer node to current node

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

Nodes: automatically move viewer node to current node #121951