Linux: assert creating Desktop with either of 2 connected monitors if for separate screens

Linux
1

Hi there, we’re getting an assert during the sanity checking of the displays within juce::Displays created for the desktop.
This is occurring in a particular, fairly uncommon setup - its on Linux when xrandr is being used and there are 2 monitors each being used for a separate screen as configured in the X11 conf file.
Rather that tweak/create a demo program to illustrate, hopefully my explanation of the issue and suggested fix will prove enough…?
The code in Displays::findDisplays in juce_linux_X11_Windowing.cpp is not discounting monitors connected to a different desktop (I think that’s the correct terminology in JUCE land).
The code lower down when not using xrandr discounts these with its call to GetXProperty and its this check that is missing:

        for (int i = 0; i < numMonitors; ++i)
        {
            GetXProperty prop (display, RootWindow (display, i), hints, 0, 4, false, XA_CARDINAL);

            if (prop.success && prop.actualType == XA_CARDINAL && prop.actualFormat == 32 && prop.numItems == 4)

Without this check in the xrandr monitor loop the display for the monitor that is not part of this Desktop is added to its displays and then the second display doesn’t abutt the first/root one (they both have x,y of 0,0) so it has no parent set so it asserts.

Any chance this could be done…?

2

I’m not sure I quite understand what’s going on here. Are you saying that the check for _NET_WORKAREA property here is doing the correct thing? Is that property not set for the windows connected to a different desktop?

3

Yes that’s right. So if the same check can be done in the xrandr loop earlier in the method as well it will prevent the other monitor being erroneously added there too, and the assert that follows after due to them not have window coordinates that butt up.

4

OK, I’ve pushed a fix here:

https://github.com/WeAreROLI/JUCE/commit/1c033e410b84cbf76eec4e35cef2456cff455d52

5

Great that fixed the original problem I saw, but on further investigation this is only part of the problem and it results in the incorrect monitor being reported as the display for the second screen.

As far as I can tell this is down to being able to handle more than a single virtual screen on a x server display mapped multiple monitors.

With a default xorg configuration with 2 monitors connected these are mapped to the same virtual screen (:0.1) and in this case all is fine. The JUCE displays created belong to display 0 screen 1 and butt up to each other and the geometry is over both their areas.

However, if you have a bespoke xorg configuration where each monitor is mapped to its own screen as we do (so each can have a specfic gpu associated with it) then you have a :0.0 and :0.1.

The original code was erroneously adding the 2 monitors (aka outputs) to any display made (whether it was for screen 0 or screen 1). Then it asserted later as previously mentioned.

The fix we have sorted here means that first monitor/output geometry gets added for the display for both screen 0 (correctly) and screen 1 (incorrectly).

I have a fix for this when using xrandr:

--- a/modules/juce_gui_basics/native/juce_linux_X11_Windowing.cpp
+++ b/modules/juce_gui_basics/native/juce_linux_X11_Windowing.cpp
@@ -3439,60 +3439,56 @@ void Displays::findDisplays (float masterScale)
             {
                 auto& xrandr = XRandrWrapper::getInstance();
 
-                auto numMonitors = ScreenCount (display);
-                auto mainDisplay = xrandr.getOutputPrimary (display, RootWindow (display, 0));
-
-                for (int i = 0; i < numMonitors; ++i)
+                auto rootWindow  = RootWindow(display, DefaultScreen(display));
+                auto mainDisplay = xrandr.getOutputPrimary(display, rootWindow);
+                if (auto* screens = xrandr.getScreenResources (display, rootWindow))
                 {
-                    if (auto* screens = xrandr.getScreenResources (display, RootWindow (display, i)))
+                    for (int j = 0; j < screens->noutput; ++j)
                     {
-                        for (int j = 0; j < screens->noutput; ++j)
+                        if (screens->outputs[j])
                         {
-                            if (screens->outputs[j])
-                            {
-                                // Xrandr on the raspberry pi fails to determine the main display (mainDisplay == 0)!
-                                // Detect this edge case and make the first found display the main display
-                                if (! mainDisplay)
-                                    mainDisplay = screens->outputs[j];
+                            // Xrandr on the raspberry pi fails to determine the main display (mainDisplay == 0)!
+                            // Detect this edge case and make the first found display the main display
+                            if (! mainDisplay)
+                                mainDisplay = screens->outputs[j];
 
-                                if (auto* output = xrandr.getOutputInfo (display, screens, screens->outputs[j]))
+                            if (auto* output = xrandr.getOutputInfo (display, screens, screens->outputs[j]))
+                            {
+                                if (output->crtc)
                                 {
-                                    if (output->crtc)
+                                    if (auto* crtc = xrandr.getCrtcInfo (display, screens, output->crtc))
                                     {
-                                        if (auto* crtc = xrandr.getCrtcInfo (display, screens, output->crtc))
-                                        {
-                                            Display d;
-                                            d.totalArea = Rectangle<int> (crtc->x, crtc->y,
-                                                                          (int) crtc->width, (int) crtc->height);
-                                            d.isMain = (mainDisplay == screens->outputs[j]) && (i == 0);
-                                            d.dpi = getDisplayDPI (display, 0);
-
-                                            // The raspberry pi returns a zero sized display, so we need to guard for divide-by-zero
-                                            if (output->mm_width > 0 && output->mm_height > 0)
-                                                d.dpi = ((static_cast<double> (crtc->width)  * 25.4 * 0.5) / static_cast<double> (output->mm_width))
-                                                      + ((static_cast<double> (crtc->height) * 25.4 * 0.5) / static_cast<double> (output->mm_height));
-
-                                            double scale = getScaleForDisplay (output->name, d.dpi);
-                                            scale = (scale <= 0.1 ? 1.0 : scale);
-
-                                            d.scale = masterScale * scale;
-
-                                            if (d.isMain)
-                                                displays.insert (0, d);
-                                            else
-                                                displays.add (d);
-
-                                            xrandr.freeCrtcInfo (crtc);
-                                        }
-                                    }
+                                        Display d;
+                                        d.totalArea = Rectangle<int> (crtc->x, crtc->y,
+                                                                        (int) crtc->width, (int) crtc->height);
+                                        d.isMain = mainDisplay == screens->outputs[j];
+                                        d.dpi = getDisplayDPI (display, 0);
+
+                                        // The raspberry pi returns a zero sized display, so we need to guard for divide-by-zero
+                                        if (output->mm_width > 0 && output->mm_height > 0)
+                                            d.dpi = ((static_cast<double> (crtc->width)  * 25.4 * 0.5) / static_cast<double> (output->mm_width))
+                                                    + ((static_cast<double> (crtc->height) * 25.4 * 0.5) / static_cast<double> (output->mm_height));
 
-                                    xrandr.freeOutputInfo (output);
+                                        double scale = getScaleForDisplay (output->name, d.dpi);
+                                        scale = (scale <= 0.1 ? 1.0 : scale);
+
+                                        d.scale = masterScale * scale;
+
+                                        if (d.isMain)
+                                            displays.insert (0, d);
+                                        else
+                                            displays.add (d);
+
+                                        xrandr.freeCrtcInfo (crtc);
+                                    }
                                 }
+
+                                xrandr.freeOutputInfo (output);
                             }
                         }
-
-                        xrandr.freeScreenResources (screens);
                     }
+
+                    xrandr.freeScreenResources (screens);
                 }
             }
         }

I am still investigating whether this is possible if displays are set up using the other approaches in findDisplays. If not then perhaps separate screen configurations won’t be supportable without xrandr.

6

Following up on my last comment I cannot get the call:

GetXProperty prop(display, RootWindow(display, i), hints, 0, 4, false, XA_CARDINAL)

to ever work for the second screen, despite fiddling. The second argument for RootWindow should be the screen not monitor but it seems to fail when run on display :1.1 when the correct screen number 1 is passed in or RootWindow(display, CurrentDisplay(display)) as is correctly used elsewhere in juce_linux_X11_Windowing.cpp.

7

tl;dr alert!

Just to try and sum up the changes to handle multiple screens with multiple monitors arrangement correctly as well as the just single virtual screen X setup, @ed95, I would suggest that apart from my further modification suggested above to the push you made, I would remove the loop with the call

GetXProperty prop(display, RootWindow(display, i), hints, 0, 4, false, XA_CARDINAL);

And let it drop through and use the logic following that. This prevents it failing on the second screen.

There are still some inconsistencies: if xrandr or xinerama is used then the single virtual screen over 2 monitors is considered 2 JUCE displays, if they are not used its a single display with geometry of the whole virtual screen.

e.g. to hopefully clarify, this is what I get with a 1920x1080 and a 2560x1440 monitor for the different screen set-ups and the different methods for creating the JUCE displays:
(switching to preformatted text to keep idents):

App calling Displays::findDisplays with both monitors on same screen, default X configuration when DISPLAY=:1.0 
    xrandr - if enabled  
        0. 0,0 1920 x 1080
        1. 1920,0 2560 x 1440
    xinerama - if enabled
        0. 0,0 1920 x 1080
        1. 1920,0 2560 x 1440
    loop with GetXProperty prop(display, RootWindow(display, i).... call
        0. 0,0 4480 x 1440
    loop following loop with GetXProperty prop(display, RootWindow(display, i).... call
        0. 0,0 4480 x 1440
N.B. xrandr and xinerama returns the 2 physical displays, the others return the virtual screen geometries.

App calling Displays::findDisplays with one monitors on one screen, one on the other, on screen 0, when DISPLAY=:1.0 
    xrandr - if enabled  
        0. 0,0 1920 x 1080** 
    xinerama - if enabled
        Call to XineramaQueryDisplays doesn’t return anything
    loop with GetXProperty prop(display, RootWindow(display, i).... call
        0. 0,0 1920 x 1080
    loop following loop with GetXProperty prop(display, RootWindow(display, i).... call
        0. 0,0 1920 x 1080

App calling Displays::findDisplays with one monitors on one screen, one on the other, on screen 1 when DISPLAY=:1.1 
    xrandr - if enabled  
        0. 0,0 2560 x 1440*
    xinerama - if enabled
        Call to XineramaQueryDisplays doesn’t return anything
    loop with GetXProperty prop(display, RootWindow(display, i).... call
        0. 0,0 1920 x 1080***
    loop following loop with GetXProperty prop(display, RootWindow(display, i).... call
        0. 0,0 2560 x 1440

* before any changes this was returning 2 displays each 0,0 1920 x 1080.  Correct result should be a single display 0,0 1920 x 1080
** before any changes this was returning 2 displays each 0,0 1920 x 1080.  After the initial change this was return 0,0 1920 x 1080.  Correct result should be a single display 0,0 2560 x 1440
*** this is wrong, hence suggestion this loop should be removed.
1 Like
8

Nice efforts!