aseprite / laf

A C++ library to create desktop applications
https://aseprite.github.io/laf/
MIT License
281 stars 60 forks source link

Fix: Drawing using keys on Windows #70

Closed tetektoza closed 1 year ago

tetektoza commented 1 year ago

This patch is essentially the same implementation as in https://github.com/aseprite/laf/pull/69 - now the repeat count is being set correctly if we press the same event twice in a row, resulting in key events binded to mouse events being converted properly.

I agree that my contributions are licensed under the MIT License. You can find a copy of this license at https://opensource.org/licenses/MIT

dacap commented 1 year ago

About this change there are some notes and an alternative fix that I'll like to push.

The idea of the repeat field was to match the repeat count of the WM_KEYDOWN event, but never worked as expected. From the "MSDN" documentation https://learn.microsoft.com/en-us/windows/win32/inputdev/wm-keydown about the LPARAM argument:

Bits 0-15 = The repeat count for the current message. The value is the number of times the keystroke is autorepeated as a result of the user holding down the key.

And this is set here:

https://github.com/aseprite/laf/blob/592d028e5d9747c84dd846a166efc10e7363e98a/os/win/window.cpp#L1682

But the only problem is that it was not what I understood at the time. From this answer the repeat count is the number of queued WM_KEYDOWN messages that are not processed, but if we process the queue fast enough, the value is always 0. It's not what I was expecting for a "autorepeat" count.

Anyway, we can still use one specific bit of the LPARAM which is the bit 30:

Bit 30 = The previous key state. The value is 1 if the key is down before the message is sent, or it is zero if the key is up.

Which is already being used to detect if a key modifier is pressed for first time or it's being held:

https://github.com/aseprite/laf/blob/592d028e5d9747c84dd846a166efc10e7363e98a/os/win/window.cpp#L1670-L1675

So the alternative fix uses just this bit to set the repeat field: 58709a07c97243447897147c118c4e0005bbd162

dacap commented 1 year ago

Thanks anyway @tetektoza about this, I'll update the laf submodule on aseprite repository now.