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

X11 shell keyboard mapping #1779

Merged
merged 15 commits into from
Aug 15, 2021
Merged

X11 shell keyboard mapping #1779

merged 15 commits into from
Aug 15, 2021

Conversation

maan2003
Copy link
Collaborator

@maan2003 maan2003 commented May 13, 2021

Resolves #1633

I am using a fork of the xcbcommon-sys crate because it doesn't build with x11 feature enabled, waiting for meh/rust-xkbcommon-sys#5 to merged. Now just running bindgen

The xkb.rs is from #1498

@maan2003 maan2003 added the shell/x11 concerns the X11 backend label May 13, 2021
@jneem
Copy link
Collaborator

jneem commented May 13, 2021

Nice to see so much progress on x11! My main concern at this point is the dependence on xkbcommon-sys, which seems "lightly" maintained and also has a strange license...

I wonder how much work it is just to run bindgen ourselves?

@maan2003
Copy link
Collaborator Author

maan2003 commented May 13, 2021

I feel same. Maintaining a xkbcommon-sys crate ourselves feels the best solution to me. I am not inclined towards running a bindgen inside druid-shell though

maan2003 added a commit to maan2003/druid that referenced this pull request May 20, 2021
SecondFlight pushed a commit that referenced this pull request May 21, 2021
* [ci] Install libxkbcommon for #1779

* Ubuntu likes -dev a lot
@maan2003 maan2003 marked this pull request as ready for review July 1, 2021 20:28
@maan2003
Copy link
Collaborator Author

maan2003 commented Jul 1, 2021

Ready for review :)

@maan2003 maan2003 added the S-needs-review waits for review label Jul 2, 2021
Copy link
Collaborator

@jneem jneem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! I just had a few minor questions/suggestions.

druid-shell/src/platform/gtk/keycodes.rs Outdated Show resolved Hide resolved
druid-shell/src/platform/x11/application.rs Outdated Show resolved Hide resolved
///
/// Reference counted under the hood.
// Assume this isn't threadsafe unless proved otherwise. (e.g. don't implement Send/Sync)
// TODO do we need UnsafeCell?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would we need to check to find out? Is it a question of whether the FFI calls modify anything through the pointer?

Copy link
Collaborator Author

@maan2003 maan2003 Jul 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uh took code from wayland backend and missed this TODO 😬 . I will look into it

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure, why we need it; we only interact with *mut xkb_context

cc @derekdreery

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've honestly forgot about how I did this with wayland, sorry!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need UnsafeCell, UnsafeCell is used to get *mut T from &T(well &UnsafeCell<T>) but already have *mut T

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought about this again, I have no doubts.
I will merge in 2 days if no one objects.

druid-shell/src/platform/x11/xkb.rs Outdated Show resolved Hide resolved
druid-shell/src/platform/x11/xkb.rs Outdated Show resolved Hide resolved
druid-shell/src/platform/x11/xkb.rs Outdated Show resolved Hide resolved
@cmyr
Copy link
Member

cmyr commented Jul 26, 2021

Anything I can do to push this forward?

@maan2003
Copy link
Collaborator Author

still no consensus on if UnsafeCell is needed. everything else is settled I think

@maan2003 maan2003 merged commit dfe8eed into linebender:master Aug 15, 2021
@xStrom xStrom removed the S-needs-review waits for review label May 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
shell/x11 concerns the X11 backend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remapped keys in X11 have their original value in druid
5 participants