Geometry Nodes: Glitch when connecting a socket to the Group Input #114461

New Issue

Open

opened 2023-11-03 15:12:59 +01:00 by Dalai Felinto · 6 comments

Dalai Felinto commented

2023-11-03 15:12:59 +01:00

Owner

System Information
Operating system: Linux-6.2.0-36-generic-x86_64-with-glibc2.35 64 Bits, X11 UI
Graphics card: NVIDIA RTX A5000/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 525.125.06

Blender Version
Broken: version: 3.3 LTS, 3.6.4, 4.0 beta
Worked: ?

Short description of error
Glitch when connecting a socket to the Group Input.

In Blender 3.3 the glitch is there, but only for a moment. On 3.6 (and 4.0) you need to move the mouse for it to go away.

So far I can only reproduce this on the Geometry Nodes editor.

See video:

Exact steps for others to reproduce the error
Glitch observed in Geometry Nodes editor

Go to Geometry Nodes Workspace
Press the New button to create a new geometry node group
Add a node for the Primitive Star (other nodes must have this problem too)
Connect the Points input to the Group Input

**System Information** Operating system: Linux-6.2.0-36-generic-x86_64-with-glibc2.35 64 Bits, X11 UI Graphics card: NVIDIA RTX A5000/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 525.125.06 **Blender Version** Broken: version: 3.3 LTS, 3.6.4, 4.0 beta Worked: ? **Short description of error** Glitch when connecting a socket to the Group Input. In Blender 3.3 the glitch is there, but only for a moment. On 3.6 (and 4.0) you need to move the mouse for it to go away. So far I can only reproduce this on the Geometry Nodes editor. ![image](/attachments/850f4d52-770b-40b1-a3c8-2476df9e5025) See video: <video src="/attachments/c0e40194-0624-4e4f-85b4-229da423bab8" title="glitch-2023-11-03_15.07.55.mp4" controls></video> **Exact steps for others to reproduce the error** Glitch observed in Geometry Nodes editor - Go to Geometry Nodes Workspace - Press the `New` button to create a new geometry node group - Add a node for the Primitive Star (other nodes must have this problem too) - Connect the `Points` input to the `Group Input`

75 KiB

glitch-2023-11-03_15.07.55.mp4

138 KiB

Dalai Felinto added the

labels 2023-11-03 15:13:00 +01:00

Dalai Felinto added this to the User Interface project 2023-11-03 15:13:08 +01:00

Dalai Felinto added the

Nodes & Physics

label 2023-11-03 15:13:13 +01:00

Germano Cavalcante commented

2023-11-03 21:17:28 +01:00

Member

I can confirm.

I noticed that it doesn't always happen and it also doesn't happen in Debug builds.

System Information

Operating system: Windows-10-10.0.23575-SP0 64 Bits

Graphics card: Radeon (TM) RX 480 Graphics ATI Technologies Inc. 4.6.0 Core Profile Context 23.20.01.04.230725

version: 4.1.0 Alpha, branch: main, commit date: 2023-10-24 14:09, hash: `7b7cef7998aa`

I can confirm. I noticed that it doesn't always happen and it also doesn't happen in Debug builds. <details> <summary>System Information</summary> <ul> <div>Operating system: Windows-10-10.0.23575-SP0 64 Bits</div> <div>Graphics card: Radeon (TM) RX 480 Graphics ATI Technologies Inc. 4.6.0 Core Profile Context 23.20.01.04.230725</div> <div>version: 4.1.0 Alpha, branch: main, commit date: 2023-10-24 14:09, hash: `7b7cef7998aa`</div> </ul> </details>

Germano Cavalcante added

and removed

labels 2023-11-03 21:17:40 +01:00

Leon Schittek commented

2023-11-04 00:11:11 +01:00

Member

A few more observations after testing this for a bit:

It indeed only happens in the geometry nodes editor.
I can only reproduce it when linking the first input socket of a node.
I can reproduce this when linking to any node (not just the group input)
It only happens with the following types: Integer, Boolean, Float (as well as Object, Collection and Color when they have no name set)

This is what I figured out from looking through the code:

It seems only sockets using uiItemR in std_node_socket_draw cause this (basically, when the socket switches from drawing the property button to drawing a plain label)
The reason it only happens in the geometry node editor seems to be because of the tool tip buttons for sockets. Removing those also removes the issue.

It might be that ui_but_equals_old isn't properly matching the buttons when redrawing, causing the wrong flags to be copied to the label.
That doesn't completely make sense to me, yet, but making ui_but_equals_old a bit more strict by adding another comparison seems to do the trick:


diff --git a/source/blender/editors/interface/interface.cc b/source/blender/editors/interface/interface.cc
index 8fc9166190f..904a91e7916 100644
--- a/source/blender/editors/interface/interface.cc
+++ b/source/blender/editors/interface/interface.cc
@@ -767,6 +767,9 @@ static bool ui_but_equals_old(const uiBut *but, const uiBut *oldbut)
   {
     return false;
   }
+  if (but->tip_func != oldbut->tip_func) {
+    return false;
+  }
   if (but->optype != oldbut->optype) {
     return false;
   }

I can have another look to figure out if that's actually the issue, if noone with a better idea of what's happening wants to take this on.

A few more observations after testing this for a bit: * It indeed only happens in the geometry nodes editor. * I can only reproduce it when linking the first input socket of a node. * I can reproduce this when linking to any node (not just the group input) * It only happens with the following types: `Integer`, `Boolean`, `Float` (as well as `Object`, `Collection` and `Color` when they have no name set) --- This is what I figured out from looking through the code: * It seems only sockets using `uiItemR` in `std_node_socket_draw` cause this (basically, when the socket switches from drawing the property button to drawing a plain label) * The reason it only happens in the geometry node editor seems to be because of the tool tip buttons for sockets. Removing those also removes the issue. It might be that `ui_but_equals_old` isn't properly matching the buttons when redrawing, causing the wrong flags to be copied to the label. That doesn't completely make sense to me, yet, but making `ui_but_equals_old` a bit more strict by adding another comparison seems to do the trick: ```Diff diff --git a/source/blender/editors/interface/interface.cc b/source/blender/editors/interface/interface.cc index 8fc9166190f..904a91e7916 100644 --- a/source/blender/editors/interface/interface.cc +++ b/source/blender/editors/interface/interface.cc @@ -767,6 +767,9 @@ static bool ui_but_equals_old(const uiBut *but, const uiBut *oldbut) { return false; } + if (but->tip_func != oldbut->tip_func) { + return false; + } if (but->optype != oldbut->optype) { return false; } ``` I can have another look to figure out if that's actually the issue, if noone with a better idea of what's happening wants to take this on.

Dalai Felinto commented

2023-11-04 13:50:13 +01:00

Author

Owner

@HooglyBoogly feel like reviewing Leon's suggestion?

@HooglyBoogly feel like reviewing Leon's suggestion?

Julian Eisel commented

2023-11-07 15:42:39 +01:00

Member

Thanks @lone_noel, this is quite useful. I'm not sure about the snippet above either. What this function is supposed to do is compare the "identity" of the button, which is done using the data or the operation it represents. I wouldn't consider a tooltip callback part of the button's identity, so this check seems incorrect semantically, although practically it might work just fine. It will still break if buttons happen to use the same tooltip callback though.

Here's what's going on:

On redraws we reconstruct buttons, but keep the active button (the one receiving event input) to preserve its state. For that buttons are compared to find the new version of the button, and the new version gets replaced by the old one.
Here the invisible label button created for socket tooltips becomes active (it's under the cursor when the user starts dragging on the socket to create a link).
UI event handling is aborted while the user drags the link (modal node link operator runs).
After the mouse is released, the invisible label button is still active, so the UI will try to preserve its state.
When doing this, the wrong button is identified as the new version of the invisible label button. Label buttons don't usually carry data that would define any "identity", so there's nothing to compare really. So the first label button it finds will be considered matching.

I see two issues:

The button shouldn't be activate anymore after UI event handling is aborted due to a running modal operator.
The button comparison is doing a bad job here. Suggestion: Don't consider buttons matching if the old one is in an active state (for interaction) and the new one is not interactive (ui_but_is_interactive()) -- see snippet below. This will at least avoid many incorrect matches, but label buttons still don't have a real "identity" we could use for comparison.

diff --git a/source/blender/editors/interface/interface.cc b/source/blender/editors/interface/interface.cc
index bbcd30bbbfd..e546225f546 100644
--- a/source/blender/editors/interface/interface.cc
+++ b/source/blender/editors/interface/interface.cc
@@ -774,6 +774,13 @@ static bool ui_but_equals_old(const uiBut *but, const uiBut *oldbut)
     return false;
   }
 
+  /* If the old button is active for interaction, but the new one isn't even interactive, it makes
+   * no sense to consider them matching. Avoids many incorrect matches with label buttons, which
+   * usually have no data that could be used for the button comparison. */
+  if (oldbut->active && !ui_but_is_interactive(but, false)) {
+    return false;
+  }
+
   if ((but->type == UI_BTYPE_VIEW_ITEM) && (oldbut->type == UI_BTYPE_VIEW_ITEM)) {
     uiButViewItem *but_item = (uiButViewItem *)but;
     uiButViewItem *oldbut_item = (uiButViewItem *)oldbut;

Thanks @lone_noel, this is quite useful. I'm not sure about the snippet above either. What this function is supposed to do is compare the "identity" of the button, which is done using the data or the operation it represents. I wouldn't consider a tooltip callback part of the button's identity, so this check seems incorrect semantically, although practically it might work just fine. It will still break if buttons happen to use the same tooltip callback though. Here's what's going on: - On redraws we reconstruct buttons, but keep the active button (the one receiving event input) to preserve its state. For that buttons are compared to find the new version of the button, and the new version gets replaced by the old one. - Here the invisible label button created for socket tooltips becomes active (it's under the cursor when the user starts dragging on the socket to create a link). - UI event handling is aborted while the user drags the link (modal node link operator runs). - After the mouse is released, the invisible label button is still active, so the UI will try to preserve its state. - When doing this, the wrong button is identified as the new version of the invisible label button. Label buttons don't usually carry data that would define any "identity", so there's nothing to compare really. So the first label button it finds will be considered matching. I see two issues: - The button shouldn't be activate anymore after UI event handling is aborted due to a running modal operator. - The button comparison is doing a bad job here. Suggestion: Don't consider buttons matching if the old one is in an active state (for interaction) and the new one is not interactive (`ui_but_is_interactive()`) -- see snippet below. This will at least avoid many incorrect matches, but label buttons still don't have a real "identity" we could use for comparison. ```diff diff --git a/source/blender/editors/interface/interface.cc b/source/blender/editors/interface/interface.cc index bbcd30bbbfd..e546225f546 100644 --- a/source/blender/editors/interface/interface.cc +++ b/source/blender/editors/interface/interface.cc @@ -774,6 +774,13 @@ static bool ui_but_equals_old(const uiBut *but, const uiBut *oldbut) return false; } + /* If the old button is active for interaction, but the new one isn't even interactive, it makes + * no sense to consider them matching. Avoids many incorrect matches with label buttons, which + * usually have no data that could be used for the button comparison. */ + if (oldbut->active && !ui_but_is_interactive(but, false)) { + return false; + } + if ((but->type == UI_BTYPE_VIEW_ITEM) && (oldbut->type == UI_BTYPE_VIEW_ITEM)) { uiButViewItem *but_item = (uiButViewItem *)but; uiButViewItem *oldbut_item = (uiButViewItem *)oldbut; ```

Leon Schittek commented

2023-11-07 22:44:34 +01:00

Member

Thanks for the explanation, @JulianEisel.
That makes a lot of sense and that diff works great for this in my tests!

Thanks for the explanation, @JulianEisel. That makes a lot of sense and that diff works great for this in my tests!

Harley Acheson referenced a pull request that will close this issue

2023-12-15 18:54:50 +01:00

Fix #114461: Check Interactivity in button comparisons #116228

Harley Acheson referenced this issue from a commit

2023-12-20 01:14:19 +01:00

Fix #114461: Check Interactivity in button comparisons

Harley Acheson closed this issue

2023-12-20 01:14:21 +01:00

Blender Bot added

and removed

labels 2023-12-20 01:14:22 +01:00

Harley Acheson commented

2023-12-21 17:13:14 +01:00

Member

@JulianEisel > see snippet...

I tried that and it fixed this issue very nicely, but then caused problems as reported in #116384 and #116426

@JulianEisel > see snippet... I tried that and it fixed this issue very nicely, but then caused problems as reported in #116384 and #116426

Harley Acheson reopened this issue

2023-12-21 17:13:18 +01:00

Blender Bot added

and removed

labels 2023-12-21 17:13:19 +01:00

Iliya Katushenock added

and removed

labels 2023-12-21 17:34:09 +01:00

Sign in to join this conversation.

No Label

Animation & Rigging

Asset Browser Project Overview

Automated Testing

Blender Asset Bundle

Dependency Graph

Development Management

EEVEE & Viewport

Images & Movies

Motion Tracking

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Render Pipeline

Sculpt, Paint & Texture

Video Sequencer

Virtual Reality

Blender 2.8 Project

Milestone 1: Basic, Local Asset Browser

Good First Issue

Animation & Rigging

Development Management

EEVEE & Viewport

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Sculpt, Paint & Texture

Needs Info from Developers

Needs Information from User

No Milestone

No project

No Assignees

5 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#114461

No description provided.