Multi monitor support

[Foaly]

Good evening everbody!
I'm starting another attempt at the good old multi monitor setup issue But this time I am bringing a fully functional reference implementation for windows with me! You can check it out here: https://github.com/Foaly/SFML/tree/multi-monitor-windows (commit d3bfdbfb)
This weekend I dug through the old discussions and the MSDN and implemented a working multi monitor API for windows. It follows the ideas brought up inprevious threads. I designed the API so that it is fully backwards compatiple with the current SFML master. The usage looks like this:

#include <SFML/Window.hpp>
#include <iostream>

int main()
{
    std::cout << "# of connected Screens: " << sf::VideoMode::getScreenCount() << std::endl;

    // display all supported fullscreen resolutions for all monitors
    for (unsigned int id = 0; id < sf::VideoMode::getScreenCount(); ++id)
    {
        std::vector<sf::VideoMode> modes = sf::VideoMode::getFullscreenModes(id);
        std::cout << "Valid resolutions for screen " << id << std::endl << std::endl;
        for(std::size_t i = 0; i < modes.size(); ++i)
        {
            sf::VideoMode mode = modes[i];
            std::cout << "Mode #" << i << ": "
                      << mode.width << "x" << mode.height << " - "
                      << mode.bitsPerPixel << " bpp" << std::endl;
        }
        std::cout << std::endl;
    }

    // display the desktop resolution for all screens
    for (unsigned int i = 0; i < sf::VideoMode::getScreenCount(); ++i)
    {
        sf::VideoMode mode = sf::VideoMode::getDesktopMode(i);
        std::cout << "Desktop mode for screen #" << i << ": "
        << mode.width << "x" << mode.height << " - "
        << mode.bitsPerPixel << " bpp" << std::endl << std::endl;
    }

    // get infos about the second screen
    sf::Screen screen = sf::VideoMode::getScreenInfo(1);
    std::wcout << "Monitor 1 name: " << screen.realName << std::endl
               << "x: " << screen.geometry.left << " y: " << screen.geometry.top << " width: " << screen.geometry.width << " height: " << screen.geometry.height << std::endl
               << "framerate: " << screen.frameRate << "Hz" << std::endl;


    // open a window on the second monitor
    sf::VideoMode videoMode(1280, 1024, 32, 1);
    sf::Window window(videoMode, "Multi Monitor Example!" /*, sf::Style::Fullscreen*/);

    // run the program as long as the window is open
    while (window.isOpen()) {
        // check all the windows events
        sf::Event event;
        while (window.pollEvent(event)) {
            // "close requested" event: we close the window
            if (event.type == sf::Event::Closed)
                window.close();

            else if (event.type == sf::Event::KeyReleased) {
                // close if escape key was pressed
                if (event.key.code == sf::Keyboard::Escape) {
                    window.close();
                }
            }
        }

        // display the windows content
        window.display();
    }

    return 0;
}

Of course this is a proof of concept rather than a polished and finished implemention. It's supposed to be a base for further discussion about the API and the implementation. I know there are some dirty hacks in there (like including sf::Rect from the graphics package, exposing platform specific details in the public API or breaking compilation on Linux/Mac... ) but I'd still love to hear feedback (both on the implementation and the API). Also the code needs testing. I tryed it with my setup (win7 + NVIDIA GTX 970 + 3 monitors) and everything works as expected, but there are loads of different configurations and the winapi is quiet complicated, so if some more people with different configuration would test this, that would be awesome!

Ok I'd love to hear improvents, comments or criticsim. Let's get this feature implemented!

edit: also just noticed 400th post yay

[dabbertorres]

I like it! Reminds me a bit of how GLFW does it.

I see 2 things I'd like to comment on:
1. I'd rename sf::Screen::frameRate to sf::Screen::refreshRate, to be a bit more semantically (is that the right usage? can never remember...) correct.

2. On that note, refresh rate is more of a video mode concept, not really a monitor concept. A monitor will define its max refresh rate, but a monitor can support more than 1 refresh rate. For instance, my monitors claim to support 60, 59, 50, 30, 29, and 25 (the last 3 being interlaced). But also support 75 Hz at 1280x1024 and below. So maybe refresh rate should be an addition to sf::VideoMode, ideally.

Now that I'm looking at it again...

3. I don't think sf::Screen should be a member/accessible through sf::VideoMode. I think you did for backwards compatibility, but that kind of feels backward. A video mode is dependent on a screen/monitor, not the other way around. That's my thoughts on it. I think it might make more sense to have sf::VideoMode through sf::Screen. For example:

std::size_t totalMonitors = sf::Screen::count();

for(std::size_t i = 0; i < totalMonitors; ++i)
{
    std::vector<sf::VideoMode> modes = sf::Screen::get(i).getFullscreenModes();
   
    for(auto& vm : modes)
    {
        std::cout << "Screen #" << i << ": " << mode.width << "x" << mode.height << " - " << mode.bitsPerPixel << " bpp @ " << mode.refreshRate << " Hz\n";
    }
}
 

That's my idea for an API at least. That would allow for less modification to sf::VideoMode as well. Minus the additional refreshRate member. sf::Screen having a "maxRefreshRate" or something might not be a bad thing. Maybe a bit redundant though.

I do think sf::VideoMode should still be accessible without sf::Screen though. That would keep things backwards compatible. Just use the primary monitor maybe?

Anyways, those are my (much more numerous than I initially thought!) thoughts as of now... I'm interested in what other people say!

If/when a final public API is decided on, I'd love to contribute, maybe with a Linux implementation?

EDIT: Downloading/compiling your fork now.

EDIT 2: Working great here! (Win 8.1, 3 monitors, GTX970, unfortunately quite similar to your config, haha.) Changed which monitor to open the window on and that works great as well.

[Foaly]

Thank you for testing and the feedback!

I like your thoughts about the API. Having sf::Screen seperate from sf::VideoMode sounds interesting. Although it might make the backwards compatibility confusing. (Being able to create video modes through sf::VideoMode and sf::Screen) But let's wait and see what other people think.

About the sf::Screen::frameRate. I agree that refreshRate is the more fitting name in this case. But I don't know if it should be kept in the struct. I initially thought you could use it to sync your rendering with the monitors refresh rate (using sf::Window::setFramerateLimit()), but of course that's what sf::Window::setVerticalSyncEnabled() is for...
Also I thought that I'm putting the monitors nativ refresh rate in there, but looking at the code again, I am actually putting the desktop modes refresh rate in. I don't know how useful that is. Maybe it could still be kept for informational purpose (maybe rename it to defaultRefreshRate).

I was able to get rid of the biggest hack. The platform specific device name is no longer exposed in the public API. I added a new priv::getDisplayDeviceFromId(unsigned int screenId) function to the windows implementation and seperated the deviceName into it's own vector. The API is now cleaner and platform-independent. Here is the new commit 86a6eb93.

I wanted to wait with the Linux implementation until we have decided on a final API, to keep the rewriting work low. I only had a quick look at the Linux code and it seems even more complicated Also I don't have a proper multi-monitor linux setup yet, so any help is appreciated.

[eXpl0it3r]

I (also) think that hijacking the sf::VideoMode is not a very good design. Starting just with the intuition of "a screen has video modes" and not "video modes have screens", it could be quite confusing to use, because people would probably try to create sf::Screen objects on their own.

As for the backwards compatibility, there are multiple ways to add things and we can also mark some functions as deprecated etc. So maybe first worry about the nice API and then how it could potentially be added.

[Foaly]

Hmm yeah the more I think about it the less confusing and better it seems to me to have a seperate sf::Screen class. So I am making a new proposal. The code API would look something like mentioned in dabbertorres post. A seperate class sf::Screen with two static methods for getting the screen count and a screen object based on its ID. A screen object would contain all the info (name, geometry, etc.) and the desktop and fullscreen modes. The VideoMode class would only be used to store the actuall video mode data (width, heigth, bpp, etc.). The VideoMode::getFullscreenModes() and VideoMode::getDesktopMode() would still return the modes to the primary screen to keep backward compatibility, but they would be marked as deprected and could be removed later.

Here is what the code would look like:

class Screen
{
public:
    static unsigned int count();

    static const Screen& get(unsigned int id);


    const std::string name;
    const FloatRect geometry;
    const unsigned int frequency;
    const std::vector<VideoMode> fullscreenModes;
    const VideoMode desktopMode;

private:
    Screen(std::string Name, FloatRect Geometry, unsigned int Frequency);
}


class VideoMode
{
public:
    VideoMode(unsigned int modeWidth, unsigned int modeHeight, unsigned int modeBitsPerPixel = 32, unsigned int screenID = 0);

    bool isValid() const;


    SFML_MARKED_AS_DEPRECATED(static VideoMode getDesktopMode(););
    SFML_MARKED_AS_DEPRECATED(static const std::vector<VideoMode>& getFullscreenModes(););


    unsigned int width;        ///< Video mode width, in pixels
    unsigned int height;       ///< Video mode height, in pixels
    unsigned int bitsPerPixel; ///< Video mode pixel depth, in bits per pixels
    unsigned int screenId;     ///< The Id of the screen this video mode is assosiated with
}

So tell me your opinion. I would love to hear some feedback! What does the rest of the team think?

[Hiura]

It looks nice! Kudos.

A few thoughts of mine:

- With Joystick, we have one class with only static member functions that takes an id as parameter. For consistency in the API, we could either have a similar pattern for sf::Screen or in the future (understand SFML 3) change the Joystick API to have a static get method. (Dunno which is best though.)

- The DPI could be added to the properties of sf::Screen.

- You use the term `geometry`. I'm just curious why. Is it because it's used in Windows API? (just curiosity here; on OS X it's `frame` but I'm fine with any of them).

- Some screen have some "widgets" (OS bar, "Dock" on OS X, ...) that take space where no window can be displayed. On OS X we have -[NSScreen visibleFrame] property for that -- would be nice to have something similar here too.

- The current API has no notification system for screen configuration change, such as scaling factor change (kind of DPI change) or virtual space reordering. This could be interesting to have in SFML as well.

[Foaly]

Thanks for your thoughts!

I was looking at the Joystick API when I wrote the new proposal. But to me it seemed cleaner to have an immutable screen object, because a screens properties don't change (except for reconfiguration, but I come to that in a minute). So using sf::Screen::get(unsigned int) gets you an object with only const members and you can't construct one yourself.
For sf::Joystick it might make sense to use the other approach, because the joystick objects are constantly updated by the joystick manager and their values change frequently. Then again it might only be a leftover from the 1.6 sf::Input class which has not been refactored properly when it was put into a seperate class. I don't know. So maybe we should take a closer look for SFML 3.

I added DPI to the class. Good idea!

I actually took the term 'geometry' from Qt I thought it was quiet fitting because it includes both the position and the size. I don't think 'frame' would be better because it is associated with lots of other things. But I'm of course open for suggestions.

Of course we can add a property that specifies the usable area. I left it out originally, because in my first draft the screen was associated with a VideoMode and used mostly for the fullscreen stuff, so it didn't make sense. Now that we have sf::Screen seperate, I'll put it back in. In Qt it's called 'availableGeometry' by the way.

I really like the idea of being able to react to hotplugging/screen resolution changes/rearrangement in virtual screen space/scaling factor changes. I think it would really improve SFML in terms of a multimedia library. You can hotplug all input devices (mouse, keyboard, joystick, sound input), so why not have it for the output devices? I already looked into it for windows and it is defintly possible. But I guess it's not that important for most of the use cases (games). It is more something for multimedia installation or multi-screen apps involving beamers and multiple screens. Also this brings up new questions. For example: Should we put screen events into the window event queue? So I guess we leave that for later and focus on getting basic multi monitor support running. Reacting to events can always be added later.

So if noone else has anything to add I would start rewriting the API/windows implementation to match the proposal. I have some time during the next two weeks that I would like to dedicate to this. Also set up a linux testing rig, so I might start the linux implementation too.

[zsbzsb]

I'll just chime in and say I agree with what eXpl0it3r suggested. It indeed makes more sense to have screens containing video modes and not the other way around (it is the way I would design the API).

One thing I would suggest however, add a flag (boolean) to the screen struct that will determine if it is the primary screen or not. Also is the Screen::geometry supposed to be the bounds of the monitor? If it is wouldn't a IntRect make more sense as the type? And I would also suggest naming it 'bounds' / 'workingBounds' or 'area' / 'workingArea' compared to 'geometry'. Or maybe I am just biased... 

[Hiura]

Then again it might only be a leftover from the 1.6 sf::Input class which has not been refactored properly when it was put into a seperate class. I don't know. So maybe we should take a closer look for SFML 3.

Yep, most probably. But that's also a nice way to deal with changes/invalidation. Actually we would have some issues if the system is reconfigured and we don't notify the user: what would happen if a screen is moved around, or unplugged? With the current API we would have a static view of the screen system when the application is started.

Should we put screen events into the window event queue? So I guess we leave that for later and focus on getting basic multi monitor support running. Reacting to events can always be added later.

Currently we have joystick events connected to the window so we could do the same for now (or leave it out for a second pass -- I'm totally fine with that) but at the same time having those notifications polled from a window feel kind of weird... They are more system-wide than window-specific.

I would also suggest naming it 'bounds' / 'workingBounds' or 'area' / 'workingArea' compared to 'geometry'. Or maybe I am just biased... 

Actually, now that you mention 'bounds'... it would be more consonant with `getGlobalBounds` than `geometry`.

[Mario]

I think screen events should be received by all windows. Input events only by the active one. (This might change current behavior? I'm not sure right now.)

[Hiura]

What I meant by system wide was a system that will notify the user even if no windows are opened.

[Mario]

Ah, I see. That could be tricky (and would be something completely new).

But if you think about it, sensors should go into something global like that as well. You can't have a set of sensors per window (assuming you could own more than one window on mobile ports).

[zsbzsb]

And continuing the logic of how inputs are global.... <joke>Mouse events are given in desktop coordinates and then converted to window coords so mouse events should be global also.</joke> Just employ KISS and handle all events in each window.

If do think about adding 'global events' add to the that fonts installed/uninstalled, power mode changes, logout/shutdown, and time changed.

https://msdn.microsoft.com/en-us/library/microsoft.win32.systemevents%28v=vs.110%29.aspx

[Foaly]

Alrighty it took a little longer than I thought, but here is the new implementation: commit 3902d95d on a new branch.
Here is my testing code:

(click to show/hide)

#include <SFML/Window.hpp>
#include <iostream>

int main()
{
    // query screen count
    std::size_t totalMonitors = sf::Screen::count();
    std::cout << "# of connected Screens: " << totalMonitors << std::endl;

    // enumerate over all screens
    for(std::size_t id = 0; id < totalMonitors; ++id)
    {
        sf::Screen screen = sf::Screen::get(id);
        const std::vector<sf::VideoMode> modes = screen.getFullscreenModes();
        std::cout << "Valid resolutions for screen " << modes[0].screenId << std::endl << std::endl;

        // fullscreen modes
        for(std::size_t i = 0; i < modes.size(); ++i)
        {
            sf::VideoMode mode = modes[i];
            std::cout << "Mode #" << i << ": "
            << mode.width << "x" << mode.height << " - "
            << mode.bitsPerPixel << " bpp" << std::endl;
        }
        std::cout << std::endl;

        // desktop mode
        sf::VideoMode mode = screen.getDesktopMode();
        std::cout << "Desktop mode for screen #" << id << ": "
        << mode.width << "x" << mode.height << " - "
        << mode.bitsPerPixel << " bpp" << std::endl << std::endl;

        // screen information
        const sf::IntRect bounds = screen.getBounds();
        const sf::IntRect workingArea = screen.getWorkingArea();
        std::wcout << "Monitor " << screen.getId() << " name: " << screen.getName() << std::endl
        << "x: " << bounds.left << " y: " << bounds.top << " width: " << bounds.width << " height: " << bounds.height << std::endl
        << "x: " << workingArea.left << " y: " << workingArea.top << " width: " << workingArea.width << " height: " << workingArea.height << std::endl
        << "refreshRate: " << screen.getRefreshRate() << "Hz"
        << " dpi: " << screen.getDpi().x << "x" << screen.getDpi().y << std::endl << std::endl << std::endl;
    }

    // backward compatibility with depreaction note
    sf::VideoMode::getDesktopMode();

    // open a fullscreen window on the second screen
    sf::VideoMode videoMode(1024, 768, 32, 1);
    sf::Window window(videoMode, "Multi Monitor Example!" , sf::Style::Fullscreen);
    window.setFramerateLimit(sf::Screen::get(videoMode.screenId).getRefreshRate());

    // run the program as long as the window is open
    while (window.isOpen()) {
        // check all the windows events
        sf::Event event;
        while (window.pollEvent(event)) {
            // "close requested" event: we close the window
            if (event.type == sf::Event::Closed)
                window.close();

            else if (event.type == sf::Event::KeyReleased) {
                // close if escape key was pressed
                if (event.key.code == sf::Keyboard::Escape) {
                    window.close();
                }
                if (event.key.code == sf::Keyboard::Return) {
                    window.create(videoMode, "Multi Monitor Example", sf::Style::Resize);
                    window.setFramerateLimit(sf::Screen::get(videoMode.screenId).getRefreshRate());
                }
            }
        }

        // display the windows content
        window.display();
    }

    return 0;
}

As you can see I had to change the API a little bit. Originally I wanted sf::Screen to be an immutable object with public const attributes (see above). Apperently there is no way to make an object like this copy-/assignable in C++. So I had to go with an object that has private members and getters. If somebody has a better suggestion I'd be glad to hear it!
So this is what the current API looks like:

class Screen
{
public:

    static std::size_t count();
    static const Screen& get(unsigned int id);


    std::wstring getName() const;
    unsigned int getId() const;
    const IntRect& getBounds() const;
    const IntRect& getWorkingArea() const;
    unsigned int getRefreshRate() const;
    Vector2u getDpi() const;
    bool isPrimary() const;
    const std::vector<VideoMode>& getFullscreenModes() const;
    const VideoMode& getDesktopMode() const;


private:

    friend class priv::ScreenImpl;

    Screen(std::wstring name, unsigned int id, IntRect bounds, IntRect workingArea, unsigned int refreshRate, Vector2u dpi, bool isPrimary, std::vector<VideoMode> fullscreenModes, VideoMode desktopMode);

    void updateId(unsigned int id);


    ////////////////////////////////////////////////////////////
    // Member data
    ////////////////////////////////////////////////////////////
    std::wstring           m_name;            ///< Name of the screen
    unsigned int           m_id;              ///< Id of the screen
    IntRect                m_bounds;          ///< Bounds of the screen in virtual screen space
    IntRect                m_workingArea;     ///< Working area of the screen in virtual screen space
    unsigned int           m_refreshRate;     ///< Refresh rate of the screen
    Vector2u               m_dpi;             ///< Dpi (dots per inch) of the screen
    bool                   m_isPrimary;       ///< Is this the primary screen?
    std::vector<VideoMode> m_fullscreenModes; ///< Supported fullscreen modes of this screen
    VideoMode              m_desktopMode;     ///< Desktop mode of the screen
};
 

I changed a couple things: Reworked the code into the new design, changed the implementation quiet a bit (added dpi and workingArea for example), added a SFML_DEPRECATED macro, moved <SFML/Graphics/Rect.hpp> to <SFML/System/Rect.hpp>, removed the obsolete <Window/Win32/VideoModeImpl.hpp> and some more stuff.
I found the names bounds and workingArea most fitting, so I used those

Since quiet a lot changes I'd be happy when some people would test my code and give me some feedback. (For example if the dpi is working on pre win8). It would also be nice if someone from the team could take a look at the implementation and tell me if I got implementation right (especially the inheritance for the platform specific implementation).

[...]we would have a static view of the screen system when the application is started.[...]

Yep thats the current state. I did a bt of reading and it seems that on windows a WM_DEVICECHANGE message is send when a monitor is plugged in or out. So we could react to that and reinitilize the screens. I didn't find anything concerning rearranging in virtual screen space yet.
Like I said I am all for supporting a way to deal with change. I also think we can stick it into the window event loop, because realistically what are you gonna do as a reaction to a screenChangedEvent if not adjusting your windows?
But of course if there is more global event stuff (like sensors) it might be worth rethinking the window event queue. But that's for somebody else to decide
I'd say for now we should focus on getting the API and the implementation setteled. The reaction to events can easily be added later.


So check it out and tell me what you think!

[Laurent]

Originally I wanted sf::Screen to be an immutable object with public const attributes (see above). Apperently there is no way to make an object like this copy-/assignable in C++. So I had to go with an object that has private members and getters. If somebody has a better suggestion I'd be glad to hear it!

Const instances containing non-const members. Which means that you can construct and manipulate a sf::Screen instance as you wish, but those returned by the SFML API are read-only.