[Bug 29871] drawing in photoshop cs5 is almost impossible

http://bugs.winehq.org/show_bug.cgi?id=29871

Lucius Windschuh  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #22 from Lucius Windschuh  ---
(In reply to comment #20)
> No. As comment #5 explains, the patch is a hack that disables a Wine
> feature. Were it included in Wine, it would break other things.
> As it is, a real solution has to be found. Regressions tend to be high
> priority for the devs though so this will be fixed eventually.

I am sorry, but commit cb3b7237925a24ba4c5696dd079fdc5d99a48577 is still
faulty:

Each thread has its own key cache that is used in
dlls/user32/input.c:GetAsyncKeyState(.).
Unfortunately, Photoshop uses different threads to query the mouse button state
when drawing.
I added printf() calls in GetAsyncKeyState to visualise this:
--8<--
(button 1 is pressed)
tkey 0x7ffd8044 <-- Thread 1
key 1: c1 (81)
-
tkey 0x81ed0044 <-- Thread 2
key 1: SHORTCUT 0, 79008969, 79008921 <---- WRONG
tkey 0x81ed0044
key 1: 81 (81) <-- Aha, button 1 still pressed
-
tkey 0x7ffd8044
key 1: 81 (81)
-
tkey 0x7ffd8044
key 1: 81 (81)
-
tkey 0x7ffd8044
key 1: 81 (81)
--8<--

To show this, I modified the function as follows:
--8<--
SHORT WINAPI DECLSPEC_HOTPATCH GetAsyncKeyState( INT key )
{
    struct user_thread_info *thread_info = get_user_thread_info();
    SHORT ret;

if (key < 0 || key >= 256) return 0;

check_for_events( QS_INPUT );

printf("tkey %p\n", thread_info);
    if ((ret = USER_Driver->pGetAsyncKeyState( key )) == -1)
    {

if (key == 1 && thread_info->key_state &&
            !(thread_info->key_state[key] & 0x80) &&
            GetTickCount() - thread_info->key_state_time < 50) {
            printf("key %d: SHORTCUT %x, %lu, %lu\n", (int) key,
(int)thread_info->key_state[key], (unsigned long) GetTickCount(),
thread_info->key_state_time);
            return 0;
         }

if (!thread_info->key_state) thread_info->key_state = HeapAlloc(
GetProcessHeap(), 0, 256 );

ret = 0;
        SERVER_START_REQ( get_key_state )
        {
            req->tid = 0;
            req->key = key;
            if (thread_info->key_state) wine_server_set_reply( req,
thread_info->key_state, 256 );
            if (!wine_server_call( req ))
            {
                if (reply->state & 0x40) ret |= 0x0001;
                if (reply->state & 0x80) ret |= 0x8000;
                thread_info->key_state_time = GetTickCount();
                printf("key %d: %x (%x)\n", (int) key, (int) reply->state,
(int) thread_info->key_state[key]);
            }
        }
        SERVER_END_REQ;
    }
    printf("-\n");
    return ret;
}
--8<--

That's why Photoshop stops drawing the stroke right at the beginning: The
second thrad is told that mouse button 1 is not pressed, because outdated data
is cached by in the thread with thread info @0x81ed0044, while the drawing was
initiated by another thread with thread_info @0x7ffd8044.

So, the mentioned commit is not thread-safe and breaks applications, like
Photoshop.
Is it really more that a performance improvement for other apps?
What now? Revert the commit, admit that fixing the threading issue will reduce
the benefit of the cache? ;-)

I am really interested in a solution that is acceptable for all.