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.

Sign up for GitHub

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

Point SDL2 port at upstream #15452

Merged
merged 6 commits into from
Jan 4, 2022
Merged

Point SDL2 port at upstream #15452

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f0f8377
Avoid calling SDL_Quit in SDL2 tests
Daft-Freak Nov 8, 2021
ebb5d39
Fix sdl_togglefullscreen for new event target behaviour
Daft-Freak Nov 8, 2021
adbe679
Remove creating a renderer from test_webgl_context_attributes_sdl2
Daft-Freak Nov 8, 2021
86efa76
Update SDL2 port to point at upstream
Daft-Freak Nov 8, 2021
7ea9bff
changelog
Daft-Freak Dec 2, 2021
587b066
Merge branch 'main' into upstream-sdl
kripken Jan 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions ChangeLog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ See docs/process.md for more on how version tagging works.

3.1.1
-----
- Updated SDL 2 port to upstream version 2.0.18 (from a patched 2.0.10). This
includes all downstream patches and many upstream changes.

3.1.0 - 12/22/2021
------------------
Expand Down
2 changes: 1 addition & 1 deletion tests/sdl2_glalphatest.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ int main(int argc, char* argv[]) {

glDeleteTextures(1, &texture);

SDL_Quit();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove these calls/

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise all windows get destroyed and the tests fail because the canvas is gone (well, 0x0).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So have these tests been failing forever?

Can we perhaps land all these test fixes separately (first), since they seem independent of the SDL2 update?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these were passing with the old version, so something may have changed there. I can split the test fixes out though.

(The testfullscreen one has likely been failing for a while.)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did slightly worry that we have a behavioral change here? My understanding is that switching to upstream SDL would essentially be a no-op but this seem to contradict that.

Can you explain a little more what you mean by "tests fail because the canvas is gone (well, 0x0)."? Is this test run as part of CI? i.e. would reverting this part of the change cause CI to fail?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, thanks for clarifying. Makes sense. Could you replace these with a comment maybe // Exiting without calling SDL_Quit so the test framework can the canvas data (or something like that.. I'm not sure I have the working corret.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, some of the other tests already have something like that:

emscripten/tests/sdl2glshader.c

Line 169 in df034c9

// Don't quit - we need to reftest the canvas! SDL_Quit();

Must have been missed in these ones due to the missing SDL_Init and the bug in quit... will update

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe also a good idea to add a changlog entry for this? Both in general for the change to use latest upstream, and specifically for this noticeable issue.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooh, yeah +4 SDL releases is probably worth a changelog entry! I'll try to come up with something...

(note that the issue here is only noticeable if you don't call SDL_Init but do call SDL_Quit... otherwise it happened a few updates ago)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, thanks! Yeah, I guess the breaking change here is not that important to mention then. But the +4 versions is 😄 Any positive highlights that people might notice could also be worth mentioning briefly in the log.

// Don't quit - we need to reftest the canvas! SDL_Quit();

return 0;
}
2 changes: 1 addition & 1 deletion tests/sdl2_glclipplane_gllighting.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ int main(int argc, char* argv[]) {

SDL_GL_SwapWindow(window);

SDL_Quit();
// Don't quit - we need to reftest the canvas! SDL_Quit();

return 0;
}
2 changes: 1 addition & 1 deletion tests/sdl2_gldrawelements.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ int main(int argc, char* argv[]) {
// Now we can delete the OpenGL texture and close down SDL
glDeleteTextures(1, &texture);

SDL_Quit();
// Don't quit - we need to reftest the canvas! SDL_Quit();

return 0;
}
2 changes: 1 addition & 1 deletion tests/sdl2_glmatrixmode_texture.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ int main(int argc, char* argv[]) {
// Now we can delete the OpenGL texture and close down SDL
glDeleteTextures(1, &texture);

SDL_Quit();
// Don't quit - we need to reftest the canvas! SDL_Quit();

return 0;
}
3 changes: 2 additions & 1 deletion tests/sdl_togglefullscreen.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,9 @@ int main() {
/* SDL 1 limits which events can be used here.
* Click and mouseup don't work.
*/
if (emscripten_set_mouseup_callback(NULL, NULL, 1, mouseup) != EMSCRIPTEN_RESULT_SUCCESS) {
if (emscripten_set_mouseup_callback("#canvas", NULL, 1, mouseup) != EMSCRIPTEN_RESULT_SUCCESS) {
printf("Couldn't to set mouse callback. Test failed.\n");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add return 1; here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

return 1;
}

printf("You should see a yellow canvas.\n");
Expand Down
2 changes: 1 addition & 1 deletion tests/test_webgl_context_attributes_sdl2.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ int main(int argc, char *argv[]) {
SDL_WINDOWPOS_UNDEFINED,
WINDOWS_SIZE, WINDOWS_SIZE,
SDL_WINDOW_OPENGL);
SDL_Renderer *sdlRenderer = SDL_CreateRenderer(sdlWindow, -1, 0);

SDL_GLContext context = SDL_GL_CreateContext(sdlWindow);

glewInit();
Expand Down
15 changes: 8 additions & 7 deletions tools/ports/sdl2.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@

import os

TAG = 'version_24'
HASH = '5a8181acdcce29cdda7e7a4cc876602740f5b9deebd366ecec71ae15c4bbf1f352da4dd0e3c5e0ba8160709dda0270566d64a6cd3892da894463ecf8502836aa'
SUBDIR = 'SDL2-' + TAG
TAG = 'release-2.0.18'
HASH = '89e716846f72d0de291b5e1b4b98275358fbb415624ed40dde2405c4fc8b0a0c72ac68cd11088188ae69d3ef794a84a335b0cfdb4c0bd4f7df7793d7a2252a93'
SUBDIR = 'SDL-' + TAG


def needed(settings):
Expand All @@ -20,7 +20,7 @@ def get_lib_name(settings):

def get(ports, settings, shared):
# get the port
ports.fetch_project('sdl2', 'https://github.com/emscripten-ports/SDL2/archive/' + TAG + '.zip', SUBDIR, sha512hash=HASH)
ports.fetch_project('sdl2', 'https://github.com/libsdl-org/SDL/archive/' + TAG + '.zip', SUBDIR, sha512hash=HASH)

def create(final):
# copy includes to a location so they can be used as 'SDL2/'
Expand All @@ -31,8 +31,8 @@ def create(final):
srcs = '''SDL.c SDL_assert.c SDL_dataqueue.c SDL_error.c SDL_hints.c SDL_log.c atomic/SDL_atomic.c
atomic/SDL_spinlock.c audio/SDL_audio.c audio/SDL_audiocvt.c audio/SDL_audiodev.c
audio/SDL_audiotypecvt.c audio/SDL_mixer.c audio/SDL_wave.c cpuinfo/SDL_cpuinfo.c
dynapi/SDL_dynapi.c events/SDL_clipboardevents.c events/SDL_dropevents.c events/SDL_events.c
events/SDL_gesture.c events/SDL_keyboard.c events/SDL_mouse.c events/SDL_quit.c
dynapi/SDL_dynapi.c events/SDL_clipboardevents.c events/SDL_displayevents.c events/SDL_dropevents.c
events/SDL_events.c events/SDL_gesture.c events/SDL_keyboard.c events/SDL_mouse.c events/SDL_quit.c
events/SDL_touch.c events/SDL_windowevents.c file/SDL_rwops.c haptic/SDL_haptic.c
joystick/SDL_gamecontroller.c joystick/SDL_joystick.c
power/SDL_power.c render/SDL_d3dmath.c render/SDL_render.c
Expand All @@ -41,7 +41,8 @@ def create(final):
render/opengles2/SDL_render_gles2.c render/opengles2/SDL_shaders_gles2.c
render/psp/SDL_render_psp.c render/software/SDL_blendfillrect.c render/software/SDL_blendline.c
render/software/SDL_blendpoint.c render/software/SDL_drawline.c render/software/SDL_drawpoint.c
render/software/SDL_render_sw.c render/software/SDL_rotate.c sensor/SDL_sensor.c
render/software/SDL_render_sw.c render/software/SDL_rotate.c render/software/SDL_triangle.c
sensor/SDL_sensor.c sensor/dummy/SDL_dummysensor.c
stdlib/SDL_getenv.c stdlib/SDL_iconv.c stdlib/SDL_malloc.c stdlib/SDL_qsort.c
stdlib/SDL_stdlib.c stdlib/SDL_string.c thread/SDL_thread.c timer/SDL_timer.c
video/SDL_RLEaccel.c video/SDL_blit.c video/SDL_blit_0.c video/SDL_blit_1.c video/SDL_blit_A.c
Expand Down