Yeah, I see your point, the way I was rooting for would also mean stealing in that scenario, just the other way around. Well, I don’t know what to tell you, it sure looks difficult.
But if the final behaviour is as in the video, I think the visual hierarchy could be improved to indicate that it’s a co-parenting situation with no specific order. The semi-opaque colours indicate a behaviour that don’t follow what actually happens.
Anyway, I’ll sit back for now and shut up, I don’t want to muddle the thread with half-baked ideas.
Ok, new personal rule for myself - no replying at a very late hour, on my phone, on a critical point without sitting down at the computer instead of trying to rely on a failing memory.
Sincere apologies, it works as you’ve stated. I was remembering all of the “node stealing”, but not that I was letting go of the mouse.
So, having cleared that up - I’ll revert to my initial feedback on that. I don’t think one frame should be able to grab nodes from another frame in this manner. I actually realized it did this on accident when (yay for testing things) while resizing one frame (which crossed over another filled with nodes), and thought - wow, that will be a problem one day.
I think it should be considered that very frequently, node frames aren’t intentionally overlapping- it’s usually a side effect of adding a node. Your comment only considers intentional error; but this patch must also address un-intentional scenarios, and what I’m seeing in the video is a very common un-intentional scenario
No, quite the contrary. By error I generally meant something that user did not intend
What I meant by intentional error in my reply was that he intentionally created a scenario which will commonly happen to users by accident. He just did it on purpose to illustrate a point. But it doesn’t make sense to create whole new advanced behavior in Blender’s frame handling code just for frames to behave better in scenarios which users will probably not want to get into in the first place
When the user gets into such scenario, they will probably want to fix the frame overlap instead of leaving it as is, and then relying on some advanced, frame prioritizing behavior code to read their mind and move their nodes exactly according to their expectation in those rare case of one node being contained by two partially overlapping frames.
Thanks for the feedback so far, guys. A lot of food for thoughts!
I’m not yet at a point where I’m super sure about where to take things from here, but I’ll try to give my thoughts on some things.
Node Stealing
This has been contentious and I can see why considering how different it is from the current behavior.
That being said, it’s inherent to getting the bounds of the frame to actually correlate with the nodes it affects (“What you see is what you get.”). So I’d still like to pursue this idea a bit further to really see where this goes.
Very valid point! If we show overlapping frames you kinda expect it to matter.
Maybe putting a stronger focus on the border and making the border always be on top, helps? Here’s a first shot to see what that could look like.
If we should move forward with the no-parenting approach we definitely have to take a good look at visually representing the relationships clearly.
My general thoughts
I’ve been using the patch in the past few days and have been enjoying it so far. (With shrinking enabled by default - I personally find manually resizing the frame a bit bothersome.)
With shrinking enabled it doesn’t feel that different from what we have right now, but I found the ability to resize the frames to add nodes to them to be super nice (and this is only possible without parenting). It’s just very flexible to change the node-frame-relationship both ways.
This is definitely a new direction while the original patch was more of a “faster horse” type of deal.
Similar to what @LudvikKoutny mentioned I haven’t had trouble with node stealing edge cases in practice. But I’m very concious of the issues people have reported so far and am still in the process of organizing the feedback to think this through from more angles!
No harm done! (Well, you did give me a scare that I might have done something dumb…) Thanks for checking.
So it took a while, but I went through all the feedback and tried to come to some kind of conclusion. Thanks for all the participation, everyone!
Feedback So Far
If you’re interested, here are my compiled notes: Node Editor: Frames - HackMD
Though, I don’t think you’ll gain too much new insight, if you’ve been following this thread.
Overall people didn’t seem to like the node stealing issue, but did enjoy being able to resize the frames to freely add/remove nodes.
Next Experiment
To move the discussion forward, I’m now trying to see if there is a compromise that manages to avoid the pitfalls of node stealing while still giving us some of the nice things from the recent prototype.
Here’s what I came up with:
Keep the “one frame per node” (parenting) concept for now
There doesn’t seem to be an actual usecase to have multiple frames claim ownership of the same node, so why allow it.
Always add (currently unframed) nodes to frames if they are fully inside the frame
This avoids situations where an (unframed) node is on top of a frame without being part of it for the most part.
Allow resizing frames to add and remove nodes
This works with parenting as long as you accept that you can’t steal nodes from other frames doing this.
Allow unparenting nodes during transform by pressing Shift
Since the feedback on using Alt was a bit mixed, I thought it be good to give this a try as an alternative.
The improvements to frame resizing (Increased hitzone for resizing, Space to move etc.) are still part of the patch.
You can test this version of the patch with this build:
Node Stealing
Is there still node stealing? To some extend, yes. Free/unframed nodes will be joined into a frame that is moved under them, but you won’t have to fear that framed nodes will be stolen by a frame that’s partially overlapping.
I hope that this is a good compromise.
Note: The logic for adding a node to a frame is a bit more nuanced to also allow nodes being added to frames when they are part of the same frame and so on. So “unframed” is just a shorthand for “node that won’t be stolen if it’s added to this frame”.
Wait, this looks good ! I’m downloading the build now. If I understand correctly there can’t be a situation where a node is on top of one or several frames but isn’t parented to any of them ? so no need for the visual aid/outline ?
OK, this is GREAT. The shift hotkey to unparent is a godsend, since I don’t have to reach for the “unparent” hotkey anymore. Node insertion as an opt-in (alt+click) is fantastic, it’s now possible to work in cramped areas without messing everything up. And there’s never the ambiguous case where a node is over a frame but not parented to it. It doesn’t seem like much, but in practice it’s many small mental burdens lifted off of me.
I’m very, very happy about this version. Thank you so much for working on it.
Select a node within a frame, it leaves the frame as expected.
Move the frame, and that node now travels along with it.
This works as I believe is as expected:
Shrink: OFF
Click LMB
Select a node within a frame, it leaves the frame as expected.
Hold SHIFT, left go of LMB
Move the frame, and that node now stays where it was dropped.
I can see that the SHIFT combo is of value, if Shrink is ON (as it appear to over-ride the shrink, for obvious reasons), but if Shrink is off, I believe SHIFT shouldn’t be required. So perhaps some sort of… if shrink on/then shift LMB req, else LMB ok … sort of thing.
If I resize a frame, such that it unframes a contained node, that node leaves the group as it should.
Node stealing seems to be completely resolved. It picks up orphans off the “pasteboard”, but I’ve no problem with that whatsoever.
Repeating, just because it’s great and I love it: the “click border” on frames, so much easier to grab than release build.
Overall shrink behavior (other than the glitch above) works well.
Was about to comment, but more or less is the same, the only weird thing to me was what @thorn-neverwake is talking about (I noticed it with nodes and frames), because I tend to keep autoshrink disabled.
Other than that, I really like it. And I saw that in the HackMD doc there is a couple of ideas about further improvements (color and labeling I look forward to).
Very cool! Thanks for working on this, Leon.
Selecting the entire frame + nodes, then duplicating = intact copy as desired.
Selecting only the frame: it duplicates the frame, but not the nodes within.
Might be how it needs to work, but mentioning for completeness.
Dragging the edge of a node larger/smaller works 100% in affecting shrink size, no issues there.
FRAME COLORING:
Label of command is “copy color”. I did so, selected another frame, and expected to see a “paste color” option appear. Went back and forth a few times, wondering how to assign this color. Only when the tooltip appears, does the user figure out how it works. I think it may need a different label, as “copy” is a ubiquitous command in the world of computers and I think most people expect a “paste” command to be involved when offered the chance to copy.
Maybe “Apply Color to Selected Frames”…or “Assign Color”… or just “Apply to Selected”?
FRAME SELECTED command - still leaves a frack ton of empty space around the entire layout. Completely not within the scope of your work, but though I’d throw out a “while you’re in there…”
I’m glad that this version seems to be on the right track!
This is how it was supposed to work, but I made an error when updating the parenting state. Have it fixed on my end. Thanks for testing and reporting, @thorn-neverwake and @txo!
It has worked this way for a long time and I’m not sure copying the frame’s contents is always desired.
What I could see working nicely is an easy way to select a frame together with all its nodes. I would really like to just double-click a frame for that but maybe there already is an established design-paradigm for that somewhere in Blender.
I’ve never used that and agree that the naming is confusing. It seems the only reason for it to exist is that the “Copy to Selected” operator only working on nodes of the same type.
Maybe “Copy Color to Selected” would be more clear and also in line with “Copy to Selected”?
I’ll add it to the list of things to look into
For the most part, yes.
The way it works right now is that it updates the parenting states after moving nodes and after resizing frames.
I’ve noticed a few cases where this misses something but those are easily fixed.
One case that I’m inclined to keep that way is, when the frame grows because the size of its child nodes changes. Since currently the parenting isn’t updated immediately there is a moment, where nodes can be overlapping the frame without being part of it (until the next time any node is moved or resized).
The reason why I think this might be desirable is, that it allows unhiding big nodes to expose more inputs without having to worry about it immediately absorbing adjacent nodes into the frame. Here is how that looks and I could see it being annoying:
With the current rules those nodes below would not become part of the frame until a node is moved or a frame resized. So making the group input node inside the frame smaller again would also return the frame to it’s original size (unless a node has been moved or a frame resized).
Not yet sure if this is just inconsistent or cleverly aligns with what most people would want. Making it consistent is simple though so it’s just a matter of deciding what behavior is preferable.
Since this is not necessary for this patch to work, anymore, I think this will have to be discussed separately.
To me it makes sense for this to be opt-in rather than opt-out but ideally we’d be able to actually customize the modal keymap. I’m certain there are people that prefer the link insertion to be on by default.
Not as far as I know, however I read people wishing that such a thing would exist for object hierarchies before.
Makes sense to me !
That’s how it worked when I tested your build, opt-in. I had to hold alt for the node insertion to happen. I may have changed the keymap for this, I’m not sure. But at least it means it can be changed
Yeah, I hadn’t reverted that change from the original patch. I’m just trying to set expectations since it won’t be bundled with the frame changes.
From what I understand the modal keymap can’t (easily) be customized. So for the most part you’ll be stuck with whatever is the default, unfortunately.
The shift hotkey to unparent is a godsend, since I don’t have to reach for the “unparent” hotkey anymore. Node insertion as an opt-in (alt+click) is fantastic, it’s now possible to work in cramped areas without messing everything up. And there’s never the ambiguous case where a node is over a frame but not parented to it. It doesn’t seem like much, but in practice it’s many small mental burdens lifted off of me.