KWayland takes always the top-most child surface at a given position for its
pointer input. But if a sub-surface sets its input region, it should not select
this one when the position is out of its input region, but rather try the
surface below.
Details
Details
My testing was only on my Xwayland branch. Supposed to also fix a problem
with Firefox native Wayland port.
Diff Detail
Diff Detail
- Repository
- R127 KWayland
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Comment Actions
Please verify that this does not break Qt! (Qt has a very special handling of subsurfaces and we have lots of workarounds for Qt there)
Also please add a unit test which exposes the problem.
| src/server/surface_interface.cpp | ||
|---|---|---|
| 859 | Could you try to share the logic between surfaceAt and inputSurfaceAt methods? I cannot spot a difference except the method it calls and that could be handled in a more generic way (e.g. function pointer). | |
Comment Actions
I'm currently on a tight time budget, so this will need to wait a little bit. I'll roll with the change on my machine for a while and if everything works fine add a unit test in the end.
Comment Actions
I like that refactoring 👍 - though processAt might not be the best fitting name. Don't have a good name idea, maybe findChildSurfaceAt?
Comment Actions
Currently I'm working on other stuff and don't want to increase the mental load. I really only need this when looking again into sub-surfaces to handle Xwayland Present on child windows. But if there are some real-life problems associated with this patch missing I will look at it again.
| src/server/surface_interface.cpp | ||
|---|---|---|
| 837 | Shouldn't it be QRect instead? | |
| src/server/surface_interface.cpp | ||
|---|---|---|
| 837 | position is a float | |
| src/server/surface_interface.cpp | ||
|---|---|---|
| 837 | So, it would be fine to return true in the following case: QRectF(QPoint(0, 0), QSize(10, 10)).contains(QPointF(10, 10)); ? | |
Comment Actions
I've written you a unit test.
I can either take over this or upload as a separate review that you can merge in.
| src/server/surface_interface.cpp | ||
|---|---|---|
| 835 | This needs to be && (surface.inputIsInfinite() || input.contains(..)) which means either an extra arg in our function pointer or go back to the frankly simpler first revision. Writing N complex lines to save duplicating N simple lines doesn't make much sense to me, especially when they end up deviating. | |
| 837 | I would have said so. But then it means sizeContains and inputContains behave differently as QRegion(QRect(0,0,10,10)).contains(10,10); returns false which gives us some inconsistency. | |
Comment Actions
- Update version number
- Do code duplication instead of pointers
- Add comment about code duplication
Comment Actions
QRectF(QPoint(0, 0), size()).contains(position)
Shouldn't it be QRect instead? (again)
If it should be QRectF indeed, could someone please explain why?