Fix #112462: Panel buttons remain active after redrawing #112503

Lukas Tönne · 2023-09-18T11:28:30+02:00

Lukas Tönne commented

2023-09-18 11:28:30 +02:00

This seems to be an issue deep inside UI drawing code. The nodes render
the panel button in the correct place,
but it doesn't correctly update the button state after a redraw, unless
the mouse is moved. The button is still in a BUTTON_STATE_HIGHLIGHT
state after drawing, and a mouse press will trigger it regardless of
where the mouse cursor actually is.

A common hack to force a reset of the button state is to add a fake
mouse move event using WM_event_add_mousemove.

This seems to be an issue deep inside UI drawing code. The nodes render the panel button in the correct place, but it doesn't correctly update the button state after a redraw, unless the mouse is moved. The button is still in a `BUTTON_STATE_HIGHLIGHT` state after drawing, and a mouse press will trigger it regardless of where the mouse cursor actually is. A common hack to force a reset of the button state is to add a fake mouse move event using `WM_event_add_mousemove`.

Lukas Tönne added 1 commit 2023-09-18 11:28:45 +02:00

4b9fa425ce Fix #112462 : Panel buttons remain active after redrawing.

This seems to be an issue deep inside UI drawing code. The nodes render
the panel button in the correct place ([code](64976a396b/source/blender/editors/space_node/node_draw.cc (L2033-L2050))),
but it doesn't correctly update the button state after a redraw, unless
the mouse is moved. The button is still in a `BUTTON_STATE_HIGHLIGHT`
state after drawing, and a mouse press will trigger it regardless of
where the mouse cursor actually is.

A common hack to force a reset of the button state is to add a fake
mouse move event using `WM_event_add_mousemove`.

Lukas Tönne requested review from Jacques Lucke 2023-09-18 11:28:54 +02:00

Lukas Tönne requested review from Julian Eisel 2023-09-18 11:29:02 +02:00

Jacques Lucke commented

2023-09-18 13:10:04 +02:00

Never knew about this hack, but indeed looks reasonable given how this is used elsewhere. Will leave this to a UI dev to review though.

Jacques Lucke refused to review 2023-09-18 13:10:09 +02:00

Iliya Katushenock commented

2023-09-18 13:34:15 +02:00

Does this mean that you are now just always redrawing the editor?

Lukas Tönne added this to the Nodes & Physics project 2023-09-18 13:47:59 +02:00

Lukas Tönne commented

2023-09-18 13:59:44 +02:00

Does this mean that you are now just always redrawing the editor?

No, the WM_event_add_mousemove does not trigger a redraw, it just resets the button state so that a click event won't toggle the button.

> Does this mean that you are now just always redrawing the editor? No, the `WM_event_add_mousemove` does not trigger a redraw, it just resets the button state so that a click event won't toggle the button.

👍 1

Hans Goudey reviewed 2023-09-18 14:26:25 +02:00

Hans Goudey left a comment

Hmm, I don't see this hack used in other drawing code.

but it doesn't correctly update the button state after a redraw, unless the mouse is moved

Any idea why this is a problem now, or why it isn't a problem elsewhere?

Hmm, I don't see this hack used in other drawing code. >but it doesn't correctly update the button state after a redraw, unless the mouse is moved Any idea why this is a problem now, or why it isn't a problem elsewhere?

Lukas Tönne commented

2023-09-18 14:29:49 +02:00

Any idea why this is a problem now, or why it isn't a problem elsewhere?

It's only a problem when you don't move the mouse after hiding sockets and click, which is probably quite rare in practice. The issue only happens when buttons change location on redraw. This workaround was suggested by @JulianEisel who maybe knows other places this is used.

> Any idea why this is a problem now, or why it isn't a problem elsewhere? It's only a problem when you don't move the mouse after hiding sockets and click, which is probably quite rare in practice. The issue only happens when buttons change location on redraw. This workaround was suggested by @JulianEisel who maybe knows other places this is used.

Hans Goudey commented

2023-09-18 14:41:11 +02:00

Could the workaround be applied in the place that hide sockets then, rather than being in every node redraw? Just think it being a bit more localized and close to the source of the problem might be better.

TBH, I wonder why BUTTON_STATE_HIGHLIGHT isn't cleared if the mouse isn't over the button anymore. But maybe that question is out of scope here :P

Could the workaround be applied in the place that hide sockets then, rather than being in every node redraw? Just think it being a bit more localized and close to the source of the problem might be better. TBH, I wonder why `BUTTON_STATE_HIGHLIGHT` isn't cleared if the mouse isn't over the button anymore. But maybe that question is out of scope here :P

Lukas Tönne added 2 commits 2023-09-21 11:54:52 +02:00

fff3bd3f4c Merge branch 'main' into fix-panel-button-sticky-state

970de047bd Moved the mouse move hack to the operator instead of drawing code.

Julian Eisel commented

2023-09-21 11:57:26 +02:00

Agreed with Hans here, this shouldn't be done in drawing. If it is really necessary it should be done by the handling, ie the operator in this case.

Julian Eisel approved these changes 2023-09-21 12:03:34 +02:00

Lukas Tönne merged commit fdf1040532 into main

2023-09-21 12:05:15 +02:00

Lukas Tönne deleted branch fix-panel-button-sticky-state

2023-09-21 12:05:17 +02:00

Lukas Tönne referenced this issue from a commit