GPencil: Fast clicks over Dopesheet left icons active layer renaming #76481

New Issue

Lukas Sabaliauskas · 2020-05-06T18:49:22+02:00

2020-05-06 18:49:22 +02:00

System Information
Operating system: Windows 10 Famille 64-bit (10.0, Build 18363) (18362.19h1_release.190318-1202)
Graphics card: NVIDIA GeForce GTX 1050

Blender Version
Broken: 2.82

Short description of error
Double clicking on a grease pencil layer's options icons twice triggers the renaming of that layer.

Exact steps for others to reproduce the error
Based on the default startup, with any grease pencil object with a layer. The problem occurs in the Dope Sheet editor panel, under the grease pencil drop down menu, in the left pannel where the layers are at. Double clicking on any of a layer's options icons (Mask, Onion Skin, Eye, Lock) twice triggers the renaming of that layer as if I was clicking on the name of the layer. This window resembles a lot the Object Data Properties, under the Layer dropout, but here I can quickly switch on and off any of the icons.

{F8516514, size=full}

Thanks !

**System Information** Operating system: Windows 10 Famille 64-bit (10.0, Build 18363) (18362.19h1_release.190318-1202) Graphics card: NVIDIA GeForce GTX 1050 **Blender Version** Broken: 2.82 **Short description of error** Double clicking on a grease pencil layer's options icons twice triggers the renaming of that layer. **Exact steps for others to reproduce the error** Based on the default startup, with any grease pencil object with a layer. The problem occurs in the Dope Sheet editor panel, under the grease pencil drop down menu, in the left pannel where the layers are at. Double clicking on any of a layer's options icons (Mask, Onion Skin, Eye, Lock) twice triggers the renaming of that layer as if I was clicking on the name of the layer. This window resembles a lot the Object Data Properties, under the Layer dropout, but here I can quickly switch on and off any of the icons. {[F8516514](https://archive.blender.org/developer/F8516514/image.png), size=full} Thanks !

Lukas Sabaliauskas commented

2020-05-06 18:49:22 +02:00

Added subscriber: @Trukas

Antonio Vazquez commented

2020-05-06 19:38:27 +02:00

Added subscribers: @JulianEisel, @antoniov

Antonio Vazquez commented

2020-05-06 19:38:27 +02:00

Changed status from 'Needs Triage' to: 'Confirmed'

Antonio Vazquez commented

2020-05-06 19:38:27 +02:00

@JulianEisel do you think is possible to reduce the sensitivity here to fast clicks?

Antonio Vazquez changed title from ~~Grease Pencil layer renaming~~ to GPencil: Fast clicks over Dopesheet left icons active layer renaming

2020-05-06 19:39:03 +02:00

Hans Goudey commented

2020-05-06 20:42:04 +02:00

Added subscriber: @HooglyBoogly

Hans Goudey commented

2020-05-06 20:42:04 +02:00

IMO the proper solution is for clicks on buttons to swallow the first click of a double-click event. The double click timer shouldn't need to be adjusted per region.

Another solution would be to change NLACHANNEL_NAMEWIDTH in animchannels_channel_get to subtract the width of the buttons on the right from the size of the channel list region. That way the double-click event would only find a channel and execute if wasn't on a button.
That second solution is pretty simple, it just loses some generality because the number of buttons changes in different contexts.

IMO the proper solution is for clicks on buttons to swallow the first click of a double-click event. The double click timer shouldn't need to be adjusted per region. Another solution would be to change `NLACHANNEL_NAMEWIDTH` in `animchannels_channel_get` to subtract the width of the buttons on the right from the size of the channel list region. That way the double-click event would only find a channel and execute if wasn't on a button. That second solution is pretty simple, it just loses some generality because the number of buttons changes in different contexts.

Antonio Vazquez commented

2020-05-06 21:46:20 +02:00

@HooglyBoogly Can you manage it? I don't know the UI code too much.

Hans Goudey commented

2020-05-12 20:48:00 +02:00

Added subscriber: @ideasman42

Hans Goudey commented

2020-05-12 20:48:00 +02:00

I looked into this issue for a while. I was trying to change is the fact that the first click event which toggles the button is also the first click of a double-click event.

P1387 My idea here was to mark "handled" events so they don't act as the first event of a double event. This works, but the problem is that handlers that aren't from buttons can return "handled == true," meaning that double-click doesn't work at all in the dope-sheet list.

Building the interface to get rid of the overlapping buttons is probably the right approach.

@ideasman42 Disregarding the implementation in the paste, do you agree that trying to not use handled events as the first of a double event is not the right approach?

I looked into this issue for a while. I was trying to change is the fact that the first click event which toggles the button is also the first click of a double-click event. [P1387](https://archive.blender.org/developer/P1387.txt) My idea here was to mark "handled" events so they don't act as the first event of a double event. This works, but the problem is that handlers that aren't from buttons can return "handled == true," meaning that double-click doesn't work at all in the dope-sheet list. Building the interface to get rid of the overlapping buttons is probably the right approach. @ideasman42 Disregarding the implementation in the paste, do you agree that trying to not use handled events as the first of a double event is not the right approach?

Julian Eisel commented

2020-05-13 02:29:40 +02:00

The issues you're facing with that approach are just what I'd have expected, although I think it was worth playing with. The way this is done is pretty old, and I'd expect quite some cases to depend on the exact event system behavior. That's why changes in the event system are always tricky...

I'd propose fixing this by letting the toggle icons respect double-click events, so letting them swallow the double click before it get a chance to be passed to the renaming operator. This works well for me:

diff --git a/source/blender/editors/interface/interface_handlers.c b/source/blender/editors/interface/interface_handlers.c
index ead36fdbd19..ebef83ab688 100644
--- a/source/blender/editors/interface/interface_handlers.c
+++ b/source/blender/editors/interface/interface_handlers.c
@@ -4443,7 +4443,7 @@ static int ui_do_but_TOG(bContext *C, uiBut *but, uiHandleButtonData *data, cons
         do_activate = (event->val == KM_RELEASE);
       }
       else {
-        do_activate = (event->val == KM_PRESS);
+        do_activate = ELEM(event->val, KM_PRESS, KM_DBL_CLICK);
       }
     }
 
@@ -8871,7 +8871,7 @@ static int ui_handle_button_event(bContext *C, const wmEvent *event, uiBut *but)
        * This is needed to make sure if a button was active,
        * it stays active while the mouse is over it.
        * This avoids adding mousemoves, see: [#33466] */
-      if (ELEM(state_orig, BUTTON_STATE_INIT, BUTTON_STATE_HIGHLIGHT)) {
+      if (ELEM(state_orig, BUTTON_STATE_INIT, BUTTON_STATE_HIGHLIGHT, BUTTON_STATE_WAIT_DRAG)) {
         if (ui_but_find_mouse_over(region, event) == but) {
           button_activate_init(C, region, but, BUTTON_ACTIVATE_OVER);
         }

(The second change is needed to detect the hovered button correctly. Without it appeared to randomly work or not with a touchpad.)

The issues you're facing with that approach are just what I'd have expected, although I think it was worth playing with. The way this is done is pretty old, and I'd expect quite some cases to depend on the exact event system behavior. That's why changes in the event system are always tricky... I'd propose fixing this by letting the toggle icons respect double-click events, so letting them swallow the double click before it get a chance to be passed to the renaming operator. This works well for me: ```lines=10 diff --git a/source/blender/editors/interface/interface_handlers.c b/source/blender/editors/interface/interface_handlers.c index ead36fdbd19..ebef83ab688 100644 --- a/source/blender/editors/interface/interface_handlers.c +++ b/source/blender/editors/interface/interface_handlers.c @@ -4443,7 +4443,7 @@ static int ui_do_but_TOG(bContext *C, uiBut *but, uiHandleButtonData *data, cons do_activate = (event->val == KM_RELEASE); } else { - do_activate = (event->val == KM_PRESS); + do_activate = ELEM(event->val, KM_PRESS, KM_DBL_CLICK); } } @@ -8871,7 +8871,7 @@ static int ui_handle_button_event(bContext *C, const wmEvent *event, uiBut *but) * This is needed to make sure if a button was active, * it stays active while the mouse is over it. * This avoids adding mousemoves, see: [#33466] */ - if (ELEM(state_orig, BUTTON_STATE_INIT, BUTTON_STATE_HIGHLIGHT)) { + if (ELEM(state_orig, BUTTON_STATE_INIT, BUTTON_STATE_HIGHLIGHT, BUTTON_STATE_WAIT_DRAG)) { if (ui_but_find_mouse_over(region, event) == but) { button_activate_init(C, region, but, BUTTON_ACTIVATE_OVER); } ``` (The second change is needed to detect the hovered button correctly. Without it appeared to randomly work or not with a touchpad.)

Hans Goudey commented

2020-05-13 03:10:21 +02:00

In #76481#930350, @JulianEisel wrote:
The issues you're facing with that approach are just what I'd have expected, although I think it was worth playing with. The way this is done is pretty old, and I'd expect quite some cases to depend on the exact event system behavior. That's why changes in the event system are always tricky...

Yeah, it would have been nice if it worked out. I learned a fair bit anyway. Although the two variables "evt" and "event" are a bit cruel :P

I'd propose fixing this by letting the toggle icons respect double-click events, so letting them swallow the double click before it get a chance to be passed to the renaming operator.

Oh, clever, I didn't think of that. I guess it's not the "right" way but I don't see anything wrong with it!

> In #76481#930350, @JulianEisel wrote: > The issues you're facing with that approach are just what I'd have expected, although I think it was worth playing with. The way this is done is pretty old, and I'd expect quite some cases to depend on the exact event system behavior. That's why changes in the event system are always tricky... Yeah, it would have been nice if it worked out. I learned a fair bit anyway. Although the two variables "evt" and "event" are a bit cruel :P > I'd propose fixing this by letting the toggle icons respect double-click events, so letting them swallow the double click before it get a chance to be passed to the renaming operator. Oh, clever, I didn't think of that. I guess it's not the "right" way but I don't see anything wrong with it!

blender-admin commented

2020-05-13 11:45:28 +02:00

This issue was referenced by 099dd0690c

This issue was referenced by 099dd0690ced05c38819861b3dca84e03ccdb3c1

Julian Eisel commented

2020-05-13 11:52:07 +02:00

Changed status from 'Confirmed' to: 'Resolved'

Julian Eisel closed this issue

2020-05-13 11:52:07 +02:00

Julian Eisel self-assigned this 2020-05-13 11:52:07 +02:00

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

GPencil: Fast clicks over Dopesheet left icons active layer renaming #76481