-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Emscripten: Allow for Multiple Windows #12575
base: main
Are you sure you want to change the base?
Conversation
Also, can you please do:
.. and then do a Because those merge commits are really annoying. |
I think it would be useful to add multi emscripten window support to at least one test. (Or create a new test?) |
test/testmultiwindow.c
Outdated
|
||
props = SDL_CreateProperties(); | ||
SDL_SetStringProperty(props, SDL_PROP_WINDOW_CREATE_TITLE_STRING, title); | ||
SDL_SetStringProperty(props, SDL_PROP_WINDOW_CREATE_EMSCRIPTEN_CANVAS_ID, title); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this test needs a custom HTML shell file: https://emscripten.org/docs/compiling/Deploying-Pages.html#build-files-and-custom-shell
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
emscripten support files can be stored in test/emscripten
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is not specific to Emscripten. It works on desktops and also tests multiple things:
- Creating multiple windows
- Unique rendering to each window
- Getting the global mouse position and spawning a window at that position
- Destroying windows
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's good, but without multiple HTML items to show windows, the test won't work on emscripten.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've verified this PR with this test on Emscripten. So, it does work.
In this PR, a new canvas element is created in SDL_CreateWindow
if the canvas ID specified wasn't already present in the DOM.
Does creating a hidden window with OpenGL context work the same way on other platforms? e.g. would this PR fix #12678? |
Co-authored-by: Sam Lantinga <slouken@libsdl.org>
I've done some quick testing but this still seems broken. Mouse position is still offset when using Vector2i getPosition(const WindowBase& relativeTo)
{
return getPosition() - relativeTo.getPosition();
} and now my game also seems to crash when recreating the HTML canvas via a window resize. |
OK, I've done some further digging and the crash seems to happen because |
Make windows change their position every iteration Make windows have random sizes
Add popup functionality Update SDL_SyncWindow so that it syncs the position and size Ensure windows with parents are offseted by their parent's position Improved multi window test
The window size wasn't being updated in |
SDL_Window
Emscripten_GLES_MakeCurrent
to pointModule['canvas']
to the canvas of the renderer's window that is currently being drawn toSDL_ShowWindow
,SDL_HideWindow
, andSDL_SetWindowPosition
for Emscripten windowsCloses #12512