High CPU usage when left mouse button is pressed

[raphCode]

Pressing and holding the left mouse button leads to 5-20% CPU usage in the zellij server and 1-3% in the client, depending on the machine and the process running inside zellij.

I can see that applications inside zellij are spammed with mouse escape sequences whenever zellij thinks the mouse button is down.
This happens even when not moving the mouse, whereas normally only cursor updates should be sent to the application.

This bug is particularly bad because in some cases zellij fails to detect the mouse-up event and continues burning CPU time whereas the physical mouse button is not even pressed anymore: #1334

I am using alacritty.

[tlinford]

Good find, I'm aware of this behavior, but didn't realize that cpu usage was so bad.

This is due to how we implemented the scrolling while holding down the mouse button, we are basically repeating the mouse events to get the pane to scroll and update the text selection:

zellij/zellij-client/src/stdin_handler.rs

Lines 19 to 44 in 6822b9a

	let parse_input_event = |input_event: InputEvent| {
	holding_mouse = should_hold_mouse(input_event.clone());
	if holding_mouse {
	let mut poller = os_input.stdin_poller();
	loop {
	let ready = poller.ready();
	if ready {
	break;
	}
	send_input_instructions
	.send(InputInstruction::KeyEvent(
	input_event.clone(),
	current_buffer.clone(),
	))
	.unwrap();
	}
	} else {
	send_input_instructions
	.send(InputInstruction::KeyEvent(
	input_event,
	current_buffer.drain(..).collect(),
	))
	.unwrap();
	}
	};
	input_parser.parse(&buf, parse_input_event, maybe_more);

When these fake mouse events reach Tab, we currently don't have a way to differentiate them mouse events from the original ones, so they also get forwarded as is to applications.

This second problem is easier to fix, we could for example modify the various InputInstruction, Action, etc..., to carry this info and avoid forwarding to apps if the event is fake.

For the root issue, not sure yet how solve this.

[raphCode]

Thanks for the explanation.

Not sure if that is possible, but I feel it would be cleaner to not send any duplicate / fake events and let the server do the scrolling all by itself if it detects the mouse reaches the top / bottom border?

For the root issue, not sure yet how solve this.

Do you feel we should separate the missing-mouseup problem into a new issue?

[tlinford]

Not sure if that is possible, but I feel it would be cleaner to not send any duplicate / fake events and let the server do the scrolling all by itself if it detects the mouse reaches the top / bottom border?

Something like that would be great IMO, but it needs figuring out :)

Do you feel we should separate the missing-mouseup problem into a new issue?

Sure that would be great!

[tlinford]

Related #1309, #1325.

@imsnif since the event repeater is apparently causing quite a lot of issues, would it maybe make sense to disable it until we find a better way to handle scrolling up the pane while selecting?

[imsnif]

This issue I understand, but do you think it's really related to the others? #1325 - I'd guess is mostly a positional thing (we shouldn't be scrolling if the click goes to another pane) and #1309 I think only started happening when we switched STDIN parsing libs?

[tlinford]

#1325 - I'd guess is mostly a positional thing (we shouldn't be scrolling if the click goes to another pane)

You are right, this still happens without the repeater. There is an incorrect assumption that a click will always focus a pane and initiate selection, but that's not the case for plugin panes.

#1309 I think only started happening when we switched STDIN parsing libs?

Yep, but as @raphCode said we are spamming apps with the synthetic events and that causes some other problems.