Skip to content

WIP: add basic touch support - #226

Closed
androw wants to merge 1 commit into
raphamorim:mainfrom
androw:touch
Closed

WIP: add basic touch support#226
androw wants to merge 1 commit into
raphamorim:mainfrom
androw:touch

Conversation

@androw

@androw androw commented Sep 16, 2023

Copy link
Copy Markdown
Contributor

#225
First work on it
Need winit upgrade because of rust-windowing/winit#3035

@raphamorim can you point me to the correct way to send click and cursor move event please?

@raphamorim

Copy link
Copy Markdown
Owner

Hey @androw,

That's great! So for the mouse Rio uses mouse_position function that does return a Pos

rio/rio/src/screen/mod.rs

Lines 227 to 250 in 65ffe90

pub fn mouse_position(&self, display_offset: usize) -> Pos {
let layout = &self.sugarloaf.layout;
let line_fac =
((layout.sugarheight) * self.sugarloaf.layout.scale_factor) as usize;
let mouse_x = self.mouse.x + layout.margin.x as usize;
let col = mouse_x
/ (layout.sugarwidth.floor() * self.sugarloaf.layout.scale_factor) as usize;
// TODO: Refactor
let col = col.saturating_sub(1);
let col = col.saturating_sub(1);
let col = std::cmp::min(Column(col), Column(layout.columns));
// println!("{:?}", self.mouse.x);
// println!("{:?}", layout.sugarwidth);
// println!("{:?}", col);
let line = self.mouse.y.saturating_sub(
(layout.margin.top_y * 2. * self.sugarloaf.layout.scale_factor) as usize,
) / line_fac;
let calc_line = std::cmp::min(line, layout.lines - 1);
let line = Line(calc_line as i32) - (display_offset);
Pos::new(line, col)

With this Pos it uses mouse_report function that reports

rio/rio/src/screen/mod.rs

Lines 1399 to 1433 in 65ffe90

pub fn mouse_report(&mut self, button: u8, state: ElementState) {
let mut terminal = self.ctx().current().terminal.lock();
let display_offset = terminal.display_offset();
let mode = terminal.mode();
drop(terminal);
let pos = self.mouse_position(display_offset);
// Assure the mouse pos is not in the scrollback.
if pos.row < 0 {
return;
}
// Calculate modifiers value.
let mut mods = 0;
let mod_state = self.modifiers.state();
if mod_state.shift_key() {
mods += 4;
}
if mod_state.alt_key() {
mods += 8;
}
if mod_state.control_key() {
mods += 16;
}
// Report mouse events.
if mode.contains(Mode::SGR_MOUSE) {
self.sgr_mouse_report(pos, button + mods, state);
} else if let ElementState::Released = state {
self.normal_mouse_report(pos, 3 + mods);
} else {
self.normal_mouse_report(pos, button + mods);
}
}

But I think for your case, you will need to use on_left_click function https://github.com/raphamorim/rio/blob/main/rio/src/screen/mod.rs#L1247-L1276 it receives the Pos, I think the biggest challenge here is translate touch into Pos (that's why I sent the mouse_position function before)

Note the mouse has a state called ClickState, it can be a click, double or triple. The click state is defined here https://github.com/raphamorim/rio/blob/main/rio/src/sequencer.rs#L438-L474

I think would be worth to make touch reuse the clickState though, then doesn't need to duplicate logic.

Keep in minde that mouse_position function is still not the best and needs to be fixed since whenever you change fonts it can lead to some issues (ref #182)

@androw

androw commented Sep 16, 2023

Copy link
Copy Markdown
Contributor Author

If someone want to test and add feedback. We should have some basics working on X11 (for Wayland we need to wait for winit next version):

  • select text
  • zoom in/out
  • scroll

@nyabinary

Copy link
Copy Markdown

What version of Winit is needed, more specifically what commit? Because Rio uses the latest beta ver of Winit.

@androw

androw commented Sep 21, 2023

Copy link
Copy Markdown
Contributor Author

What version of Winit is needed, more specifically what commit? Because Rio uses the latest beta ver of Winit.

We need this commit rust-windowing/winit@c2ed160 which is not included in latest beta build.

@raphamorim

raphamorim commented Sep 21, 2023

Copy link
Copy Markdown
Owner

@androw since we are already using the beta, I wouldn’t mind add winit dependency pointing to a git hash for the next build. in my experience Winit releases can take a while to wrap up (I know they are also considering a major refactor of how the event loop will work)

@nyabinary

Copy link
Copy Markdown

That's seems like a solid idea tbh, could take advantage of some new features winit too if you point to the latest git hashes. Of course, it'll need proper testing, which I'll be happy to help with :)

@raphamorim

Copy link
Copy Markdown
Owner

@androw just a fyi, updated to winit 0.29.2 in main branch

@androw

androw commented Oct 23, 2023

Copy link
Copy Markdown
Contributor Author

Working on Wayland without patch now!
But simple text selection is not working anymore. I might check this over the weekend

@raphamorim

Copy link
Copy Markdown
Owner

Niceee! thanks @androw let me know anything can help with :D and great job with it

@raphamorim
raphamorim force-pushed the main branch 2 times, most recently from b928ed6 to 0d3fbb5 Compare November 23, 2023 14:56
@nyabinary

Copy link
Copy Markdown

What the status on this?

@androw

androw commented Nov 27, 2023

Copy link
Copy Markdown
Contributor Author

I've updated the branch and selection is still not working. I didn't have time to work on it and won't be able for some times.

@raphamorim

Copy link
Copy Markdown
Owner

No stress @androw, take your time on it 🙏
@nyabinary please don't comment like this, feels as pressure to release. It will come naturally 🙏

@nyabinary

Copy link
Copy Markdown

No stress @androw, take your time on it 🙏 @nyabinary please don't comment like this, feels as pressure to release. It will come naturally 🙏

Oh, I was under the impression that it was ready to review, sorry! Should mark it as a draft PR too.

@raphamorim

Copy link
Copy Markdown
Owner

No stress @nyabinary 🙏

@androw

androw commented Dec 11, 2023

Copy link
Copy Markdown
Contributor Author

@raphamorim i do not find what's wrong with my way of emulating a click. Can you have a look when you have some time?

@raphamorim

Copy link
Copy Markdown
Owner

@raphamorim i do not find what's wrong with my way of emulating a click. Can you have a look when you have some time?

Of course, you are talking about the CI error?

@androw

androw commented Dec 12, 2023

Copy link
Copy Markdown
Contributor Author

No (I haven't check the CI error yet).
For example, I'm using the following code to simulate a click

                route.window.screen.mouse.x = x;
                route.window.screen.mouse.y = y;

                let now = Instant::now();
                route.window.screen.mouse.last_click_timestamp =
                                            now;
                route.window.screen.mouse.last_click_button = MouseButton::Left;

                route.window.screen.mouse.click_state = ClickState::Click;
                route.window.screen.mouse.left_button_state = ElementState::Pressed;
                route.window.screen.on_left_click(route.window.screen.mouse_position(0));

and the following for releasing the click

            route.window.screen.mouse.click_state = ClickState::None;
            route.window.screen.mouse.left_button_state = ElementState::Released;

It used to work but it isn't (for example selection doesn't) since some changes on the main branch.
I think I'm not doing this the right way...

@raphamorim

Copy link
Copy Markdown
Owner

It used to work but it isn't (for example selection doesn't) since some changes on the main branch.

Got it! Will take a look once I wrap the sixel protocol 🙏

@raphamorim

Copy link
Copy Markdown
Owner

@androw I've merged your changes into main (ae77605). Overall the code does look fine for me (just added few updates in another commit), haven't tested properly the selection only the touch events but I will test before include to next release. Thank you for your work with it 🙏

@raphamorim raphamorim closed this Jan 16, 2024
raphamorim added a commit that referenced this pull request Apr 28, 2024
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.

3 participants