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

From: Miles Alan <m_AT_milesalan.com>
Date: Fri, 11 Mar 2022 11:25:18 -0500

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;
}
Received on Fri Mar 11 2022 - 17:25:18 CET

This archive was generated by hypermail 2.3.0 : Fri Mar 11 2022 - 17:36:37 CET