Page MenuHome

UI: Reduce Node Contents Jiggling When Moved
ClosedPublic

Authored by Harley Acheson (harley) on Jun 23 2021, 8:32 PM.
Tokens
"Y So Serious" token, awarded by shader."Love" token, awarded by kurocle."Love" token, awarded by antoniov."Love" token, awarded by calra."Like" token, awarded by AlexeyAdamitsky."Love" token, awarded by hitrpr."Love" token, awarded by Stig."Love" token, awarded by pablovazquez."Like" token, awarded by GeorgiaPacific."Love" token, awarded by Alaska."Love" token, awarded by franMarz."Love" token, awarded by billreynish."Like" token, awarded by Fracture128."Love" token, awarded by irfan."Cup of Joe" token, awarded by LazyDodo."Love" token, awarded by lone_noel."Love" token, awarded by mswf."Love" token, awarded by Raimund58."100" token, awarded by ankitm."Love" token, awarded by HooglyBoogly.

Details

Summary

This patch just clamps and rounds node contents and socket locations
so they don't appear to jiggle around when you move them. This issue
happens because node sizing and positioning is in floats while text
content must be pixel-aligned.


When dragging nodes around you can quite often see their parts and contents jiggling around.

With this type of issue we quite often find that it is the contents of the container that are not aligning correctly, but in this case it is the container that is wrong. This is because the drawing of the node background and outline will properly handle float values, while the layout items can only use integers. Therefore we must truncate the positions and sizes of the container. Doing so almost all the jiggling is gone. Of course there might be other parts and pieces that still jiggle and require further work, but this is the vast majority of it.

BeforeAfter

Diff Detail

Repository
rB Blender

Event Timeline

Harley Acheson (harley) requested review of this revision.Jun 23 2021, 8:32 PM
Harley Acheson (harley) created this revision.
Harley Acheson (harley) edited the summary of this revision. (Show Details)
Harley Acheson (harley) edited the summary of this revision. (Show Details)
Harley Acheson (harley) edited the summary of this revision. (Show Details)Jun 23 2021, 8:40 PM
Harley Acheson (harley) edited the summary of this revision. (Show Details)
Harley Acheson (harley) edited the summary of this revision. (Show Details)Jun 23 2021, 8:51 PM

Looks like a great improvement! I just want to point out that the yellow circle with the 4 white triangles is still jiggling, though 😛

Is "trunc" really necessary? AFAIK it rounds in the direction of 0 (so up if negative, down if positive). Maybe "ceiling" or "floor" would work here?

Harley Acheson (harley) planned changes to this revision.Jun 28 2021, 10:42 PM

Although this helps in many cases, I'm not quite understanding when it does not. Have to investigate further.

Harley Acheson (harley) edited the summary of this revision. (Show Details)

Same solution (aligning container to screen coordinates) but slightly simpler approach that works a bit better.

One weird thing in here is a change of definition of NODE_SOCKDY. It is currently set at (0.08f * U.widget_unit) but that works out to 1.6 which results in the height being sometimes inconsistent. Changed to 0.1f and so it works out to a whole number like all the other defines here.

Dalai Felinto (dfelinto) added 1 blocking reviewer(s): User Interface.

Hi @Harley Acheson (harley) I missed the poke on this indeed. It seems better, go for it. (assuming someone else from the UI team +1 the code itself). Great job

Updated to current state of master.

Hans Goudey (HooglyBoogly) requested changes to this revision.Aug 9 2021, 10:04 PM
Hans Goudey (HooglyBoogly) added inline comments.
source/blender/editors/space_node/node_draw.cc
627–628

Should we just store these as ints?

Either way, it would be good to have a comment about this somewhere, maybe on these values or in node_to_view? Otherwise the changes seem quite arbitrary, if not wrong, counterintuitively!

This revision now requires changes to proceed.Aug 9 2021, 10:04 PM
Harley Acheson (harley) updated this revision to Diff 41021.EditedAug 23 2021, 9:31 PM

Updated to current state of master.

@Hans Goudey (HooglyBoogly) - Should we just store these as ints? Either way, it would be good to have a comment about this somewhere, maybe on these values or in node_to_view? Otherwise the changes seem quite arbitrary, if not wrong, counterintuitively!

I added comments to all the changed lines to make it more clear what is going on. Really it is just a a couple of rounds for the outer border and then a few more for the socket locations.

I can't see any way of fixing this issue in a nicer way. Although the node socket locations can easily be ints, I don't like the idea of change those without also changing the more important node's totr, butr, and prvr to rcti from rctf. Attempting to change those makes a massive mess.

I think this patch looks pretty straight-forward now with the better comments?

Harley Acheson (harley) retitled this revision from UI: Node Contents Jiggling to UI: Reduce Node Contents Jiggling When Moved.Aug 27 2021, 8:16 PM
Harley Acheson (harley) edited the summary of this revision. (Show Details)
This revision was not accepted when it landed; it landed in state Needs Review.Aug 27 2021, 8:28 PM
This revision was automatically updated to reflect the committed changes.