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: Mon, 14 Mar 2022 21:16:45 -0400

On Sun, Mar 13, 2022, at 12:35 PM, Hiltjo Posthuma wrote:
> 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

Thanks for applying.

Sounds good - from my view this logic makes alot more sense then the
way it was written beforehand and solves the issue for SDL apps. But
yes let's watch for regressions if anyone spots anything.

Miles
Received on Tue Mar 15 2022 - 02:16:45 CET

This archive was generated by hypermail 2.3.0 : Tue Mar 15 2022 - 03:00:38 CET