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

Wayland/virtual keyboard #720

Merged
merged 33 commits into from
Oct 31, 2022
Merged

Conversation

rano-oss
Copy link
Contributor

@rano-oss rano-oss commented Aug 26, 2022

This adds support for virtual keyboard, it needs a new version of wayland-protocols-misc for it to be able to be merged. The pr probably needs some reviewing first anyway. ;)
edit: Everything is tested and implemented, should be ready :')

@rano-oss rano-oss marked this pull request as draft August 26, 2022 23:58
@codecov-commenter
Copy link

codecov-commenter commented Aug 31, 2022

Codecov Report

Base: 25.83% // Head: 9.78% // Decreases project coverage by -16.04% ⚠️

Coverage data is based on head (f0c84e3) compared to base (4940384).
Patch coverage: 14.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #720       +/-   ##
==========================================
- Coverage   25.83%   9.78%   -16.05%     
==========================================
  Files         118     120        +2     
  Lines       18698   18952      +254     
==========================================
- Hits         4831    1855     -2976     
- Misses      13867   17097     +3230     
Flag Coverage Δ
wlcs-core ?
wlcs-output 9.78% <14.00%> (+0.01%) ⬆️
wlcs-pointer-input ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/input/keyboard/keymap_file.rs 29.41% <0.00%> (-62.70%) ⬇️
src/wayland/seat/keyboard.rs 0.00% <0.00%> (-16.81%) ⬇️
...ayland/virtual_keyboard/virtual_keyboard_handle.rs 0.00% <0.00%> (ø)
src/input/keyboard/mod.rs 14.77% <5.00%> (-0.73%) ⬇️
src/wayland/virtual_keyboard/mod.rs 40.29% <40.29%> (ø)
anvil/src/state.rs 46.12% <100.00%> (-2.24%) ⬇️
src/wayland/compositor/cache.rs 0.00% <0.00%> (-80.89%) ⬇️
src/backend/renderer/element/surface.rs 0.00% <0.00%> (-80.30%) ⬇️
src/desktop/wayland/utils.rs 0.00% <0.00%> (-70.77%) ⬇️
src/desktop/space/wayland/window.rs 0.00% <0.00%> (-61.23%) ⬇️
... and 42 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@rano-oss rano-oss marked this pull request as ready for review August 31, 2022 12:01
@rano-oss rano-oss requested a review from i509VCB August 31, 2022 16:44
@rano-oss rano-oss requested a review from i509VCB September 10, 2022 21:17
@rano-oss rano-oss removed the request for review from i509VCB September 17, 2022 22:07
@rano-oss rano-oss requested a review from PolyMeilex September 17, 2022 22:07
@rano-oss
Copy link
Contributor Author

rano-oss commented Sep 17, 2022

Seems that github actions is not allowing clippy to run properly here?

@PolyMeilex
Copy link
Member

Seems that github actions is not allowing clippy to run properly here?

Smithay build broke after new wayland-rs 0.30 beta got released, proper fix is #752

@rano-oss
Copy link
Contributor Author

I guess I will wait for that then 🥲

Copy link
Member

@Drakulix Drakulix left a comment

Choose a reason for hiding this comment

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

Looks mostly fine from a code point of view, but I am not sure on the behavior from the protocol side.

Seems like keymap handling is real hassle and I don't know, if this is handled in the best way. I would like input from @i509VCB or @PolyMeilex before merging this.

kbd.keymap(KeymapFormat::XkbV1, fd.as_raw_fd(), size as u32)
});
if let Err(e) = res {
debug!(logger,
Copy link
Member

Choose a reason for hiding this comment

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

I think this should rather be a warning

Copy link
Contributor Author

@rano-oss rano-oss Oct 31, 2022

Choose a reason for hiding this comment

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

Should I change this, or merge and create a new shorter pr? 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Figured I might as well 😸

Copy link
Member

@PolyMeilex PolyMeilex left a comment

Choose a reason for hiding this comment

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

My concerns about keymap handling were addressed so it's good to go as far as I'm concerned.

@Drakulix
Copy link
Member

Sadly you broke CI now. 😅
I'll merge once its green again.

@rano-oss
Copy link
Contributor Author

Just a minor hiccup 🙈

@Drakulix Drakulix merged commit 44c864e into Smithay:master Oct 31, 2022
@rano-oss rano-oss deleted the wayland/virtual_keyboard branch October 31, 2022 20:48
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 this pull request may close these issues.

6 participants