Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Re-think WindowManager API #772

Open
alichraghi opened this issue Nov 7, 2024 · 3 comments
Open

Proposal: Re-think WindowManager API #772

alichraghi opened this issue Nov 7, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@alichraghi
Copy link
Member

alichraghi commented Nov 7, 2024

Motivation

Currently the windowing API is split into IWindowManager, IWindow and ICursorControl. the choosen names here mostly makes sense as all(?) windowing APIs offers similar functionality where you can query monitors and other stuff like creating the window and then the part where you manage window specific features.
however Nabla's api is kind of weird in this sense. you can create a window from IWindowManager and yet it's per window!? why is some methods inside IWindowManager and some in IWindow. we've also got a bunch of conflicting flags, etc. let alone the existence of ICursorControl..

Proposal

Considering that we need multi-windowing and the ability to query monitors (to create windows from), here is how i see the final API:

class IWindowManager {
    IWindowManager(); // initializes underlying global state (asserts not already initialized)
    ~IWindowManager(); // deinitializes underlying global state
    
    struct SMonitorInfo {
        uint32_t width;
        uint32_t height;
        uint32_t refreshRate;
        float32_t contentScaleX;
        float32_t contentScaleY;
        uint32_t virtualPosX;
        uint32_t virtualPosY;
    };
    
    struct SWindowCreationParams {
        core::smart_refctd_ptr<IEventCallback> callback;
        std::string title;
        IWindow::E_DISPLAY_MODE displayMode;
        IWindow::E_EXPANSION_MODE expansionMode;
        IWindow::E_RESIZE_MODE resizeMode;
        uint32_t width;
        uint32_t height;
        bool can_minimize;
        bool can_close;
    };
    
    virtual Monitor getPrimaryMonitor() = 0;
    virtual span<Monitor> getMonitors() = 0;
    virtual SMonitorInfo getMonitorInfo(Monitor monitor) = 0; // We separated the info from monitor handle because they might be needed while window is open (aka app is running)
    virtual IWindow createWindow(Monitor monitor, *IWindow parent, SWindowCreationParams params) = 0;
    virtual void destroyWindow(IWindow window) = 0;
};

class IWindow {
    enum E_DISPLAY_MODE {
        EDM_WINDOWED,
        EDM_BORDERLRESS,
        EDM_FULLSCREEN,
    };

    enum E_EXPANSION_MODE {
        EEM_NORMAL,
        EEM_MINIMIZED,
        EEM_MAXIMIZED,
    };
    
    enum E_RESIZE_MODE {
        ERM_NONE,
        ERM_UI,
        ERM_PROGRAM,
        ERM_ALL,
    };
    
    enum E_CURSOR_MODE {
        ECM_NORMAL,
        ECM_HIDDEN,
        ECM_DISABLED,
    };

    virtual CursorControl getCursorControl() = 0;
    virtual bool setVisibility(bool visibility) = 0;
    virtual bool setExpansionMode(E_EXPANSION_MODE mode) = 0;
    virtual bool setDisplayMode(E_DISPLAY_MODE mode) = 0;
    virtual bool setCursorMode(E_CURSOR_MODE mode) = 0;
    virtual bool setPosition(uint32_t x, uint32_t y) = 0;
    virtual bool setSize(uint32_t w, uint32_t h) = 0;
};

class ICursorControl {
    virtual bool setVisible(bool visible) = 0;
    virtual bool setPosition(SPosition pos) = 0;
    virtual bool setRelativePosition(IWindow* window, SRelativePosition pos) = 0;
};

(NOTE: i was lazy to write getters ^)

@alichraghi alichraghi added the enhancement New feature or request label Nov 7, 2024
@devshgraphicsprogramming
Copy link
Member

devshgraphicsprogramming commented Nov 7, 2024

you shouldn't be able to set the display mode dynamically on a window, should be done at creation time.

Also on android, WebGPU, etc. you won't be able to create or destroy a window, that stuff belongs in the specific Win32, Wayland, etc. window managers

Why is getmonitorInfo returning a single struct?

Btw the reason why cursor control is a separate thing is because the Cursor may be HW and separate from all windowing stuff, but also a per-window thing (Android and Web).

@alichraghi
Copy link
Member Author

you shouldn't be able to set the display mode dynamically on a window, should be done at creation time.

Also on android, WebGPU, etc. you won't be able to create or destroy a window, that stuff belongs in the specific Win32, Wayland, etc. window managers.

right, a bunch of these can't be implemented in android anyways, that's why we can return false in methods. no? because we really can't omit some features (except multi-windoing maybe?). note that this proposal doesn't remove any functionality but instead multi-monitors and ECM_DISABLED is newly added.

Why is getmonitorInfo returning a single struct?

typo. edited.

Btw the reason why cursor control is a separate thing is because the Cursor may be HW

got it. edited.

@devshgraphicsprogramming
Copy link
Member

I'll let @Erfan-Ahmadi deal wtih this, just remember that windows need to keep the window manager alive.

And on Win32 and other fun legacy platforms, every window method needs to defer/pump its action onto a dedicated thread & queue that belongs to the window manager (implementation detail I guess).

@alichraghi so that we don't "fuck it up" I'd be willing to sponsor a Linux/Wayland implementation out of our R&D budget.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants