Re: [hackers] Re: [dwm][PATCH v3] manage: For isfloating/oldstate check/set, ensure trans client actually exists

From: Hiltjo Posthuma <hiltjo_AT_codemadness.org>
Date: Sun, 13 Mar 2022 17:35:25 +0100

On Fri, Mar 11, 2022 at 11:25:18AM -0500, Miles Alan wrote:
> On Fri, Mar 11, 2022, at 9:11 AM, sir fish wrote:
> > Miles Alan <hackers_AT_suckless.org> wrote:
> >> On Mon, Feb 21, 2022, at 1:10 AM, m_AT_milesalan.com wrote:
> >> > From: Miles Alan <m_AT_milesalan.com>
> >> >
> >> > In certain instances trans may be set to a window that doesn't actually
> >> > map to a client via wintoclient; in this case it doesn't make sense
> >> > to set isfloating/oldstate since trans is essentially invalid in that
> >> > case / correlates to the above condition check where trans is set /
> >> > XGetTransientForHint is called.
> >> > ---
> >> > dwm.c | 2 +-
> >> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >> >
> >> > diff --git a/dwm.c b/dwm.c
> >> > index a96f33c..254ebf2 100644
> >> > --- a/dwm.c
> >> > +++ b/dwm.c
> >> > _AT_@ -1063,7 +1063,7 @@ manage(Window w, XWindowAttributes *wa)
> >> > XSelectInput(dpy, w,
> >> > EnterWindowMask|FocusChangeMask|PropertyChangeMask|StructureNotifyMask);
> >> > grabbuttons(c, 0);
> >> > if (!c->isfloating)
> >> > - c->isfloating = c->oldstate = trans != None || c->isfixed;
> >> > + c->isfloating = c->oldstate = t || c->isfixed;
> >> > if (c->isfloating)
> >> > XRaiseWindow(dpy, c->win);
> >> > attach(c);
> >> > --
> >> > 2.34.1
> >>
> >> You can test this patch with SDL applications which hit this edge case
> >> by default. Logic in dwm currently forces windows to become floating in
> >> this edge case.
> >>
> >> For example with the SDL games "rue" and "cdogs_sdl" on alpine, in i3wm
> >> these applications open as tiled windows by default as they should; while
> >> in dwm they open as floating windows due to this bug.
> >>
> >> This patch resolves that and SDL applications open as tiled with default
> >> flags as they should.
> >>
> >
> > Hi,
> >
> > I tried this patch on a freshly-cloned dwm, but it doesn't open
> > teeworlds, xonotic, supertuxcart games as tiled. It does, hover, open
> > the game cdogs as tiled. I couldn't try with the game rue as it wasn't
> > available on Void.
> >
> > --
> > srfsh
>
> Indeed there are certain SDL games which will still launch as floating
> with this patch (that is *if* the SDL_WINDOW_RESIZABLE flag is not set,
> in which case dwm considers the window fixed). Whether windows set with
> fixed dimensions should be floating or tiled by default is another matter.
>
> Specifically, this patch corrects the scenario where SDL_WINDOW_RESIZABLE
> *is* set and then dwm incorrectly floats the window. This is due to the
> logic that incorrectly uses the WM_TRANSIENT_FOR hint but doesn't check
> if the transient window is managed. By default SDL sets WM_TRANSIENT_FOR
> to the root window, see:
> https://github.com/libsdl-org/SDL/blob/85e6500065bbe37e9131c0ff9cd7e5af6d256730/src/video/x11/SDL_x11window.c#L364
>
> A base test case is below. In dwm currently, this window will float. With
> this patch applied, the window becomes tiled as it should. E.g. SDL
> applications which are resizable should be tiled.
>
> //gcc -lX11 -o foo foo.c -lSDL2
> #include <SDL2/SDL.h>
> #include <stdio.h>
>
> int main(int argc, char* args[]) {
> SDL_Init(SDL_INIT_VIDEO);
> SDL_Window* window = SDL_CreateWindow("test",
> SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED,
> 500, 500,
> SDL_WINDOW_SHOWN | SDL_WINDOW_RESIZABLE
> );
> SDL_Surface* s = SDL_GetWindowSurface(window);
> SDL_Rect r = { .x = 0, .y = 0 };
> SDL_FillRect(s, NULL, SDL_MapRGB(s->format, 255, 0, 0));
> SDL_UpdateWindowSurface(window);
> SDL_Delay(2000);
> SDL_DestroyWindow(window);
> SDL_Quit();
> return 0;
> }
>

Hi Miles,

I tested and pushed the patch. I could not find any issues with it.

If someone finds a regression with some (SDL) program, please let me know.

Thanks,

-- 
Kind regards,
Hiltjo
Received on Sun Mar 13 2022 - 17:35:25 CET

This archive was generated by hypermail 2.3.0 : Sun Mar 13 2022 - 17:36:36 CET