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

Added basic support for input_regions #111

Merged
merged 7 commits into from
Jan 1, 2025

Conversation

manthanabc
Copy link
Contributor

Attempts to solve #110

This creates an interface to dynamically set input region, which can be edited by user by means of a new action

   // Simple application
   LayershellCustomActoin::SetInputRegion(|region| {
            region.add(0, 0, 5000, 5000);
            region.subtract(0, 0, 50, 50);
    }))
   // Multi-Window application
   Message::SetInputRegion { 
      set_region: |region| { 
        region.add(500,500,1920,1080); 
      }, id: WindowId
   }

I'm marking this as draft, any suggestions would be much appreciate since its my first time contributing here.
This also adds nessasory changes to to_layer_message macro, and to multi window mode.

@manthanabc manthanabc marked this pull request as draft December 27, 2024 18:09
virtual_keyboard_manager.create_virtual_keyboard(seat, qh, ());
virtual_keyboard_in.keymap((*keymap_format).into(), file.as_fd(), *keymap_size);
ev.set_virtual_keyboard(virtual_keyboard_in);
let wl_compositor = globals.bind::<WlCompositor, _, _>(qh, 1..=1, ());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here can use expect, because wl_compositor should exist

LayershellCustomActions::SetInputRegion(set_region) => {
let window = ev.main_window();

if let Some(region) = &wl_input_region {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will exist after doing bind, so here can also use expect

@Decodetalkers
Copy link
Collaborator

Can you add an example for this new apo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

should i just create an examples folder and add this example there instead?, since i feel like readme should have a minimal example only

Copy link
Collaborator

Choose a reason for hiding this comment

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

I mean add an examples folder and add this example instead. And it will be better to add comments in the place of this code

@manthanabc manthanabc force-pushed the master branch 3 times, most recently from 636857b to 658ee39 Compare December 30, 2024 13:31
@Decodetalkers Decodetalkers marked this pull request as ready for review December 30, 2024 13:44
@Decodetalkers
Copy link
Collaborator

I think it is already fine to be merged

@Decodetalkers
Copy link
Collaborator

Now finally , you need to fix the clippy problem

@manthanabc
Copy link
Contributor Author

manthanabc commented Dec 30, 2024

I think it is already fine to be merged

Yea, but we can't yet pass closures to set_input, unless it does not capture the environment variables (so are automatically converted to fn). which seems really useful, since without it the shapes have to be pretty much statically decided at compile time. to allow for it i would either need to

  • change SetInputRegion(&'a (dyn Fn(&WlRegion)) which would mean i'd need to introduce the <'a> everywhere LayershellCustomAction is contained which i dont really like
  • or just wrap it in box<dyn Fn(&WlRegion)> which would need user to also wrap the closure in box which is fine but it does heap allocation so might not be desirable

i cannot decide which one i should do
edit: typos

@manthanabc
Copy link
Contributor Author

@Decodetalkers does this look good 👀 ??

@Decodetalkers
Copy link
Collaborator

ok, I will merge it

@Decodetalkers Decodetalkers merged commit af6c39a into waycrate:master Jan 1, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants