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

Map touch inputs to the single enabled display when the 'last' multi-display mode is in use #388

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

matt-oakes
Copy link

I'm not certain if this is something you will want to merge as-is, but it was a useful patch for us to resolve issues with touch input devices which do not report which display out they map to.

In our situation we run with -m last, so there is only ever a single enabled display. This PR detects this situation and automatically maps the touch and pointer inputs to this enabled display.

This avoids the need for pre-configuring this using udev rules (I beleive this is considered deprecated anyway?) and allows Cage to automatically work with any touch input device and output display automatically.

I have made sure this works both when the input devices are signalled as added before the outputs and when either the inputs or outputs change.

Related to #243.

Similar in idea to #167 and #193.

…y mode is in use

This ensures that for the situation where there is only a single display in use we can use the touch input devices correctly automatically.
@joggee-fr
Copy link
Collaborator

I thought that without mapping reported by the (touch) input, it was mapped to the whole output layout. Consequently, if you only have one display enabled, it should be always right.
That was not the case for you @matt-oakes?

@matt-oakes
Copy link
Author

That is true, which means it works fine if the output has not transforms applied to it. However, as it's not actually linked to the output itself, it doesn't take into account any transforms which are active on the display. This means that if you have a rotated or flipped display, the touch will be applied in the wrong location.

In our situation we allow have touch displays which can change orientation and rotation so we need to account for that.

This PR makes the link between the input device and the output device properly so transforms are taken into account.

@joggee-fr
Copy link
Collaborator

@matt-oakes
Ok, I got it. Thanks for the precision.

This comment from @emersion gives some ideas for input to output mapping. You PR is a very special case of the heuristic solution I guess.

@matt-oakes
Copy link
Author

I've just looked again at the wlroots code, and there might actually be a better and more "correct" way to achive the same outcome as this PR does.

When an absolute touch event comes in, it gets the mapped output and transforms it.

When getting the mapped output, it first checks the mapped_output of the input device (the property which this PR currently sets). If that isn't set, it then checks the state->mapped_output property of the wlr_cursor itself.

We can set this by calling wlr_cursor_map_to_output. We would do this whenever we change the single enabled display. This would happen at the same points we're currently calling seat_remap_inputs_to_first_enabled_output in this PR.

I think this actually is the correct behaviour for -m last mode anyway as we would be saying "the cursor is mapped to this single output". Because of this, we wouldn't need to make any changes to the current calls to wlr_cursor_map_input_to_output so if an input device does correctly set the wlr_output propert, this would still be set correctly. It would only fall through to the cursor->state->mapped_output when an input device doesn't have a mapped_output set.

Does that sound like the correct behaviour to you too?

@matt-oakes
Copy link
Author

@joggee-fr Just following up on this proposed change. Does the above sound like the correct behaviour to you?

@emersion
Copy link
Contributor

emersion commented Feb 6, 2025

I'm confused - in -m last mode, the output layout should only contain a single output, so the input events should already be routed to only that output.

@matt-oakes
Copy link
Author

That is correct, but because Cage is not currently calling wlr_cursor_map_to_output it is not setting the cursor state->mapped_output property. This means that when a touch event is handled and it calls get_mapped_output it will not have either of a cursor_device->mapped_output (unless udev rules have been setup, which is deprecated) or a cursor->state->mapped_output (because we have not called wlr_cursor_map_to_output).

Because of this, the touch event will be handled, but the apply_transformation method will not have been called. This means it will work correctly for touches on displays which are not rotated or flipped, but will be incorrect if any display transformations are present.

To resolve this, I am proposing a different and simplier change than this PR. If Cage is in "last display mode" (-m last) then when the single output changes, we will need to call wlr_cursor_map_to_output with the new output.

I'm happy to change this PR to get this working and provide a video of the behaviour before and after to make it clear what the issue is and what the fix will do.

@joggee-fr
Copy link
Collaborator

It seems the PR can at least be extended to manage every cases where only one output is available, not only when using -m last option.

@matt-oakes
Copy link
Author

Yes, that would be the case. We'd just need to handle the case where a second output becomes enabled so it doens't map incorrectly.

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