Details

Reviewers

Commits

rB9a86255da8dc: Node Editor: Visual tweaks to node links

Summary

This patch to tweaks the node links a bit to make them fit in
better with the overall look of the node editor:

Change the link thickness with the zoom level to a certain degree.
Remove the fuzziness of the node link and its shadow/outline.
Add circles at the end of dragged links when connecting to sockets.
Improve the banding of the color interpolation along the link.
Adjust the spacing of dashes along straight node links

Comparison

	master	patch
overview of different link types
color gradient
dash spacing on straight node links

Circles for dragged links
Other than most links, links that are currently dragged are drawn on top
of nodes. The clipped ends of the links drawn on top of the node socket
didn't look very polished.
Not having the shadow allows us to just cover the ends up with circles
that fit nicely into the sockets the link is dragged from/to giving
additional visual feedback when the link snaps into place.

Diff Detail

Repository

rB Blender

Branch

pretty-node-links (branched from master)

Build Status

Buildable 22286
Build 22286: arc lint + arc unit

Event Timeline

Leon Schittek (lone_noel) requested review of this revision.May 26 2022, 7:10 PM

Leon Schittek (lone_noel) created this revision.

Harbormaster completed remote builds in B22286: Diff 51877.May 26 2022, 7:10 PM

Leon Schittek (lone_noel) edited the summary of this revision. (Show Details)May 26 2022, 7:18 PM

Leon Schittek (lone_noel) added projects: User Interface, Restricted Project, Nodes & Physics.

Ludvik Koutny (rawalanche) awarded a token.May 26 2022, 7:50 PM

At first I wasn't sure about the thickness of the wires when close up, but looking at the other zoom levels it's much better. A trade-off of adapting to zoom.

Such a nice improvement, love the styling when wires snap.

+1 from me (UI wise).

Pablo Vazquez (pablovazquez) awarded a token.May 27 2022, 6:18 PM

Hans Goudey (HooglyBoogly) added a reviewer: Hans Goudey (HooglyBoogly).May 29 2022, 10:26 AM

Now that the shadow is gone I noticed there are a few things that can be simplified.

I will do another pass, before anyone should take a closer look.

Cleanup the changes in the shader.
Adjust the link thickness and its clamping a bit to work better with standard dpi screens.
Remove unnecessary geometry of the links that were used to draw the shadow.
Remove all uses of the shadow color.
Change naming of the "Wire" user preference since it now only changes the socket outline color.

Harbormaster completed remote builds in B22398: Diff 52187.Jun 2 2022, 10:46 PM

Additionally to the updated patch, I tried how it would look with an outline:

	outline	no outline (patch or transparent outline)	master
dark theme
white theme

As mentioned in the patch description there are cases, where the link and background
color could be very similar (e.g the white theme without the outline and now color overlay).
This can be circumvented by choosing different theme colors but considering the
variety of colors the links can have depending on state or connected socket, that might
not always be feasible.
Also, accessibility had been brought up recently and the ability to make themes that
prevent situations of low contrast might be desirable.

With themes, where the outline color is similar to the background (like the default dark theme)
the only real difference is, that the outline is visible on the node sockets.
That's not nice, but allowing the outline color to have alpha would fix this, because the
outline could be disabled (made transparent) by the theme.

I guess my questions are:

Outline: Yes or No?
If "Yes", should we allow alpha for the outline color so it can be disabled depending on theme.

Fix missing highlight for "node drop" functionality.
Even out the dash spacing on straight node links.

Harbormaster completed remote builds in B22499: Diff 52445.Jun 11 2022, 5:42 PM

Leon Schittek (lone_noel) edited the summary of this revision. (Show Details)Jun 11 2022, 5:45 PM

Nice improvements!

About the outline question, why are the links so invisible for the white theme only at the high zoom level? At the medium and far zoom levels they look fine.

source/blender/editors/space_node/drawnode.cc
1982	Couldn't it be `const float color[4]`?
2104	Debug print should be removed
source/blender/gpu/shaders/gpu_shader_2D_nodelink_vert.glsl
91–98	It looks like snake case is preferred for local variables here, it's a bit nicer anyway
source/blender/makesrna/intern/rna_userdef.c
2858 ↗	(On Diff #52445)	What is this used for now?

Update to current state off master.
Address comments.

Harbormaster completed remote builds in B22563: Diff 52603.Jun 16 2022, 6:48 PM

In D15036#410363, @Hans Goudey (HooglyBoogly) wrote:

About the outline question, why are the links so invisible for the white theme only at the high zoom level? At the medium and far zoom levels they look fine.

The viewport with the high zoom level is the only one where I disabled the node link color overlay to show the links "natural" coloring that happens to blend in quite a bit with the background.
In this instance it can be easily by just changing the link color to something else, but there certainly are (nice, background-worthy) shades of grey that also have bad contrast with the colors we use for the sockets, which I didn't consider in the original patch:

Turns out the shadow wasn't just there to be fancy!
Thinking forward it might be unfortunate to lock themes out of using certain colors to accommodate the hard coded socket colors.

source/blender/makesrna/intern/rna_userdef.c
2858 ↗	(On Diff #52445)	In master this is used to color the node link shadow, the node socket outline and the line under the node header when the node is muted Since the current patch (without the outline) removes point (1), I thought it would be nice to name it according to the most obvious use in the UI.

I see, thanks for the explanation. Though it does look good in the default theme, based on what you're saying it probably makes sense to add the outlines/shadow back and make sure they're theme-able. Do you agree?

Yes! I wasn't too sure at first, but at this point I believe keeping a theme-able outline will be better in the long run.

Paul Larson (GeorgiaPacific) added a subscriber: Paul Larson (GeorgiaPacific).Jun 20 2022, 9:18 PM

Add node link outline and adjust thickness a little more.
Cleanup: Use NodeLinkDrawConfig to pass around data.

Harbormaster completed remote builds in B23405: Diff 54824.Aug 19 2022, 9:44 PM

Leon Schittek (lone_noel) edited the summary of this revision. (Show Details)Aug 19 2022, 10:00 PM

Vitalii Vlasov (Fracture128) awarded a token.Aug 20 2022, 5:47 AM

The changes look good to me, thanks for working on this.

source/blender/editors/space_node/drawnode.cc
1941–1943	If you feel like going a little further, this is probably a situation to use `ColorTheme4f`
2023	This struct is large enough that it should probably be passed by reference
2183	What do you think about drawing the end markers using `SpaceNode_Runtime::linkdrag` rather than checking it for every link. Doing that in a separate loop just feels a bit clearer conceptually. It's already in a separate loop anyway.