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

Multi-window support in Sdl2Application and GlfwApplication #168

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

mosra
Copy link
Owner

@mosra mosra commented Aug 20, 2016

Work-in-progress implementation of #124, option 3.

There's no backwards-incompatible change, except for Configuration::WindowFlag::Contextless and related flags, which are now application-global as supporting some windows with GL, some with Vulkan and some with nothing at all seems like a very rare use case that'd add a lot of implementation pain.

A new Sdl2ApplicationWindow class allows you to create another window for your application. Just creating the instance pops up a new window and all events that happen on it are transferred to events in that window.

Things left to do:

  • Sdl2Window is probably a better name than Sdl2ApplicationWindow
  • Per-window event handling
    • SDL_WINDOWEVENT_CLOSE for closing the window itself -- there's a conflict between leaving window lifetime management in users hands and "the close button just working", currently clicking the close button does nothing
      • maybe just have a closeEvent() which implicitly deletes the window but leaves the instance alive, which will make it no longer respond to events, and then the user is responsible for deleting the instance?
        • that also means all cases that now check for _windows[i] have to check for _windows[i]->_window too, the Application destructor also needs to verify not just that the SDL windows are deleted but that the Window wrapper instances are gone too so they don't access the Application after ... hmm but what if SDL would then reuse window IDs??
        • this could also be (ab)used for recreating the window later, i.e. the window-specific data being always there but the actual window only sometimes
    • Why there's no windowID for multi-gesture events? -- https://discourse.libsdl.org/t/how-to-get-the-origin-window-of-a-touch-event/19499/10, "impossible", move the event handler back to the app and document that
  • Properly making GL context current for window that received the event
    • is the context switch expensive even if it's just the surface being changed?
    • do it only for drawEvent(), i.e. accessing the default framebuffer outside of it has unspecified behavior
      • and viewportEvent()? and tickEvent()? or not that one
  • Figure out how to implicitly share some properties (like all windows being GL-enabled / Vk-enabled... by default if the main one was)
  • Properly tracking framebuffer viewport state
  • Properly handle events that don't belong to any window (e.g. mouse drag out) -- currently crashes or is ignored
  • Figure out behavior with VSync (especially avoiding the refresh rate halving when another window gets opened)
  • Ensure nothing breaks for GL-less builds
  • Documentation
  • Test on Windows & OSX
  • Test that single-window apps are not affected by any of these changes
  • Ability to show/hide/raise the windows
    • And a WindowEvent to allow tracking this? Currently this all falls back to anyEvent().
  • Ability to minimize/maximize them
  • Ability to set and/query window positions
  • Assert that additional windows are not created before the main window context is created
  • All this for GLFW as well
  • Is there something like multi-canvas for WebGL?
  • Ensure assertions mention Sdl2ApplicationWindow instead of Sdl2Application where appropriate

@mosra mosra self-assigned this Aug 20, 2016
@mosra mosra marked this pull request as draft April 29, 2023 17:55
@mosra mosra changed the title [WIP] Multi-window support in Sdl2Application Multi-window support in Sdl2Application and GlfwApplication Apr 29, 2023
@mosra mosra added this to the 2023.0a milestone Apr 29, 2023
The Configuration class is defined on the Windowless*Context already,
which means it's available at the point where the constructor is defined
and thus there's no need for such workarounds anymore.
This is just documentation-facing, instead of documenting an overload as
having a default-constructed argument simply show just one constructor
with a default value.

Also simplify the wording. They either create an OpenGL context or not
(or not implicitly).
Used to be silently ignored in some places, silently discarded in some
other and probably causing an error elsewhere. Better be strict instead.
So don't bother building the correct mask of it.
Mostly just code motion and related documentation updates because
Doxygen is such a crap that it can link to functions defined in a base
class but not types! FFS.

No actual multi-window support yet, that'll be in the next commits to
avoid this noise obscuring them.

TODO: Sdl2Window instead?
TODO: unsure about certain APIs, whether they should be per-window or
not
 - contextless / gl / vulkan?
 - mouse warp?
 - ...?
Instead of per-window. It's just too complicated to allow some windows
with GL context, some with Vulkan, some with neither and some with
both. For now at least.
TODO: some of this (the configuration DPI scaling/policy) should go to
  the previos commit i guess?
Copy link

codecov bot commented Feb 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b1ba1f0) 82.25% compared to head (6394c85) 82.25%.

❗ Current head 6394c85 differs from pull request most recent head 17c74b2. Consider uploading reports for the commit 17c74b2 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #168      +/-   ##
==========================================
- Coverage   82.25%   82.25%   -0.01%     
==========================================
  Files         566      566              
  Lines       43967    43966       -1     
==========================================
- Hits        36167    36166       -1     
  Misses       7800     7800              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Status: In progress
Development

Successfully merging this pull request may close these issues.

1 participant