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

API proposal: publicizing Window #2603

Open
4 of 8 tasks
Starbuck5 opened this issue Dec 14, 2023 · 8 comments
Open
4 of 8 tasks

API proposal: publicizing Window #2603

Starbuck5 opened this issue Dec 14, 2023 · 8 comments
Labels
render/_sdl2 pygame._render or former pygame._sdl2 window pygame.Window

Comments

@Starbuck5
Copy link
Member

Starbuck5 commented Dec 14, 2023

I'd like to turn _sdl2.video.Window into a public API in 2.4.0

Firstly, where should it live? I see two contenders.

  • pygame.display.Window
  • pygame.window.Window (like pygame.surface.Surface, primarily advertised as pygame.Window)

Yunline has successfully convinced me it should live at pygame.window.Window. If you disagree, this is the issue to speak out on.

Steps to making this thing public:

  • Make the class public at its final address
  • Extricate Window docs from _sdl2.video docs and publicize them in its final namespace
  • Keep _sdl2.video.Window / _sdl2.Window as an alias for old code
  • Make sure tests are good
  • Make sure nothing will bite us with it in SDL3. (Like fullscreen vs fullscreen desktop is going away in SDL3, so we probably shouldn't expose that...)
  • Window class example that shows off cool new functionality while only using software rendering
  • Can we make private or remove things that are iffy?
    • Sharing a surface with pygame.display can be suspect
    • Every SDL display flag is exposed, but not all of them will work. We don't support OPENGL on these for example. And what would HIGHDPI actually do? (also will change in SDL3?)
  • Polish up documentation
@Starbuck5 Starbuck5 added the render/_sdl2 pygame._render or former pygame._sdl2 label Dec 14, 2023
@Starbuck5 Starbuck5 added this to the 2.4.0 milestone Dec 14, 2023
@MyreMylar
Copy link
Member

I agree with pygame.window.Window. Keep window in it's own submodule as it is a new and slightly less confusing paradigm that will likely become the dominant way to create windows for desktop pygame usages going forward.

We could probably make good use of 'undocumented' code for dubious flag features that are currently in SDL2 but may or may not be in SDL3, but if it is straight up not working at all yet then let's pop up a NotImplementedError or something similar.

There is already a Window Example, but I agree it should be revamped once the code has stabilised to it's final location.

Like fullscreen vs fullscreen desktop is going away in SDL3

In what sense? I see this in the SDL3 docs: https://wiki.libsdl.org/SDL3/SDL_SetWindowFullscreenMode which implies you can still set a 'native fullscreen' or a 'desktop fullscreen' mode for a window.

I'm not sure there is a good reason for a programmer to interop between windows created by display.set_mode() and windows created by window.Window(). I'd be in favour of warning or erroring if we detect this happening and telling people to stick to one lane or the other, especially if it will reduce bugs.

@Starbuck5
Copy link
Member Author

In what sense? I see this in the SDL3 docs: https://wiki.libsdl.org/SDL3/SDL_SetWindowFullscreenMode which implies you can still set a 'native fullscreen' or a 'desktop fullscreen' mode for a window.

Oh, I didn't know about that. What I was referring to is that the flag SDL_WINDOW_FULLSCREEN_DESKTOP for window creation has been removed.

I'm not sure there is a good reason for a programmer to interop between windows created by display.set_mode() and windows created by window.Window(). I'd be in favour of warning or erroring if we detect this happening and telling people to stick to one lane or the other, especially if it will reduce bugs.

I'm happy you bring this up, I think this will be a pain to support, it'll just keep getting bug reports in increasingly strange situations and contributors will keep adding more code to patch it up (I expect). It's redundant now that you can use get_surface() and flip() on Window. I think the way forward would be to undocument it and start it raising a deprecation warning.

@Starbuck5
Copy link
Member Author

I've done an audit of all the SDL symbols used in Window.c for SDL3 compatibility, using the current state of SDL3's README-migration.

  • Fullscreen vs fullscreen desktop. We should be fine, our current API can be emulated on top of SDL3. We won't expose the new fullscreen mode directly, but our fullscreen function will continue to take a "desktop" arg, which can set the fullscreen mode internally.
  • SDL_SetWindowSize, SDL_SetWindowPosition, SDL_MinimizeWindow, SDL_MaximizeWindow, SDL_RestoreWindow, SDL_SetWindowFullscreen are all now asynchronous requests that are launched. There will also be a SDL_SyncWindow if you absolutely need things to go after each other. I think real code won't really notice this, we can update our docs to reflect this upon SDL3 migration-- and port SDL_SyncWindow then. I think this is fine as well.
  • SDL_GetWindowDisplayIndex is being replaced with SDL_GetDisplayForWindow, which returns an SDL_DisplayID instead of a display index. I think we should make this safe for us by removing Window.display_index.
  • SDL_WINDOW_TOOLTIP and SDL_WINDOW_POPUP_MENU are supported more broadly than just x11 in SDL3, but they need to go through a special window creation. Easy solution, remove these from our end for now.
  • SDL3 notes that SDL2's SDL_WINDOW_SKIP_TASKBAR is replaced by SDL_WINDOW_UTILITY, but SDL_WINDOW_UTILITY exists in SDL2 as well. Rather than deal with this, I say we remove them both for now. They are x11 only, so of limited use to a majority of folks.

@Starbuck5
Copy link
Member Author

We decided to push this out of 2.4.0 (#2627) in order to maintain a nice end of year release for 2.4.0. I'm happy that we've made significant progress on Window in the last two weeks and I hope we can gain enough confidence to call it a public API in the next release.

@Starbuck5 Starbuck5 modified the milestones: 2.4.0, 2.5.0 Dec 25, 2023
@ankith26
Copy link
Member

We need to put more thought into the API before making this public. It's not too late to be doing this, as the window API is experimental, and I don't expect to change too much stuff that are already implemented in Window.

So, the idea is: the current Window class should serve as a base class, and any method/attribute that "checks for window being of the x type" shouldn't belong to the Window class at all. Instead such a method/attribute should be a part of a Window subclass.

Why do this? From a user perspective it immediately makes the distinction clear between different kinds of window reducing any confusion or misunderstanding. If I have an instance of a "hardware window" the surface/flip methods won't be defined in the first place. Any editors and code analysis tools will therefore immediately alert a user if they try to call a surface/flip as they code, so they don't have to deal with an error at runtime.

From a developer perspective, it also probably simplifies our implementation as we wouldn't have heavy conditional branching and error checking like display.c does.

Quoting @Starbuck5 comment, that I like the idea of

I'd suggest SurfaceWindow and RendererWindow as the names. We'd also need OpenGLWindow. (Potentially VulkanWindow and GPUWindow (SDL GPU API coming to SDL3) in the future)

Breaking changes (affecting experimental API)

The newly added get_surface and flip methods must be moved out to the SurfaceWindow class. Potentially also making get_surface a property instead of a method.

Related stuff

Related issue: #2625
PR where we first discussed this: #2632

@oddbookworm
Copy link
Member

#2659 added opengl to Window

@Starbuck5 Starbuck5 modified the milestones: 2.5.1, 2.5.2 Aug 7, 2024
@Starbuck5
Copy link
Member Author

Starbuck5 commented Aug 11, 2024

Window support

I wanted to do an audit of what we have ported right now, since I'm worried about other module APIs that use window stuff, like in event/key/mouse.

Functions found by searching include/_.h in SDL2 for "SDL_Window _".
Manually filtered out some that are in the weeds, like iphone set animation callback, or not in SDL3, like shaped windows or gamma/brightness.

Functions that take Windows:

SDL Function Used in pygame-ce pygame.Window usable Notes
SDL_IsScreenKeyboardShown No No
SDL_ShowMessageBox Yes Yes
SDL_ShowSimpleMessageBox No No Covered by SDL_ShowMessageBox
SDL_WarpMouseInWindow Yes No
SDL_CreateWindowAndRenderer No No Covered by other funcs
SDL_CreateRenderer Yes Yes
SDL_GetRenderer Yes Yes
SDL_GetWindowWMInfo Yes No System changing in SDL3?
SDL_GetWindowDisplayIndex Yes No System changing in SDL3?
SDL_SetWindowDisplayMode No No
SDL_GetWindowDisplayMode No No
SDL_GetWindowICCProfile No No
SDL_GetWindowPixelFormat Yes Yes
SDL_CreateWindow Yes Yes
SDL_GetWindowID Yes Yes
SDL_GetWindowFlags Yes Yes
SDL_SetWindowTitle Yes Yes
SDL_GetWindowTitle Yes Yes
SDL_SetWindowIcon Yes Yes
SDL_SetWindowData Yes Yes N/A
SDL_GetWindowData Yes Yes N/A
SDL_SetWindowPosition Yes Yes
SDL_GetWindowPosition Yes Yes
SDL_SetWindowSize Yes Yes
SDL_GetWindowSize Yes Yes
SDL_GetWindowBordersSize No No
SDL_GetWindowSizeInPixels No No System changing in SDL3?
SDL_SetWindowMinimumSize Yes Yes
SDL_GetWindowMinimumSize Yes Yes
SDL_SetWindowMaximumSize Yes Yes
SDL_GetWindowMaximumSize Yes Yes
SDL_SetWindowBordered Yes Yes
SDL_SetWindowResizable Yes Yes
SDL_SetWindowAlwaysOnTop Yes Yes
SDL_ShowWindow Yes Yes
SDL_HideWindow Yes Yes
SDL_RaiseWindow Yes Yes
SDL_MaximizeWindow Yes Yes
SDL_MinimizeWindow Yes Yes
SDL_RestoreWindow Yes Yes
SDL_SetWindowFullscreen Yes Yes
SDL_HasWindowSurface Yes No
SDL_GetWindowSurface Yes Yes
SDL_UpdateWindowSurface Yes Yes
SDL_UpdateWindowSurfaceRects Yes No Unnecessary w/ UpdateWindowSurface
SDL_DestroyWindowSurface Yes No
SDL_SetWindowGrab Yes Yes SDL3 -> keyboardgrab + mousegrab
SDL_GetWindowGrab Yes Yes ditto
SDL_SetWindowKeyboardGrab Yes Yes
SDL_SetWindowMouseGrab Yes Yes
SDL_GetWindowKeyboardGrab Yes Yes
SDL_GetWindowMouseGrab Yes Yes
SDL_SetWindowMouseRect Yes Yes
SDL_GetWindowMouseRect Yes Yes
SDL_SetWindowOpacity Yes Yes
SDL_GetWindowOpacity Yes Yes
SDL_SetWindowModalFor Yes Yes
SDL_SetWindowInputFocus Yes Yes
SDL_SetWindowHitTest No No
SDL_FlashWindow No No #3049
SDL_DestroyWindow Yes Yes
SDL_GL_CreateContext Yes Yes
SDL_GL_MakeCurrent Yes Yes
SDL_GL_GetDrawableSize Yes No
SDL_GL_SwapWindow Yes Yes

Functions that return Windows:

SDL Function Used in pygame-ce pygame.Window usable
SDL_GetMouseFocus Yes No
SDL_RenderGetWindow No No
SDL_CreateWindowFrom Yes No
SDL_GetWindowFromID Yes Yes/NA/No?
SDL_GetGrabbedWindow Yes Yes

@ankith26 ankith26 removed this from the 2.5.2 milestone Oct 27, 2024
@ankith26
Copy link
Member

The Window API has been made public as part of #3170 and is set to make its way to the 2.5.2 release.

There is still room for more polish, additions and stuff but that can come in future releases. I am just gonna leave this issue open to track the further enhancements that can be done here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
render/_sdl2 pygame._render or former pygame._sdl2 window pygame.Window
Projects
None yet
Development

No branches or pull requests

4 participants