Python: Event.mouse_prev_x/y broken #48173

New Issue

Jacques Lucke · 2016-04-17T01:20:33+02:00

Jacques Lucke commented

2016-04-17 01:20:33 +02:00

System Information
Windows 10 64 bit
Nvidia Quadro 600

Blender Version
Broken: 9c916b0

Short description of error
The event.mouse_prev_x and event.mouse_prev_y properties are wrong in the modal function when I press the a mouse button.

Exact steps for others to reproduce the error

Open Blender and the console.
Paste the code into the text editor.
Execute it.

Now you should see the mouse coordinates in the console. The problem appears when I press the a mouse button. I don't know why but it does print the start location of the mouse then.

import bpy

class TestMousePosition(bpy.types.Operator):
    bl_idname = "test.test_mouse_position"
    bl_label = "Test Mouse Position"
    bl_description = ""
    bl_options = {"REGISTER"}

    @classmethod
    def poll(cls, context):
        return True

    def invoke(self, context, event):
        context.window_manager.modal_handler_add(self)
        return {"RUNNING_MODAL"}

    def modal(self, context, event):
        print(event.mouse_prev_x, event.mouse_prev_y)

        if event.type in {"RIGHTMOUSE", "ESC"}:
            return {"CANCELLED"}

        return {"RUNNING_MODAL"}
        
def register():
    bpy.utils.register_module(__name__)
    bpy.ops.test.test_mouse_position("INVOKE_DEFAULT")

def unregister():
    bpy.utils.unregister_module(__name__)

if __name__ == "__main__":
    register()

btw: Can someone tell me what 'prev' in 'mouse_prev_x' stands for?

**System Information** Windows 10 64 bit Nvidia Quadro 600 **Blender Version** Broken: 9c916b0 **Short description of error** The event.mouse_prev_x and event.mouse_prev_y properties are wrong in the modal function when I press the a mouse button. **Exact steps for others to reproduce the error** 1. Open Blender and the console. 2. Paste the code into the text editor. 3. Execute it. Now you should see the mouse coordinates in the console. The problem appears when I press the a mouse button. I don't know why but it does print the start location of the mouse then. ``` import bpy class TestMousePosition(bpy.types.Operator): bl_idname = "test.test_mouse_position" bl_label = "Test Mouse Position" bl_description = "" bl_options = {"REGISTER"} @classmethod def poll(cls, context): return True def invoke(self, context, event): context.window_manager.modal_handler_add(self) return {"RUNNING_MODAL"} def modal(self, context, event): print(event.mouse_prev_x, event.mouse_prev_y) if event.type in {"RIGHTMOUSE", "ESC"}: return {"CANCELLED"} return {"RUNNING_MODAL"} def register(): bpy.utils.register_module(__name__) bpy.ops.test.test_mouse_position("INVOKE_DEFAULT") def unregister(): bpy.utils.unregister_module(__name__) if __name__ == "__main__": register() ``` ![bug console.PNG](https://archive.blender.org/developer/F304044/bug_console.PNG) btw: Can someone tell me what 'prev' in 'mouse_prev_x' stands for?

Jacques Lucke commented

2016-04-17 01:20:33 +02:00

Changed status to: 'Open'

Jacques Lucke commented

2016-04-17 01:20:33 +02:00

Added subscriber: @JacquesLucke

Philipp Oeser commented

2016-04-17 22:28:15 +02:00

Added subscriber: @lichtwerk

Philipp Oeser commented

2016-04-17 22:28:15 +02:00

can confirm this.
P350 is a 'stab in the dark' diff that makes this better [even though this may horribly break elsewhere -- not very familiar with the event system :/]

can confirm this. [P350](https://archive.blender.org/developer/P350.txt) is a 'stab in the dark' diff that makes this better [even though this may horribly break elsewhere -- not very familiar with the event system :/]

Campbell Barton commented

2016-04-17 23:11:06 +02:00

Added subscriber: @ideasman42

Campbell Barton commented

2016-04-17 23:11:06 +02:00

@lichtwerk, checked on this and it fixes the case for mouse clicks.
but this doesn't work for keyboard presses.
Thinking this could be done in a more generic way, otherwise its quite precarious where we copy previous values when handling every event.

@lichtwerk, checked on this and it fixes the case for mouse clicks. but this doesn't work for keyboard presses. Thinking this could be done in a more generic way, otherwise its quite precarious where we copy previous values when handling every event.

Brecht Van Lommel commented

2016-04-17 23:33:18 +02:00

Added subscriber: @brecht

Brecht Van Lommel commented

2016-04-17 23:33:18 +02:00

The issue is this hack in wm_event_do_handlers from d27c9f9d7:

/* XXX hrmf, this gives reliable previous mouse coord for area change, feels bad?
 * doing it on ghost queue gives errors when mousemoves go over area borders */
if (doit && win->screen->subwinactive != win->screen->mainwin) {
    win->eventstate->prevx = event->x;
    win->eventstate->prevy = event->y;

Originally this previous mouse coordinate was used for one specific purpose, which is passing mousemove events on to handlers and operators in areas that the mouse is leaving. That's how prevx is used in wm_event_inside_i. To make that case work specific exceptions were added.

But then these coordinates got exposed to Python and now you'd expect it to be generally useful previous mouse coordinates. For that these previous coordinates should be updated whenever an event is handled, regardless if a modal handler is running, if the mouse is over area borders, etc. Ideally this code could be cleaned up and all these exceptions removed. But doing that might break some handlers or operators, it would need to be carefully tested.

The issue is this hack in `wm_event_do_handlers` from d27c9f9d7: ``` /* XXX hrmf, this gives reliable previous mouse coord for area change, feels bad? * doing it on ghost queue gives errors when mousemoves go over area borders */ if (doit && win->screen->subwinactive != win->screen->mainwin) { win->eventstate->prevx = event->x; win->eventstate->prevy = event->y; ``` Originally this previous mouse coordinate was used for one specific purpose, which is passing mousemove events on to handlers and operators in areas that the mouse is leaving. That's how `prevx` is used in `wm_event_inside_i`. To make that case work specific exceptions were added. But then these coordinates got exposed to Python and now you'd expect it to be generally useful previous mouse coordinates. For that these previous coordinates should be updated whenever an event is handled, regardless if a modal handler is running, if the mouse is over area borders, etc. Ideally this code could be cleaned up and all these exceptions removed. But doing that might break some handlers or operators, it would need to be carefully tested.

Julian Eisel commented

2016-04-17 23:43:41 +02:00

Added subscriber: @JulianEisel

Julian Eisel commented

2016-04-17 23:43:42 +02:00

Another case where we don't sync a value with win->eventstate correctly (missing copy to wmEvent *evt in wm_event_add_ghostevent). This should be a proper fix:

diff --git a/source/blender/windowmanager/intern/wm_event_system.c b/source/blender/windowmanager/intern/wm_event_system.c
index 50a0320..9aad0b8 100644
--- a/source/blender/windowmanager/intern/wm_event_system.c
+++ b/source/blender/windowmanager/intern/wm_event_system.c
@@ -3191,6 +3191,7 @@ void wm_event_add_ghostevent(wmWindowManager *wm, wmWindow *win, int type, int U
                        event.type = MOUSEMOVE;
                        wm_event_add_mousemove(win, &event);
                        copy_v2_v2_int(&evt->x, &event.x);
+                       copy_v2_v2_int(&evt->prevx, &event.x);
 
                        /* also add to other window if event is there, this makes overdraws disappear nicely */
                        /* it remaps mousecoord to other window in event */
@@ -3204,6 +3205,7 @@ void wm_event_add_ghostevent(wmWindowManager *wm, wmWindow *win, int type, int U
                                oevent.type = MOUSEMOVE;
                                wm_event_add_mousemove(owin, &oevent);
                                copy_v2_v2_int(&oevt->x, &oevent.x);
+                               copy_v2_v2_int(&oevt->prevx, &oevent.x);
                        }
 
                        break;

Another case where we don't sync a value with `win->eventstate` correctly (missing copy to `wmEvent *evt` in `wm_event_add_ghostevent`). This should be a proper fix: ``` diff --git a/source/blender/windowmanager/intern/wm_event_system.c b/source/blender/windowmanager/intern/wm_event_system.c index 50a0320..9aad0b8 100644 --- a/source/blender/windowmanager/intern/wm_event_system.c +++ b/source/blender/windowmanager/intern/wm_event_system.c @@ -3191,6 +3191,7 @@ void wm_event_add_ghostevent(wmWindowManager *wm, wmWindow *win, int type, int U event.type = MOUSEMOVE; wm_event_add_mousemove(win, &event); copy_v2_v2_int(&evt->x, &event.x); + copy_v2_v2_int(&evt->prevx, &event.x); /* also add to other window if event is there, this makes overdraws disappear nicely */ /* it remaps mousecoord to other window in event */ @@ -3204,6 +3205,7 @@ void wm_event_add_ghostevent(wmWindowManager *wm, wmWindow *win, int type, int U oevent.type = MOUSEMOVE; wm_event_add_mousemove(owin, &oevent); copy_v2_v2_int(&oevt->x, &oevent.x); + copy_v2_v2_int(&oevt->prevx, &oevent.x); } break; ```

Julian Eisel commented

2016-04-17 23:46:12 +02:00

Note that the issue wasn't visible because the hack @brecht mentioned usually runs on most mouse moves. It doesn't run if a handler returns WM_HANDLER_BREAK, which is the default return value for modal operators.

Note that the issue wasn't visible because the hack @brecht mentioned usually runs on most mouse moves. It doesn't run if a handler returns `WM_HANDLER_BREAK`, which is the default return value for modal operators.

Brecht Van Lommel commented

2016-04-17 23:59:35 +02:00

I didn't test any of these patches, but note that operators probably expect prevx to be the mouse coordinates of the previous MOUSEMOVE event that was actually handled.

When ghost events come in quicker than the operators can handle, the window manager converts events to type INBETWEEN_MOUSEMOVE which most operators will ignore. So prevx would be expected to be the delta to the previous MOUSEMOVE event even if there were more ghost mouse move events in between.

I didn't test any of these patches, but note that operators probably expect `prevx` to be the mouse coordinates of the previous `MOUSEMOVE` event that was actually handled. When ghost events come in quicker than the operators can handle, the window manager converts events to type `INBETWEEN_MOUSEMOVE` which most operators will ignore. So `prevx` would be expected to be the delta to the previous `MOUSEMOVE` event even if there were more ghost mouse move events in between.

Julian Eisel commented

2016-04-18 00:19:27 +02:00

Previous patch was wrong, didn't check carefully enough. This should be better:

diff --git a/source/blender/windowmanager/intern/wm_event_system.c b/source/blender/windowmanager/intern/wm_event_system.c
index 50a0320..db1f6af 100644
--- a/source/blender/windowmanager/intern/wm_event_system.c
+++ b/source/blender/windowmanager/intern/wm_event_system.c
@@ -3188,6 +3188,7 @@ void wm_event_add_ghostevent(wmWindowManager *wm, wmWindow *win, int type, int U
                        GHOST_TEventCursorData *cd = customdata;
 
                        copy_v2_v2_int(&event.x, &cd->x);
+                       copy_v2_v2_int(&evt->prevx, &evt->x);
                        event.type = MOUSEMOVE;
                        wm_event_add_mousemove(win, &event);
                        copy_v2_v2_int(&evt->x, &event.x);

Didn't check on treating INBETWEEN_MOUSEMOVE nicely, but will try to do tomorrow.

Previous patch was wrong, didn't check carefully enough. This should be better: ``` diff --git a/source/blender/windowmanager/intern/wm_event_system.c b/source/blender/windowmanager/intern/wm_event_system.c index 50a0320..db1f6af 100644 --- a/source/blender/windowmanager/intern/wm_event_system.c +++ b/source/blender/windowmanager/intern/wm_event_system.c @@ -3188,6 +3188,7 @@ void wm_event_add_ghostevent(wmWindowManager *wm, wmWindow *win, int type, int U GHOST_TEventCursorData *cd = customdata; copy_v2_v2_int(&event.x, &cd->x); + copy_v2_v2_int(&evt->prevx, &evt->x); event.type = MOUSEMOVE; wm_event_add_mousemove(win, &event); copy_v2_v2_int(&evt->x, &event.x); ``` Didn't check on treating `INBETWEEN_MOUSEMOVE` nicely, but will try to do tomorrow.

Jacques Lucke commented

2016-04-18 00:33:41 +02:00

I wonder how many operators actually use this property?
I mean it would be easy to store the previous mouse position in the operator itself, there is no need for this to be in the Event.

If it will stay a property (for compability) the documentation should give some more information about it. At least it should say in which last event type the mouse position has been recorded (eg MOUSEMOVE).

I wonder how many operators actually use this property? I mean it would be easy to store the previous mouse position in the operator itself, there is no need for this to be in the Event. If it will stay a property (for compability) the documentation should give some more information about it. At least it should say in which last event type the mouse position has been recorded (eg `MOUSEMOVE`).

blender-admin commented

2016-04-23 03:05:48 +02:00

This issue was referenced by ab3c1de56d

This issue was referenced by ab3c1de56d279bf3c97203e21875cf9a9086842f

Brecht Van Lommel commented

2016-04-23 03:15:18 +02:00

Changed status from 'Open' to: 'Resolved'

Brecht Van Lommel closed this issue

2016-04-23 03:15:18 +02:00

Brecht Van Lommel commented

2016-04-23 03:15:30 +02:00

I looked into the history of this code further, and couldn't find a good reason to have this hack. This was added early in 2.5 development, and the event handling system changed a lot since then. So I just removed it and simplified setting of prevx and prevy.

I looked into the history of this code further, and couldn't find a good reason to have this hack. This was added early in 2.5 development, and the event handling system changed a lot since then. So I just removed it and simplified setting of `prevx` and `prevy`.

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

Python: Event.mouse_prev_x/y broken #48173