Issues latest dev branch - getMainDisplay()

I just tried the latest dev but my application keeps hitting an asset here:

It appears to be triggered when I call centreWithSize() on a DocumentWindow. If I take that out it gets a little further before it’s hit again.

Of course the Projucer and other applications I’ve tried all work fine. FWIW, my app works fine with the latest Master branch.

All methods, that affect Components, especially placement and painting, have to be called on the MessageThread. So when your app calls centreWithSize(), is it possible, that this is not from the messageThread?
You should be able to get around that by calling via MessageManager::callAsync (...);

Actually, reading again, is it the ASSERT_MESSAGE_MANAGER_IS_LOCKED or the next line, that asserts?

It’s the jassert (displays.getReference(0).isMain);. Thanks I’ll take a look at the callAsync() method. It seems each time I update to a new version of JUCE I have to rewrite some of my bad code. I expect that by about 2025 my app will be completely bug free and ready for release :rofl:

Btw, I’m pretty sure centreWithSize() is being called from the message thread, but thanks for the heads up. Gives me something to look at.

Oh, in that case you are fine. I was only looking at the first assert (maybe because it was shouting in capitals at me :wink: )
To be honest, the method you linked in looks a bit like a placeholder to me, that survived from an early stage, unless there is reason for the assumption, that the first display should be the main display.

I assume it should look like:

const Displays::Display& Displays::getMainDisplay() const noexcept
{
    ASSERT_MESSAGE_MANAGER_IS_LOCKED
    for (auto& display : displays)
        if (display.isMain)
            return display;

    return displays.getReference(0);
}

But I leave the final answer to @ed95 or @t0m, they are the experts there

Yeah, I think you’re right. Has last minute Friday evening commit written all over it :rofl: Your modified method should work fine for the time being. Thanks for that. Much appreciated.

1 Like

What platform are you on? The method which calculates the displays should swap the main one so that’s it’s always first, but I can’t see any harm in making that method a bit more flexible.

Hi @ed95. I tried on Linux earlier. And just now I tried on Windows where it all works fine.

This issue is still present when I build my apps with 5.4.1 on Linux? I’m not sure why I’m hitting that assert. As much as I enjoy the irony of reading the //no main display! comment above the assert, on my main display, I’m at a loss to explain it. Removing the jassertfalse lets me get past it, but I’m worried it will come back to bite me later. Not a biggy, enjoy ADC, and let me know if you have any ideas as to why it might be happening.

That’s strange. I did make this change a few weeks ago which made the method a bit more forgiving if the displays are added in the wrong order:

But if you’re still hitting the assertion then that implies that no main display is being added at all. Which Linux distro are you using? I’ve tested it on Ubuntu 18.04 but can’t reproduce.

Thanks Ed, I’m using 17.04 but I’ll be updating to 18.04 which will probably resolve the issue.

I have the same problem. I use fedora 17 and fedora 22. The problem on my machine (fedora 22) is that the test for output->crtc fails on the screen that is the main display.

So this is what happens:

                auto mainDisplay = xrandr.getOutputPrimary (display, RootWindow (display, 0));
                // Note: mainDisplay is not NULL.
                ...
                for (int i = 0; i < numMonitors; ++i)
                {
                    if (auto* screens = xrandr.getScreenResources (display, RootWindow (display, i)))
                    {
                        for (int j = 0; j < screens->noutput; ++j)
                        {
                          if (screens->outputs[j])
                            {
                                ...
                                if (auto* output = xrandr.getOutputInfo (display, screens, screens->outputs[j]))
                                {
                                    if (output->crtc) // <--- This test fails for the output for mainDisplay
                                    {
                                       ...
                                        d.isMain = true;
                                        ...
                                    }
                                   ...
                                } 
                               ...
                         }
                         ...
                   }

I don’t know the details about what happens here though. Perhaps I should just comment out the output->crtc test?

Oh, and the code I referred to above is from juce_gui_basics/native/juce_linux_X11_Windowing.cpp

This fix might be better than nothing:

--- a/pluginhost/JuceLibraryCode/modules/juce_gui_basics/native/juce_linux_X11_Windowing.cpp
+++ b/pluginhost/JuceLibraryCode/modules/juce_gui_basics/native/juce_linux_X11_Windowing.cpp
@@ -3440,7 +3440,8 @@ void Displays::findDisplays (float masterScale)
                 auto& xrandr = XRandrWrapper::getInstance();
 
                 auto numMonitors = ScreenCount (display);
-                auto mainDisplay = xrandr.getOutputPrimary (display, RootWindow (display, 0));
+                auto xrandrMainDisplay = xrandr.getOutputPrimary (display, RootWindow (display, 0));
+                auto mainDisplay = xrandrMainDisplay;
 
                 for (int i = 0; i < numMonitors; ++i)
                 {
@@ -3494,6 +3495,13 @@ void Displays::findDisplays (float masterScale)
                         xrandr.freeScreenResources (screens);
                     }
                 }
+
+                // xrandr.getOutputPrimary sometimes returns a display without crtc.
+                if (!displays.isEmpty() && !displays.getReference(0).isMain)
+                {
+                  jassert (xrandrMainDisplay);
+                  displays.getReference(0).isMain = true;
+                }
             }
         }

If we skip the code that’s enclosed in the if (output->crtc) then the display bounds and scale won’t be calculated correctly for the display.

Can you step into XRandrWrapper::getOutputInfo() and see if it’s returning nullptr because the function pointer isn’t loaded correctly ie. it returns here.

Hi, XRandrWrapper::getOutputInfo() does not return NULL, but output->crtc is 0, and xrandr.getCrtcInfo also returns NULL.

With

--- a/pluginhost/JuceLibraryCode/modules/juce_gui_basics/native/juce_linux_X11_Windowing.cpp
+++ b/pluginhost/JuceLibraryCode/modules/juce_gui_basics/native/juce_linux_X11_Windowing.cpp
@@ -3447,10 +3447,14 @@ void Displays::findDisplays (float masterScale)
                 {
                     if (auto* screens = xrandr.getScreenResources (display, RootWindow (display, i)))
                     {
+                      printf("num outputs for screen %d: %d\n", i, screens->noutput);^M
                         for (int j = 0; j < screens->noutput; ++j)
-                        {
+                        {                          ^M
                             if (screens->outputs[j])
                             {
+                                if(mainDisplay==screens->outputs[j])^M
+                                  printf("   mainDisplay is screen %d / output %d\n", i, j);^M
+                                ^M
                                 // 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)
@@ -3462,6 +3466,7 @@ void Displays::findDisplays (float masterScale)
                                     {
                                         if (auto* crtc = xrandr.getCrtcInfo (display, screens, output->crtc))
                                         {
+                                          printf("Has crtc. monitor: %d. output: %d\n", i, j);^M
                                             Display d;
                                             d.totalArea = Rectangle<int> (crtc->x, crtc->y,
                                                                           (int) crtc->width, (int) crtc->height);

I get this output:

num outputs for screen 0: 4
   mainDisplay is screen 0 / output 0
Has crtc. monitor: 0. output: 1

So xrandr reports that I have one monitor, 4 outputs, and only the second output has crtc info. mainDisplay is reported to be the first output (which doesn’t have crtc info).

OK, perhaps we need to make the conditions for Display::isMain a little more lax. If you change line 3467 from:

d.isMain = (mainDisplay == screens->outputs[j]) && (i == 0); to d.isMain = i == 0;

does this solve the issue?

Yes, that would solve the issue, at least for my computer as it is setup right now. But wouldn’t you risk having more than one isMain monitor this way? I don’t understand the thought behind this…

Wouldn’t it be better to do the normal iteration first, and afterwards, only if no display has “isMain==1”, then set isMain to 1 for the first display?

By the way, did you see my post above starting with the text " This fix might be better than nothing:"? I think that would be the appropriate fix.