Signed-off-by: Huw Davies huw@codeweavers.com --- server/protocol.def | 9 ++++++- server/queue.c | 62 ++++++++++++++++++++++----------------------- server/user.h | 3 --- 3 files changed, 39 insertions(+), 35 deletions(-)
diff --git a/server/protocol.def b/server/protocol.def index c1c33d7e3d1..d61d7f3c549 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -796,9 +796,16 @@ typedef struct lparam_t info; } cursor_pos_t;
+struct shared_cursor +{ + int x; /* cursor position */ + int y; + unsigned int last_change; /* time of last position change */ +}; + struct desktop_shared_memory { - int placeholder; + struct shared_cursor cursor; /* global cursor information */ };
/****************************************************************/ diff --git a/server/queue.c b/server/queue.c index 935475b4c09..f6664509ee3 100644 --- a/server/queue.c +++ b/server/queue.c @@ -377,10 +377,10 @@ static int update_desktop_cursor_pos( struct desktop *desktop, int x, int y )
x = max( min( x, desktop->cursor.clip.right - 1 ), desktop->cursor.clip.left ); y = max( min( y, desktop->cursor.clip.bottom - 1 ), desktop->cursor.clip.top ); - updated = (desktop->cursor.x != x || desktop->cursor.y != y); - desktop->cursor.x = x; - desktop->cursor.y = y; - desktop->cursor.last_change = get_tick_count(); + updated = (desktop->shared->cursor.x != x || desktop->shared->cursor.y != y); + desktop->shared->cursor.x = x; + desktop->shared->cursor.y = y; + desktop->shared->cursor.last_change = get_tick_count();
return updated; } @@ -411,8 +411,8 @@ static void get_message_defaults( struct msg_queue *queue, int *x, int *y, unsig { struct desktop *desktop = queue->input->desktop;
- *x = desktop->cursor.x; - *y = desktop->cursor.y; + *x = desktop->shared->cursor.x; + *y = desktop->shared->cursor.y; *time = get_tick_count(); }
@@ -439,9 +439,9 @@ static void set_clip_rectangle( struct desktop *desktop, const rectangle_t *rect post_desktop_message( desktop, desktop->cursor.clip_msg, rect != NULL, 0 );
/* warp the mouse to be inside the clip rect */ - x = max( min( desktop->cursor.x, desktop->cursor.clip.right - 1 ), desktop->cursor.clip.left ); - y = max( min( desktop->cursor.y, desktop->cursor.clip.bottom - 1 ), desktop->cursor.clip.top ); - if (x != desktop->cursor.x || y != desktop->cursor.y) set_cursor_pos( desktop, x, y ); + x = max( min( desktop->shared->cursor.x, desktop->cursor.clip.right - 1 ), desktop->cursor.clip.left ); + y = max( min( desktop->shared->cursor.y, desktop->cursor.clip.bottom - 1 ), desktop->cursor.clip.top ); + if (x != desktop->shared->cursor.x || y != desktop->shared->cursor.y) set_cursor_pos( desktop, x, y ); }
/* change the foreground input and reset the cursor clip rect */ @@ -1568,8 +1568,8 @@ static void queue_hardware_message( struct desktop *desktop, struct message *msg if (desktop->keystate[VK_XBUTTON1] & 0x80) msg->wparam |= MK_XBUTTON1; if (desktop->keystate[VK_XBUTTON2] & 0x80) msg->wparam |= MK_XBUTTON2; } - msg->x = desktop->cursor.x; - msg->y = desktop->cursor.y; + msg->x = desktop->shared->cursor.x; + msg->y = desktop->shared->cursor.y;
if (msg->win && (thread = get_window_thread( msg->win ))) { @@ -1737,10 +1737,10 @@ static int queue_mouse_message( struct desktop *desktop, user_handle_t win, cons WM_MOUSEHWHEEL /* 0x1000 = MOUSEEVENTF_HWHEEL */ };
- update_desktop_cursor_pos( desktop, desktop->cursor.x, desktop->cursor.y ); /* Update last change time */ + update_desktop_cursor_pos( desktop, desktop->shared->cursor.x, desktop->shared->cursor.y ); /* Update last change time */ flags = input->mouse.flags; time = input->mouse.time; - if (!time) time = desktop->cursor.last_change; + if (!time) time = desktop->shared->cursor.last_change;
if (flags & MOUSEEVENTF_MOVE) { @@ -1749,19 +1749,19 @@ static int queue_mouse_message( struct desktop *desktop, user_handle_t win, cons x = input->mouse.x; y = input->mouse.y; if (flags & ~(MOUSEEVENTF_MOVE | MOUSEEVENTF_ABSOLUTE) && - x == desktop->cursor.x && y == desktop->cursor.y) + x == desktop->shared->cursor.x && y == desktop->shared->cursor.y) flags &= ~MOUSEEVENTF_MOVE; } else { - x = desktop->cursor.x + input->mouse.x; - y = desktop->cursor.y + input->mouse.y; + x = desktop->shared->cursor.x + input->mouse.x; + y = desktop->shared->cursor.y + input->mouse.y; } } else { - x = desktop->cursor.x; - y = desktop->cursor.y; + x = desktop->shared->cursor.x; + y = desktop->shared->cursor.y; }
if ((foreground = get_foreground_thread( desktop, win ))) @@ -1775,8 +1775,8 @@ static int queue_mouse_message( struct desktop *desktop, user_handle_t win, cons msg_data->info = input->mouse.info; msg_data->flags = flags; msg_data->rawinput.type = RIM_TYPEMOUSE; - msg_data->rawinput.mouse.x = x - desktop->cursor.x; - msg_data->rawinput.mouse.y = y - desktop->cursor.y; + msg_data->rawinput.mouse.x = x - desktop->shared->cursor.x; + msg_data->rawinput.mouse.y = y - desktop->shared->cursor.y; msg_data->rawinput.mouse.data = input->mouse.data;
enum_processes( queue_rawinput_message, &raw_msg ); @@ -1967,8 +1967,8 @@ static void queue_custom_hardware_message( struct desktop *desktop, user_handle_ msg->msg = input->hw.msg; msg->wparam = 0; msg->lparam = input->hw.lparam; - msg->x = desktop->cursor.x; - msg->y = desktop->cursor.y; + msg->x = desktop->shared->cursor.x; + msg->y = desktop->shared->cursor.y;
queue_hardware_message( desktop, msg, 1 ); } @@ -2473,8 +2473,8 @@ DECL_HANDLER(send_hardware_message) } }
- reply->prev_x = desktop->cursor.x; - reply->prev_y = desktop->cursor.y; + reply->prev_x = desktop->shared->cursor.x; + reply->prev_y = desktop->shared->cursor.y;
switch (req->input.type) { @@ -2492,8 +2492,8 @@ DECL_HANDLER(send_hardware_message) } if (thread) release_object( thread );
- reply->new_x = desktop->cursor.x; - reply->new_y = desktop->cursor.y; + reply->new_x = desktop->shared->cursor.x; + reply->new_y = desktop->shared->cursor.y; set_reply_data( desktop->keystate, size ); release_object( desktop ); } @@ -3202,8 +3202,8 @@ DECL_HANDLER(set_cursor)
reply->prev_handle = input->cursor; reply->prev_count = input->cursor_count; - reply->prev_x = input->desktop->cursor.x; - reply->prev_y = input->desktop->cursor.y; + reply->prev_x = input->desktop->shared->cursor.x; + reply->prev_y = input->desktop->shared->cursor.y;
if (req->flags & SET_CURSOR_HANDLE) { @@ -3234,10 +3234,10 @@ DECL_HANDLER(set_cursor) set_clip_rectangle( desktop, (req->flags & SET_CURSOR_NOCLIP) ? NULL : &req->clip, 0 ); }
- reply->new_x = input->desktop->cursor.x; - reply->new_y = input->desktop->cursor.y; + reply->new_x = input->desktop->shared->cursor.x; + reply->new_y = input->desktop->shared->cursor.y; reply->new_clip = input->desktop->cursor.clip; - reply->last_change = input->desktop->cursor.last_change; + reply->last_change = input->desktop->shared->cursor.last_change; }
/* Get the history of the 64 last cursor positions */ diff --git a/server/user.h b/server/user.h index 9a28ba7f449..e525957c5bc 100644 --- a/server/user.h +++ b/server/user.h @@ -54,11 +54,8 @@ struct winstation
struct global_cursor { - int x; /* cursor position */ - int y; rectangle_t clip; /* cursor clip rectangle */ unsigned int clip_msg; /* message to post for cursor clip changes */ - unsigned int last_change; /* time of last position change */ user_handle_t win; /* window that contains the cursor */ };
On 11/19/20 2:09 PM, Huw Davies wrote:
Signed-off-by: Huw Davies huw@codeweavers.com
server/protocol.def | 9 ++++++- server/queue.c | 62 ++++++++++++++++++++++----------------------- server/user.h | 3 --- 3 files changed, 39 insertions(+), 35 deletions(-)
diff --git a/server/protocol.def b/server/protocol.def index c1c33d7e3d1..d61d7f3c549 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -796,9 +796,16 @@ typedef struct lparam_t info; } cursor_pos_t;
+struct shared_cursor +{
- int x; /* cursor position */
- int y;
- unsigned int last_change; /* time of last position change */
+};
- struct desktop_shared_memory {
- int placeholder;
struct shared_cursor cursor; /* global cursor information */ };
/****************************************************************/
diff --git a/server/queue.c b/server/queue.c index 935475b4c09..f6664509ee3 100644 --- a/server/queue.c +++ b/server/queue.c @@ -377,10 +377,10 @@ static int update_desktop_cursor_pos( struct desktop *desktop, int x, int y )
x = max( min( x, desktop->cursor.clip.right - 1 ), desktop->cursor.clip.left ); y = max( min( y, desktop->cursor.clip.bottom - 1 ), desktop->cursor.clip.top );
- updated = (desktop->cursor.x != x || desktop->cursor.y != y);
- desktop->cursor.x = x;
- desktop->cursor.y = y;
- desktop->cursor.last_change = get_tick_count();
updated = (desktop->shared->cursor.x != x || desktop->shared->cursor.y != y);
desktop->shared->cursor.x = x;
desktop->shared->cursor.y = y;
desktop->shared->cursor.last_change = get_tick_count();
return updated; }
@@ -411,8 +411,8 @@ static void get_message_defaults( struct msg_queue *queue, int *x, int *y, unsig { struct desktop *desktop = queue->input->desktop;
- *x = desktop->cursor.x;
- *y = desktop->cursor.y;
- *x = desktop->shared->cursor.x;
- *y = desktop->shared->cursor.y; *time = get_tick_count(); }
@@ -439,9 +439,9 @@ static void set_clip_rectangle( struct desktop *desktop, const rectangle_t *rect post_desktop_message( desktop, desktop->cursor.clip_msg, rect != NULL, 0 );
/* warp the mouse to be inside the clip rect */
- x = max( min( desktop->cursor.x, desktop->cursor.clip.right - 1 ), desktop->cursor.clip.left );
- y = max( min( desktop->cursor.y, desktop->cursor.clip.bottom - 1 ), desktop->cursor.clip.top );
- if (x != desktop->cursor.x || y != desktop->cursor.y) set_cursor_pos( desktop, x, y );
x = max( min( desktop->shared->cursor.x, desktop->cursor.clip.right - 1 ), desktop->cursor.clip.left );
y = max( min( desktop->shared->cursor.y, desktop->cursor.clip.bottom - 1 ), desktop->cursor.clip.top );
if (x != desktop->shared->cursor.x || y != desktop->shared->cursor.y) set_cursor_pos( desktop, x, y ); }
/* change the foreground input and reset the cursor clip rect */
@@ -1568,8 +1568,8 @@ static void queue_hardware_message( struct desktop *desktop, struct message *msg if (desktop->keystate[VK_XBUTTON1] & 0x80) msg->wparam |= MK_XBUTTON1; if (desktop->keystate[VK_XBUTTON2] & 0x80) msg->wparam |= MK_XBUTTON2; }
- msg->x = desktop->cursor.x;
- msg->y = desktop->cursor.y;
msg->x = desktop->shared->cursor.x;
msg->y = desktop->shared->cursor.y;
if (msg->win && (thread = get_window_thread( msg->win ))) {
@@ -1737,10 +1737,10 @@ static int queue_mouse_message( struct desktop *desktop, user_handle_t win, cons WM_MOUSEHWHEEL /* 0x1000 = MOUSEEVENTF_HWHEEL */ };
- update_desktop_cursor_pos( desktop, desktop->cursor.x, desktop->cursor.y ); /* Update last change time */
- update_desktop_cursor_pos( desktop, desktop->shared->cursor.x, desktop->shared->cursor.y ); /* Update last change time */ flags = input->mouse.flags; time = input->mouse.time;
- if (!time) time = desktop->cursor.last_change;
if (!time) time = desktop->shared->cursor.last_change;
if (flags & MOUSEEVENTF_MOVE) {
@@ -1749,19 +1749,19 @@ static int queue_mouse_message( struct desktop *desktop, user_handle_t win, cons x = input->mouse.x; y = input->mouse.y; if (flags & ~(MOUSEEVENTF_MOVE | MOUSEEVENTF_ABSOLUTE) &&
x == desktop->cursor.x && y == desktop->cursor.y)
x == desktop->shared->cursor.x && y == desktop->shared->cursor.y) flags &= ~MOUSEEVENTF_MOVE; } else {
x = desktop->cursor.x + input->mouse.x;y = desktop->cursor.y + input->mouse.y;
x = desktop->shared->cursor.x + input->mouse.x;y = desktop->shared->cursor.y + input->mouse.y; } } else {
x = desktop->cursor.x;y = desktop->cursor.y;
x = desktop->shared->cursor.x;y = desktop->shared->cursor.y; } if ((foreground = get_foreground_thread( desktop, win )))@@ -1775,8 +1775,8 @@ static int queue_mouse_message( struct desktop *desktop, user_handle_t win, cons msg_data->info = input->mouse.info; msg_data->flags = flags; msg_data->rawinput.type = RIM_TYPEMOUSE;
msg_data->rawinput.mouse.x = x - desktop->cursor.x;msg_data->rawinput.mouse.y = y - desktop->cursor.y;
msg_data->rawinput.mouse.x = x - desktop->shared->cursor.x;msg_data->rawinput.mouse.y = y - desktop->shared->cursor.y; msg_data->rawinput.mouse.data = input->mouse.data; enum_processes( queue_rawinput_message, &raw_msg );@@ -1967,8 +1967,8 @@ static void queue_custom_hardware_message( struct desktop *desktop, user_handle_ msg->msg = input->hw.msg; msg->wparam = 0; msg->lparam = input->hw.lparam;
- msg->x = desktop->cursor.x;
- msg->y = desktop->cursor.y;
msg->x = desktop->shared->cursor.x;
msg->y = desktop->shared->cursor.y;
queue_hardware_message( desktop, msg, 1 ); }
@@ -2473,8 +2473,8 @@ DECL_HANDLER(send_hardware_message) } }
- reply->prev_x = desktop->cursor.x;
- reply->prev_y = desktop->cursor.y;
reply->prev_x = desktop->shared->cursor.x;
reply->prev_y = desktop->shared->cursor.y;
switch (req->input.type) {
@@ -2492,8 +2492,8 @@ DECL_HANDLER(send_hardware_message) } if (thread) release_object( thread );
- reply->new_x = desktop->cursor.x;
- reply->new_y = desktop->cursor.y;
- reply->new_x = desktop->shared->cursor.x;
- reply->new_y = desktop->shared->cursor.y; set_reply_data( desktop->keystate, size ); release_object( desktop ); }
@@ -3202,8 +3202,8 @@ DECL_HANDLER(set_cursor)
reply->prev_handle = input->cursor; reply->prev_count = input->cursor_count;
- reply->prev_x = input->desktop->cursor.x;
- reply->prev_y = input->desktop->cursor.y;
reply->prev_x = input->desktop->shared->cursor.x;
reply->prev_y = input->desktop->shared->cursor.y;
if (req->flags & SET_CURSOR_HANDLE) {
@@ -3234,10 +3234,10 @@ DECL_HANDLER(set_cursor) set_clip_rectangle( desktop, (req->flags & SET_CURSOR_NOCLIP) ? NULL : &req->clip, 0 ); }
- reply->new_x = input->desktop->cursor.x;
- reply->new_y = input->desktop->cursor.y;
- reply->new_x = input->desktop->shared->cursor.x;
- reply->new_y = input->desktop->shared->cursor.y; reply->new_clip = input->desktop->cursor.clip;
- reply->last_change = input->desktop->cursor.last_change;
reply->last_change = input->desktop->shared->cursor.last_change; }
/* Get the history of the 64 last cursor positions */
diff --git a/server/user.h b/server/user.h index 9a28ba7f449..e525957c5bc 100644 --- a/server/user.h +++ b/server/user.h @@ -54,11 +54,8 @@ struct winstation
struct global_cursor {
- int x; /* cursor position */
- int y; rectangle_t clip; /* cursor clip rectangle */ unsigned int clip_msg; /* message to post for cursor clip changes */
- unsigned int last_change; /* time of last position change */ user_handle_t win; /* window that contains the cursor */ };
Just a quick thought, as the shared desktop data struct is flagged volatile, this will probably prevent optimizations on the server-side reads too, maybe we could avoid that and only make the writes volatile?
On 19 Nov 2020, at 13:42, Rémi Bernon rbernon@codeweavers.com wrote:
On 11/19/20 2:09 PM, Huw Davies wrote:
Signed-off-by: Huw Davies huw@codeweavers.com
server/protocol.def | 9 ++++++- server/queue.c | 62 ++++++++++++++++++++++----------------------- server/user.h | 3 --- 3 files changed, 39 insertions(+), 35 deletions(-)
Just a quick thought, as the shared desktop data struct is flagged volatile, this will probably prevent optimizations on the server-side reads too, maybe we could avoid that and only make the writes volatile?
Hmm, interesting idea. That would most likely involve volatile casts while writing (or macros to hide them), neither of which are particularly appealing. I'd be tempted to wait to see if this becomes a real issue before doing this, but I'm open to being persuaded otherwise.
Huw.
On 11/19/20 3:01 PM, Huw Davies wrote:
On 19 Nov 2020, at 13:42, Rémi Bernon rbernon@codeweavers.com wrote:
On 11/19/20 2:09 PM, Huw Davies wrote:
Signed-off-by: Huw Davies huw@codeweavers.com
server/protocol.def | 9 ++++++- server/queue.c | 62 ++++++++++++++++++++++----------------------- server/user.h | 3 --- 3 files changed, 39 insertions(+), 35 deletions(-)
Just a quick thought, as the shared desktop data struct is flagged volatile, this will probably prevent optimizations on the server-side reads too, maybe we could avoid that and only make the writes volatile?
Hmm, interesting idea. That would most likely involve volatile casts while writing (or macros to hide them), neither of which are particularly appealing. I'd be tempted to wait to see if this becomes a real issue before doing this, but I'm open to being persuaded otherwise.
Huw.
We could also keep the non-shared state like it is and add a separate shared state that gets updated with the non-shared state, in the helper?
On 19 Nov 2020, at 14:12, Rémi Bernon rbernon@codeweavers.com wrote:
On 11/19/20 3:01 PM, Huw Davies wrote:
On 19 Nov 2020, at 13:42, Rémi Bernon rbernon@codeweavers.com wrote:
On 11/19/20 2:09 PM, Huw Davies wrote:
Signed-off-by: Huw Davies huw@codeweavers.com
server/protocol.def | 9 ++++++- server/queue.c | 62 ++++++++++++++++++++++----------------------- server/user.h | 3 --- 3 files changed, 39 insertions(+), 35 deletions(-)
Just a quick thought, as the shared desktop data struct is flagged volatile, this will probably prevent optimizations on the server-side reads too, maybe we could avoid that and only make the writes volatile?
Hmm, interesting idea. That would most likely involve volatile casts while writing (or macros to hide them), neither of which are particularly appealing. I'd be tempted to wait to see if this becomes a real issue before doing this, but I'm open to being persuaded otherwise. Huw.
We could also keep the non-shared state like it is and add a separate shared state that gets updated with the non-shared state, in the helper?
Again that's possible, but also seems fairly ugly. Not least because a future patch will add the desktop keystate to the shared mapping and keeping two copies of that in sync seems wrong.
Huw.
On 11/19/20 3:21 PM, Huw Davies wrote:
On 19 Nov 2020, at 14:12, Rémi Bernon rbernon@codeweavers.com wrote:
On 11/19/20 3:01 PM, Huw Davies wrote:
On 19 Nov 2020, at 13:42, Rémi Bernon rbernon@codeweavers.com wrote:
On 11/19/20 2:09 PM, Huw Davies wrote:
Signed-off-by: Huw Davies huw@codeweavers.com
server/protocol.def | 9 ++++++- server/queue.c | 62 ++++++++++++++++++++++----------------------- server/user.h | 3 --- 3 files changed, 39 insertions(+), 35 deletions(-)
Just a quick thought, as the shared desktop data struct is flagged volatile, this will probably prevent optimizations on the server-side reads too, maybe we could avoid that and only make the writes volatile?
Hmm, interesting idea. That would most likely involve volatile casts while writing (or macros to hide them), neither of which are particularly appealing. I'd be tempted to wait to see if this becomes a real issue before doing this, but I'm open to being persuaded otherwise. Huw.
We could also keep the non-shared state like it is and add a separate shared state that gets updated with the non-shared state, in the helper?
Again that's possible, but also seems fairly ugly. Not least because a future patch will add the desktop keystate to the shared mapping and keeping two copies of that in sync seems wrong.
Huw.
But if we consider other architectures than X86, at least for wineserver itself, and as it's done in set_user_shared_data_time, I don't think updating the sequence is theoretically enough, and we probably should do the updates using atomic intrinsics anyway.
So maybe doing it with the __atomic intrinsics for every arch is easier and adding volatile casts there too wouldn't be much uglier. Or it could also be custom macros that does either volatile or intrinsics depending on the arch, and they could also be used for set_user_shared_data_time.
I also noticed that the client side of the shared data isn't flagged volatile, it should probably be, assuming it's always X86 there.
Cheers,
On 19 Nov 2020, at 14:41, Rémi Bernon rbernon@codeweavers.com wrote:
On 11/19/20 3:21 PM, Huw Davies wrote:
On 19 Nov 2020, at 14:12, Rémi Bernon rbernon@codeweavers.com wrote:
On 11/19/20 3:01 PM, Huw Davies wrote:
On 19 Nov 2020, at 13:42, Rémi Bernon rbernon@codeweavers.com wrote:
On 11/19/20 2:09 PM, Huw Davies wrote:
Signed-off-by: Huw Davies huw@codeweavers.com
server/protocol.def | 9 ++++++- server/queue.c | 62 ++++++++++++++++++++++----------------------- server/user.h | 3 --- 3 files changed, 39 insertions(+), 35 deletions(-)
Just a quick thought, as the shared desktop data struct is flagged volatile, this will probably prevent optimizations on the server-side reads too, maybe we could avoid that and only make the writes volatile?
Hmm, interesting idea. That would most likely involve volatile casts while writing (or macros to hide them), neither of which are particularly appealing. I'd be tempted to wait to see if this becomes a real issue before doing this, but I'm open to being persuaded otherwise. Huw.
We could also keep the non-shared state like it is and add a separate shared state that gets updated with the non-shared state, in the helper?
Again that's possible, but also seems fairly ugly. Not least because a future patch will add the desktop keystate to the shared mapping and keeping two copies of that in sync seems wrong. Huw.
But if we consider other architectures than X86, at least for wineserver itself, and as it's done in set_user_shared_data_time, I don't think updating the sequence is theoretically enough, and we probably should do the updates using atomic intrinsics anyway.
For non-intel archs it using fences before and after the sequence is updated, so the actual data doesn't need protecting.
So maybe doing it with the __atomic intrinsics for every arch is easier and adding volatile casts there too wouldn't be much uglier. Or it could also be custom macros that does either volatile or intrinsics depending on the arch, and they could also be used for set_user_shared_data_time.
We need to support older compilers on intel that don't have the atomic ops.
I also noticed that the client side of the shared data isn't flagged volatile, it should probably be, assuming it's always X86 there.
The client should always use get_desktop_shared_memory() which does return a volatile ptr, so I don't think there's a need to flag the actual ptr in struct user_thread_info (of course doing so doesn't hurt).
Huw.
-
Huw Davies -
Huw Davies -
Rémi Bernon