lv2 / pugl

A minimal portable API for embeddable GUIs
https://gitlab.com/lv2/pugl/
ISC License
174 stars 34 forks source link

Several window managers break when PBaseSize is set #118

Closed falkTX closed 2 months ago

falkTX commented 10 months ago

I think pugl use of PBaseSize for meaning "default size" is wrong. Under fluxbox this value is used on top of the minimum size hint, basically acting as another minimum size, making it so that changing this hint prevents the user from shrinking the window.

I cannot find any docs for what this flag is exactly supposed to mean, except "program specified base for incrementing".

Removing it fixes up the behaviour under fluxbox.

drobilla commented 10 months ago

Well, you'll need to find something else that does that under every other environment I guess. As far as I know, this is what does the expected thing on everything except fluxbox. There is PSize and USize, but IIRC those don't work.

drobilla commented 10 months ago

It looks like fluxbox sets the minimum size to the base size if the minimum size isn't set: https://github.com/fluxbox/fluxbox/blob/0f95d62b1b1add8ee7327305db7d372010fdb2f4/src/WindowState.cc#L119

Seems like you need to set a minimum, then. I don't see anything Pugl could do about this in general.

drobilla commented 10 months ago

Judging by https://tronche.com/gui/x/icccm/sec-4.html , PResizeInc is more associated with PBaseSize, if anything. I suspect fluxbox ended up where it did because of an overly general reading of this:

If a base size is not provided, the minimum size is to be used in its place and vice versa.

It's poorly worded, but I think this is referring to the increment calculation directly above it (it's the second sentence in a two-sentence paragraph about this calculation specifically). I don't think the intent is to always replace the base size with the minimum size and vice-versa.

In any case, the client setting a minimum (that isn't larger than the default) is the workaround, and probably best practice anyway (can your view really draw something sensible at 1x1?). It could be made a hard error in Pugl easily enough, but I don't know if this is worth that.

falkTX commented 10 months ago

for the application in question (Cardinal) I am already setting a minimum size, and this still breaks fluxbox. taking out the "base size" then the minimum works as expected.

so your assumptions are not quite correct. please try with a minimal test case if possible and confirm, basically:

  1. set minimum size to something obviously small
  2. set default size to something bigger
  3. run it in fluxbox, and notice how you cannot resize it smaller than default.
drobilla commented 10 months ago

Yep, it seems fluxbox is broken when the PBaseSize is set.

Double-checking in the handiest pedestrian Linux environment I have around, Mint (so Cinnamon), it works as expected: pugl_cairo_demo starts with the default size of 512x512, and can be shrunk down to 256x256. IIRC it also works as expected in KDE and GNOME. PBaseSize is also used the same way (initial/default) in LV2 wrapping.

Seems like a fluxbox bug to me. It acts the same way if you do set a resize increment (although then they are supported). It looks like some old X11 programs (some in xorg itself, xev, xcalc, and such) use the PSize as a sort of "default" (initial anyway), and this works in fluxbox.

falkTX commented 10 months ago

Yeah on my KDE desktop this works fine too, @dromer was the one that reported to me that this broke with a recent pugl update (we didnt make use of "default" size much before).

It is unlikely that fluxbox is going to change at this point, and other toolkits behave as expected, so while we can argue it is a bug in fluxbox I think it is still worth having a work-around in pugl. X11 is considered legacy now anyways, so I dont expect desktops/WMs to change at this point.

drobilla commented 10 months ago

Setting PSize instead works at least for the case of initial display of pugl_cairo_demo in both fluxbox and Cinnamon. That only makes sense during initial display, though, and when you happen to actually get your default.

I don't see a way of working around a window manager that breaks when you provide a default size with PBaseSize without depriving other window managers of that information. Just crudely doing that replacement in x11.c would break a bunch of embedding scenarios, too.

Everyone except fluxbox seems to be treating PBaseSize as a preferred size (we usually say "default", ICCM says "preferred"). IIRC, if you idealistically read the spec, it seems like there's a system of program and user specified sizes with USSize, USPosition, PSize, and PPosition, but I don't think any of that is actually implemented coherently / at all in practice (or I've never gotten them to do anything sensible anyway).

falkTX commented 10 months ago

FYI my workaround for now is to remove the default size hint after the window is created, so future resizes and size hint updates wont contain that info. Sounds like we should investigate what other toolkits are doing, xwininfo can help here.

drobilla commented 10 months ago

Oh, I missed a later part: https://github.com/fluxbox/fluxbox/blob/0f95d62b1b1add8ee7327305db7d372010fdb2f4/src/WindowState.cc#L159

So yeah, fluxbox literally implements "make the min size at least the base size".

drobilla commented 10 months ago

Sounds like we should investigate what other toolkits are doing

Gdk2 used to expose a concept of base size directly to it, but Gtk no longer seems to use it: https://github.com/GNOME/gtk/blob/5ca65f04fe629ec647a27c76283c741c54029e68/gdk/x11/gdksurface-x11.c#L2379

Qt seems to only use it when specifying increments (maybe that's always, I don't know): https://github.com/qt/qt/blob/0a2f2382541424726168804be2c90b91381608c6/src/gui/kernel/qwidget_x11.cpp#L2325

gedit seems to just not set any of these at all. I suspect that's a somewhat common modern practice that avoids this problem: just don't set size hints at all (but set the size of your window when you create it). I don't know if or how Pugl could do this, the API currently (and intentionally) forces you to set things up with hints initially, and those hints are shared directly with the window manager.

It could just... not share the PBaseSize for a top-level program, I guess, the window will be created at that size anyway. I don't know if there's window managers with features based on the base size though.

drobilla commented 10 months ago

Neither qt5ct nor vlc set it. Just a minimum, and user-supplied ones that look like nonsense, but that might just be bspwm.

So, I imagine you can get away with just not setting it. I wouldn't want to do that in pugl without understanding the implications better, though.

drobilla commented 10 months ago

dwm (and so probably a million of its forks) seems to do a similar thing (in a duplicated branch no less): https://github.com/chadcat7/dwm/blob/5edbae0af70643c5db923a582583a8694876a83d/dwm.c#L2508

I guess if Gtk and Qt things don't set it, then it's unlikely to be missed.

drobilla commented 10 months ago

Reopening as this is apparently an issue with more than one window manager, even though the behaviour makes no sense and seems to be mainly a lazy copy/paste bug that has spread through "minimal" window manager projects.

drobilla commented 2 months ago

Well, this sucks, but nothing much pugl can do about it as far as I can tell: 1438617