Implement pointer-constraints and relative-pointer

[vanfanel]

Info

dwl's commit: b8bc54b65d7dfaf1c7aa92de32f8ca37d8e011f9 (current GIT, wlroots-next branch) wlroots version: stable and b7e2a2584e99f0b11d564a5cc9c44fc5ba974a3d (current GIT)

Description

SDL2 programs using cursor confinement don't get cursor confinement at all, like for example Scummvm in fullscreen mode (where 4:3 games have black pillars on both sides on 16:9 screen when aspect ratio correction is ON). Any other program using cursor confinement will do the same: no working cursor confinement.

SDL2 uses SDL_SetWindowMouseRect() to confine the mouse pointer, which in turn uses Wayland_input_confine_pointer() on Wayland, so my guess is that Wayland_input_confine_pointer() is not doing anything in DWL.

Other compositors have it working, like Sway or Weston.

[sevz17]

@djpohly, shall we include pointer constraints in main?, lately, people had been requesting this feature (https://github.com/djpohly/dwl/issues/313, https://github.com/djpohly/dwl/issues/265 and some other pr's) I said I don't plan to add it, but maybe we should work on one of the patches to get it merged?.

[fauxmight]

+1 Yes, please include pointer-constraints/relative-pointer in main.

With full understanding that this will mean substantial code lines addition, there is no other recognized benefit to keeping this OUT of dwl.

[djpohly]

The benefit of keeping it out is exactly that though: managing the complexity of the code.

Is there a use case for this outside of video games?

[vanfanel]

+1 here too, please make it into dwl, it's a basic compositor function. Anything using SDL2 and fullscreen graphics with aspect ratio correction is affected by the lack of this, that's a LOT of software, not only videogames.

[pm4rcin]

+1 here too, please make it into dwl, it's a basic compositor function. Anything using SDL2 and fullscreen graphics with aspect ratio correction is affected by the lack of this, that's a LOT of software, not only videogames.

@djpohly asked about use case other than video games. I would like to see it in main (just because I patch it manually) but I understand his view. So if you could name a few programs that use it then it could convince him. Because saying "that's a LOT of software" without giving examples is not a strong argument. If it's so common than it shouldn't be hard for you to find it right? :)

[vanfanel]

@djpohly I for one use UAE on SDL2 to load up a lot of Amiga software (design and music making programs, like Lightwave and Deluxe Paint). Since aspect ratio correction is needed on those programs, the cursor gets sticky around the edges because it's not confined.

Same goes for ANY DOS program running on DOSBOX with aspect-ratio correction. There I use trackers and VistaPro.

None of these are games, even if they run on emulated enviroments: Anything needing fullscreen + aspect ratio correction will have sticky pointer without this.

[sevz17]

@vanfanel, those programs, while not games, are simply not used by the vast majority of dwl users, so they are not a good incentive to get pointer constraints in main (I use pointer constraint in my custom build, so it's not because I wouldn't use it!)

@djpohly, in the other hand, parity with dwm is a good point, and the xorg server implements pointer constraints :)

[pm4rcin]

@sevz17 I see one (not so) small problem. There's not enough lines left for the current patch...

[sevz17]

Yeah, that is another point, dwm is limited to 2000 lines while our limit is 2200[^1], which is obviously not on par with dwm

[^1]: to be honest I don't think 2200 lines are sufficient, we have to implement things from dwm AND xorg server

[czettnersandor]

I found this issue searching for a solution to my problem. I sometimes enjoy a quick Quake 1 run while programming. Quakespasm was not able to capture the mouse. I tried other patches, but they did not work, then finally this solved it:

https://github.com/sevz17/dwl/commit/a18dab86893714b4103d9965ec0e143e53ffbfd8.patch

I see it takes some effort to maintain this kind of patch, so it would be great to see it in dwl master. It's an essential feature that makes some programs work. In DWM, it's part of X11, so they can fit into 2000 lines. I don't see a reason why enforcing a limit on LOC is more important than a working environment.

Thanks for this port (dwl) btw, I'm now finally switched to Wayland as a hardline dwm user.

[fauxmight]

@czettnersandor Speaking with experience on this matter, not authority: this has been discussed quite a bit and is not likely to change any time soon due to the noted substantial additional code. The branch you mention from @sevz17's repository is available via the patches on the wiki as sway-pointer-constraints and is regularly maintained and updated.

Not all dwl users are game-players or require the effects of either of the available pointer-constraints patches.

[czettnersandor]

@fauxmight thanks, understandable. I did not find that one, I'll use the patch in the future. pointerConstraints did not work for me, so maybe remove it from the wiki? It looks like it's missing the pixman parts that's in sway-pointer-constraints, and that's the only difference in @PalanixYT 's patch

[pm4rcin]

As of today I've realized that this whole thing should be treated as "temporary" because recently there are moves to upstream the Wayland driver for Wine so it would no longer be needed. Of course it will still take some time but we're getting closer each day. The plan is to make it usable this year so...

[PalanixYT]

The wayland-wine driver will remove the need for xwayland, but the pointer-constraints patches will still be needed.

[PalanixYT]

It looks like it's missing the pixman parts that's in sway-pointer-constraints, and that's the only difference in @PalanixYT 's patch

That is probably not the case, since pixman is used for pixel manipulation. There has probably been a change to the interface that I haven't looked at yet. I'll be sitting down today to look at it

[pm4rcin]

The wayland-wine driver will remove the need for xwayland, but the pointer-constraints patches will still be needed.

Could you explain why it'll be needed?

[PalanixYT]

For the same reason it's needed now.

[PalanixYT]

pointerConstraints did not work for me. It looks like it's missing the pixman parts that's in sway-pointer-constraints

@czettnersandor I've tried the patch before and it still seemed to work. Could you give me some more information?

[fbushstone]

Could you explain why it'll be needed?

Pointer constraints is built directly in to the X11 protocol, but is an extension in Wayland. WINE has no control over something that is generally the compositor's job in Wayland, or X11's job in X11.

Is there a use case for this outside of video games?

While I'm unaware of any off the top of my head, games are part of the reason a lot of people use their computers so much, so in my opinion this is functionality that should be included in dwl OOTB.

Yeah, that is another point, dwm is limited to 2000 lines while our limit is 22001, which is obviously not on par with dwm

dwm doesn't seem to really pay attention to that limit, as they have 2,775 SLOC. dwl is also way over 2200, which one could argue is a poor limit for a Wayland compositor- which has to do much more work than an X11 WM to be usable. Still, even if the patch makes it into dwl, dwl+the patch+wlroots still has a fraction of the C lines of code that dwm+Xorg has.

[pm4rcin]

dwm doesn't seem to really pay attention to that limit, as they have 2,775 SLOC.

It's about Source Lines Of Code not total lines including empty ones and comments. Just try cloc on clean dwm.c and see for yourself. ;)

[ghost]

I'm sorry if this sounds ignorant, but can't this be done with a patch? Or is it something that is required to be done inside the "dwl.c" file?

[fauxmight]

@aliteral There are two existing patches that implement pointer constraints. Both are available on the wiki and discussed at several points in this issue.

Or is it something that is required to be done inside the "dwl.c" file?

dwl patches do modify dwl.c

EDIT: You are welcome to use the patches. Many people do. The discussion here (and elsewhere) is whether the matter is substantial enough to warrant a rather large code addition violating dwl's stated intention to stay in line with dwm's 2000 SLOC limit, to which dwl no longer exactly holds.

[ghost]

@aliteral There are two existing patches that implement pointer constraints. Both are available on the wiki and discussed at several points in this issue.

Or is it something that is required to be done inside the "dwl.c" file?

dwl patches do modify dwl.c

EDIT: You are welcome to use the patches. Many people do. The discussion here (and elsewhere) is whether the matter is substantial enough to warrant a rather large code addition violating dwl's stated intention to stay in line with dwm's 2000 SLOC limit, to which dwl no longer exactly holds.

I understand. If my opinion is useful on the matter, I do not see the need to get it on the main, even if the stated limit of SLOC has been surpassed. If it's not something that breaks user experience across the board, and there are patches that do the trick, then it doesn't make much sense. But, if it's something that would benefit a large majority of users and it's really an improvement (like it is something that occurs in most Wayland compositors, is useful, makes the experience better for the user and for the devs), then maybe is worth considering. The SLOC limit should be more than a guideline than an strict rule, IMHO.

[mewkl]

Maybe it's just me but I think it's weird to have so many includes, defines, enums, typedefs and functions at the start of the dwl.c file, only to find out there is a really nice and small main function at the bottom. (Please don't tell me this is to reduce #include lines...)

It could just be put in (a) separate file(s) and included with 1 or a couple lines to keep things visually more organised in my opinion. Would be more clear for someone new when opening the program and trying to understand it. I think the goto statements are used in a really cool and clean way though.

You can keep SLOC the same if you simply program it as a module with for example the pre-processor. Just disable it by default and the effective sloc of the source of the binary is still the same as it was before.

If this is done instead of patches for core features such as pointer constraints it also has the benefit that it should never randomly break, reducing double administration of code. It's more efficient (and social I think). (Yes in my opinion pointer constraints are a core feature, with a portability viewpoint)

The whole reason for the lines limit is having clean code isn't it? I don't think patching in general is a clean method, because of the double administration required. The developers of this repo don't see any errors when changing the code, that could maybe break a lot of patches.

If just that last point could be fixed, it could be perfect! Maybe nothing else needs to happen.

I don't think this is the case, but if the reason isn't having clean and understandable code, but having a more efficient binary, then SLOC isn't even a good way to measure that. Looking at the assembly instead of the source code is better then.

[mpolitzer]

pointerConstraints did not work for me. It looks like it's missing the pixman parts that's in sway-pointer-constraints

@czettnersandor I've tried the patch before and it still seemed to work. Could you give me some more information?

Works for me. It also had the advantage of not warping the pointer when toggling relative mode. Tested with this code:

#include<SDL2/SDL.h>

int main()
{
    if (SDL_Init(SDL_INIT_VIDEO | SDL_INIT_EVENTS) < 0) {
        printf("Error SDL2 Initialization : %s\n", SDL_GetError());
        return 1;
    }

    SDL_Window* window = SDL_CreateWindow
        ("First program"
        ,SDL_WINDOWPOS_CENTERED
        ,SDL_WINDOWPOS_CENTERED
        ,800
        ,600
        ,SDL_WINDOW_OPENGL);
    SDL_Renderer* renderer = SDL_CreateRenderer
        (window
        ,-1
        ,SDL_RENDERER_ACCELERATED);
    if (window == NULL) {
        printf("Failed to create window: %s\n", SDL_GetError());
        return 1;
    }

    SDL_RenderClear(renderer);
    SDL_RenderPresent(renderer);

    for (;;) {
        SDL_Event e;
        if (SDL_WaitEvent(&e)) {
            if (e.type == SDL_QUIT) {
                break;
            }
            if (e.type == SDL_KEYDOWN && e.key.keysym.sym == 'c') {
                printf("%d\n", e.key.keysym.sym);
                SDL_SetRelativeMouseMode(SDL_TRUE);
            }
            if (e.type == SDL_KEYDOWN && e.key.keysym.sym == 'u') {
                printf("%d\n", e.key.keysym.sym);
                SDL_SetRelativeMouseMode(SDL_FALSE);
            }
        }

    }
}