Re: RFC: ctwm enhancement for multiple monitors

Le 10/04/2023 à 18:16, Quentin Barnes a écrit :
> On Mon, Apr 10, 2023 at 12:31:49PM +0200, Maxime Soulé wrote:
>> Le 08/04/2023 à 22:31, Quentin Barnes a écrit :
>>> On Sat, Apr 08, 2023 at 07:09:54PM +0200, Maxime Soulé wrote:
> [...]
>> Another thing that comes in mind: don't forget that these new macros are
>> bound to Xrandr initial detection. If the layout has later been overridden
>> using MonitorLayout config variable, the macros won't reflect the user
>> layout.
>   
> This is what I mentioned about boundary cases in my initial post.
> Being a noob to window manager code, I'm sure there are all sorts of
> boundary cases I'm not aware of coming from existing ctwm features
> and xrandr behavior.
> 
> One area I didn't test was what happened if ctwm isn't configured
> with xrandr.  Is any special handling needed?

If Xrandr is not available, or cannot be used because its version is too 
low, or any other reason XrandrNewLayout 
(https://github.com/fullermd/ctwm-mirror/blob/master/xrandr.c#L22) 
returns NULL, the layout is initialized with root coordinates: 
https://github.com/fullermd/ctwm-mirror/blob/master/ctwm_main.c#L455-L457

> 
> As far as I can tell, it looks like when there's no xrandr compiled
> in, appropriate monitor values are just substituted, so things
> should just work as-is, right?

I added a comment to your commit to handle this specific case, for which 
there is no monitor names at all.

> 
> 
> On redundant checks in my patch, after reading more code, it looks
> like I could remove the "if (RLayoutNumMonitors(Scr->Layout) > 0) {
> ... }" check.
> https://github.com/qbarnes/ctwm-mirror/blob/e462a9e8ad1677fce6f01176e006f07e9b37278b/parse_m4.c#L271
> 
> Prior to main() calling LoadTwmrc(), which in turn runs my patch,
> the previous code in main() has already checked to ensure that
> RLayoutNumMonitors(Scr->Layout) will return a value greater than 0,
> correct?

Yes.

Just as a note, ctwm can be launched with --cfgchk and no X available, 
to check its configuration and leave:

DISPLAY= ctwm --cfgchk .ctwmrc

for example. In this case, we fall into the case where Xrandr is not 
used, so the layout defaults to root coordinates, so always one monitor.

> 
> I'm still not sure how "Scr->Layout" all works with multiple screens
> and when it can change, so I had left it in there the patch when I
> first coded it.

Once set at startup using Xrandr info, Scr->Layout can only change using 
the MonitorLayout variable during the parsing of the configuration, 
never after. Your code should be called before any MonitoLayout use, so 
it is OK. As ctwm boots very quickly, it has been decided to restart it 
when monitors layout changes instead of handling the change without 
restarting it. It is a way easier to handle.

> 
>> Perhaps XRANDR_ (or XRR_) prefix could eliminate a possible confusion? I
>> don't use MonitorLayout myself, but for those using it, it could be
>> misleading.
> 
> Ah!  Excellent point!  I hadn't thought about how the macros' values
> are the ones that are returned by xrandr calls (or substituted values
> if no xrandr), and not necessarily the monitors' values after being
> fully configured by the user.
> 
> I think "XRR_" is a good prefix.  With not abbreviating "MONITOR",
> the macro names are already long enough!

I agree with XRR_ prefix, but I have no opinion about removing or not 
the following MONITOR_ word.

++

Max.