SDL icon indicating copy to clipboard operation
SDL copied to clipboard

CommitWindowChanges

Open icculus opened this issue 3 years ago • 4 comments

Throwing this out there because it's something I consistently ran into while working on Wayland, and is a consistent pain point for other backends like X: The SDL_Window API spec should be modified to make all changes buffered, with a CommitWindowChanges call to push everything at once.

This fixes the main problem we've been having lately, which is that the order of window calls can finally stop mattering and SDL can enforce consistent behavior without having to fight the OS as much. On top of being a pretty major bugfix (see the numerous issues for this, usually it's when modifying a window size after leaving fullscreen) it's also an optimization; Wayland for example has to commit the window surface every time a call is made, while in reality we shouldn't have to do it more than once (or twice if you count presentation) per frame. Right now compositors hate us for doing it like this, and when you actually look at the interaction at a low level it's easy to see why (see #4563).

This is similar to the old SetVideoMode without forcing SetVideoMode - someone could make multiple CommitWindowChanges calls if they absolutely needed to, while still knowing the consequences of not optimizing the batch of changes.

Originally posted by @flibitijibibo in https://github.com/libsdl-org/SDL/issues/3519#issuecomment-1091891558

icculus avatar Nov 26 '22 05:11 icculus

Here's a drastic thought: if that's a good direction, should we remove all window-modifying functions and replace them with a single one which takes a struct containing all necessary setup? There could be an extra getter function to populate that struct when you just want to change some of it rather than all of it.

It would avoid confusion when someone forgets to call the Commit API after calling the other Set functions (since that scenario wouldn't be possible anymore).

slime73 avatar Nov 28 '22 00:11 slime73

I think that'd be good; the only question is how to prepare the struct... it's easy to add new functions, not so much struct members. Do we want to lean on window flags to do the heavy lifting or should we give the struct a bunch of padding just in case?

flibitijibibo avatar Nov 28 '22 01:11 flibitijibibo

Imma put this idea out there for maybe more smart people to build upon this:

What vulkan does is have a void* at the end of the struct which a user can chain certain extra/newer options to.

Instead of the user chaining structs would it be possible from an ABI standpoint to at first have this void* at the end that is unused, but when new member would need to be added it gets changed to WindowCommitSettings2* (or any better name) where these new members would reside and chained to that struct at creation time?

A problem I could possibly see is it could get unwieldy to change anything if it would come to multiple of these nested structs

NekkoDroid avatar Dec 01 '22 16:12 NekkoDroid

Maybe this is overcomplicating things but I wonder if it makes sense to combine multiple approaches: have a bunch of padding in the main struct to allow for growth, and also have something at the end to allow for extension when there's no more padding left.


With this single function approach, there will be situations where one part of it can fail on some platforms, right? I wonder what should happen in terms of error-reporting and also with setting the rest of the state, in that case. It also might be harder to verify each backend's code is applying all the state it should be (especially if there are additions in the future), since there'd be a lot at once.

slime73 avatar Dec 05 '22 23:12 slime73

This is being discussed in #7907, so I'm going to close this as a duplicate.

icculus avatar Jul 12 '23 04:07 icculus