touch re-factor #1326
touch re-factor #1326
Conversation
|
As noted in matrix previously, I am not yet convinced, that dropping the But I guess the only way to figure this out, is to implement it. And apart from that I do agree with the general structure. |
cmeissl
force-pushed
the
feature/touch
branch
3 times, most recently
from
ad75176
to
846b662
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1326 +/- ##
==========================================
- Coverage 21.23% 20.38% -0.86%
==========================================
Files 156 158 +2
Lines 25023 25818 +795
==========================================
- Hits 5313 5262 -51
- Misses 19710 20556 +846
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
cmeissl
force-pushed
the
feature/touch
branch
from
a35d89d
to
110d66f
Compare
| FocusTarget::Popup(p) => PointerTarget::enter(p.wl_surface(), seat, data, event), | ||
| PointerFocusTarget::WlSurface(w) => PointerTarget::enter(w, seat, data, event), | ||
| #[cfg(feature = "xwayland")] | ||
| PointerFocusTarget::X11Surface(w) => PointerTarget::enter(w, seat, data, event), |
There was a problem hiding this comment.
If Window/LayerSurface/Popup can be combined as just WlSurface, shouldn't that also cover X11Surface?
It seems PointerTarget<D> for X11Surface just calls PointerTarget methods on the underlying WlSurface, if it exists.
There was a problem hiding this comment.
Yes, that would be possible, but it felt strange to leave the KeyboardTarget impl there (which does a few x related stuff) and only remove the PointerTarget impl.
I've been thinking about how to simplify this code for a while. Implementations of these traits become particularly verbose with a more complicated shell like cosmic-comp: https://github.com/pop-os/cosmic-comp/blob/master/src/shell/focus/target.rs. It turns out it can be make object-safe, so we can have a `&dyn PointerTarget`, if `PartialEq` and `Clone` requirements are moved out of the trait itself. I still wonder if it could be better to use enums for input events instead of a million methods. Anyway, something to think about. Leaving as a draft pending the other changes in input targets in Smithay#1326.
I've been thinking about how to simplify this code for a while. Implementations of these traits become particularly verbose with a more complicated shell like cosmic-comp: https://github.com/pop-os/cosmic-comp/blob/master/src/shell/focus/target.rs. It turns out it can be make object-safe, so we can have a `&dyn PointerTarget`, if `PartialEq` and `Clone` requirements are moved out of the trait itself. I still wonder if it could be better to use enums for input events instead of a million methods. Anyway, something to think about. Leaving as a draft pending the other changes in input targets in Smithay#1326.
|
Linking to #1334 (comment) as it contains some background info about my motivation behind removing the |
cmeissl
force-pushed
the
feature/touch
branch
2 times, most recently
from
679b4b7
to
5151f2f
Compare
cmeissl
force-pushed
the
feature/touch
branch
from
5151f2f
to
15cb1ca
Compare
|
So the idea is that a compositor (like cosmic-comp) would have some concept of a focus stack, without such a thing existing explicitly in smithay itself? Makes sense for that to be represented as a stack, though I don't know exactly how that will end up looking. It's good to have touch brought up to parity with keyboard/pointer. |
For the moment, yes. Though cnce we figure out a generic way to represent it we probably should add an abstraction to smithay. |
especially the pointer target is problematic as it dispatches the pointer target to surfaces based on the location of the pointer. this can break guarantees a grab tries to enforce.
this brings touch handling to the same level as we already provide for pointer and keyboard.
the default touch grab will invoke the touch down grab which will keep the focus on a single surface. while this is a sane default downstream might want to override this behavior.
this adds a new function to PointerTarget that is called when the focus is replaced. this allows implementing a nested stack of focus targets without sacrificing grab guarantees. the default impl will do the same like the old behavior, so call leave on the previous target and enter on the new one.
cmeissl
force-pushed
the
feature/touch
branch
from
15cb1ca
to
e29f8be
Compare
The current implementation has a few limitations and does not provide an abstraction on the same level as pointer or keyboard. For proper support including DnD we also need to support grabs.
This also removes the implementation of
PointerTargetandKeyboardTargeton the desktop abstractions. I described the motivation behind this hereThe last commit also implements the base for the idea mentioned in the above comment.