Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KeyReleaseEvent.Bytes() doesn't write type #6

Closed
tesselslate opened this issue Apr 23, 2022 · 6 comments · Fixed by #7
Closed

KeyReleaseEvent.Bytes() doesn't write type #6

tesselslate opened this issue Apr 23, 2022 · 6 comments · Fixed by #7

Comments

@tesselslate
Copy link

I believe that KeyReleaseEvent.Bytes() should set the first byte of the event to 3, which is the KeyRelease event type. The current behavior with just returning the value of KeyPressEvent.Bytes() returns the bytes for a key press event (which has type 2), not a key release event.

xgb/xproto/xproto.go

Lines 3414 to 3416 in 7a4fade

func (v KeyReleaseEvent) Bytes() []byte {
return KeyPressEvent(v).Bytes()
}

Manually setting the 0th byte to 3 after creates a proper KeyRelease event to be sent with SendEvent. (excerpt from my code)

    bytes := evt.Bytes()
    if press.State == KeyUp {
        bytes[0] = 3
    }

This same issue seems to be present with other event types as well, such as ButtonPressEvent and ButtonReleaseEvent.

xgb/xproto/xproto.go

Lines 528 to 530 in 7a4fade

func (v ButtonReleaseEvent) Bytes() []byte {
return ButtonPressEvent(v).Bytes()
}

@jezek
Copy link
Owner

jezek commented Apr 25, 2022

Yes, you're definitely right. I've created test programs in c (c++?) using xcb and go using xgb and the xcb version gives different first byte for key release.

$ ./x11KeyPressReleaseBytes_in_c
KeyPress  : 18
KeyPressBytes  : 02 18 02 00 0d f6 21 07 96 04 00 00 00 00 20 01 00 00 00 00 01 00 76 02 cf ff 05 02 00 00 01 00 
KeyRelease: 18
KeyReleaseBytes: 03 18 02 00 99 f6 21 07 96 04 00 00 00 00 20 01 00 00 00 00 01 00 76 02 cf ff 05 02 00 00 01 00 
KeyPress  : 9
KeyPressBytes  : 02 09 02 00 5a fa 21 07 96 04 00 00 00 00 20 01 00 00 00 00 01 00 76 02 cf ff 05 02 00 00 01 00 

$ ./x11KeyPressReleaseBytes_in_go
KeyPress  : 18
KeyPressBytes  :02 18 00 00 e7 1e 22 07 96 04 00 00 01 00 20 01 00 00 00 00 01 00 76 02 cf ff 05 02 00 00 01 00 
KeyRelease: 18
KeyReleaseBytes:02 18 00 00 87 1f 22 07 96 04 00 00 01 00 20 01 00 00 00 00 01 00 76 02 cf ff 05 02 00 00 01 00 
KeyPress  : 9
KeyPressBytes  :02 09 00 00 cf 24 22 07 96 04 00 00 01 00 20 01 00 00 00 00 01 00 76 02 cf ff 05 02 00 00 01 00 

Not that I've doubted you, but now there is a proof.

The thing is that the ./xproto/xproto.go file is generated by ./xgbgen (see MAKEFILE). The command for generation is xgbgen/xgbgen --proto-path /usr/share/xcb /usr/share/xcb/xproto.xml > xproto/xproto.go, or using make command make xproto.xml . The current xproto.go file and other were generated a long time ago on some previous version of xcb xml's (I think xcb-proto 1.10) and with my current version (1.14) it is not working (it panics).

What I want to say is that the solution to simply edit the xproto.go file would not be the proper solution. The proper solution would be to patch the xgbgen and then regenerate for the working xcb-proto version (1.10), or better for current xcb-proto version (1.14).

Help would be appreciated. Can you try to make the patches?

Note: I've seen @aarzilli has a branch with changes to xgbgen for xcb-proto 1.12, but I don't know if it works for 1.14. But it could be a good start.

@tesselslate
Copy link
Author

Sure, I'll take a look at getting xgbgen running and patching it this weekend.

@tesselslate
Copy link
Author

The actual fix for the initial issue is pretty simple but 1.14 support is more problematic. I've managed to get it running with 1.14 on my fork, but I think aarzilli's version might be better. My code has a few pretty bad workarounds for things I didn't understand within xgbgen.

All of the tests in xgb and xproto pass for me, but I'm far less confident about the extensions. I will have to write a few tests for those to see if they work or not. I'll post again when I have those working.

@jezek
Copy link
Owner

jezek commented Apr 30, 2022

I see, the patch for the pre-1.14 version is really simple. Maybe before the bigger patch to 1.14, the code could be regenerated with your patch on the original xcb-proto version (1.10, 1.11?), so there will be only minimal changes fixing only this issue. After that the transition to newer xcb-proto versions can be done in new issue. Do you agree?

@tesselslate
Copy link
Author

Sure, sounds good. I can make a PR with just the 3 line change if you want, or you can push it yourself since it's pretty simple.

@jezek
Copy link
Owner

jezek commented Apr 30, 2022

Can you make a PR? Ideal would be 2 commits.

  • First will be the patch to xgbgen.
  • Second will be the all the code regenerated using the patched xgbgen with the original/past version of xcb-proto (you have to find out which version).

This way it should be easy to review the changes, because only the release events should change.

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants