Upgrade to SDL 2.0.9

[rasky]

Among many other things, this fixes an annoying bug on MacOS (where all new SDL windows are full black until moved), and adds Vulkan support.

Fixes #808

Closes #816

As explained in #808 (comment), SDL 2.0.9 contains the patch that fixes build on Mac. Verified that I can run cargo build --features=bundled,static-link on Mac.

[rasky]

It looks like the failing test is flaky:

    ev.push_event(event.clone()).unwrap();
    let received = ep.poll_event().unwrap();
    assert_eq!(&event, &received);

SDL_PushEvent always changes the timestamp field putting the current time. So the test should be fixed not to compare the timestamp as it might change. For instance, adding a sleep:

    ::std::thread::sleep(::std::time::Duration::from_millis(100));
    ev.push_event(event.clone()).unwrap();
    let received = ep.poll_event().unwrap();
    assert_eq!(&event, &received);

makes the test reliably fail:

---- test_events stdout ----
thread 'test_events' panicked at 'assertion failed: `(left == right)`
  left: `User { timestamp: 0, window_id: 0, type_: 32770, code: 456, data1: 0x1234, data2: 0x5678 }`,
 right: `User { timestamp: 101, window_id: 0, type_: 32770, code: 456, data1: 0x1234, data2: 0x5678 }`', tests/events.rs:47:5
note: Run with `RUST_BACKTRACE=1` for a backtrace.

I'm pushing a fix.

[rasky]

@Cobrand ready for review!

[Cobrand]

I'll jump on this after work, thanks!

[mattiascibien]

@rasky shouldn't also the embedded headers be updated?

[rasky]

@mattiascibien I'm not sure what the embedded headers are used for, I grepped around but could not find a reference. Any pointers?

This PR updates the version that is shipped when activating the bundled feature. My understanding is that rust-sdl2 is meant to work with whatever version of SDL2 is installed on the system by default (maybe there's a minimum version), so unless the embedded headers are used in bundled mode, I don't think it's a problem if they are a different version. In fact, right now on master the bundled version is 2.0.5 and the embedded headers are on 2.0.8.

[Cobrand]

The headers are used with the bindgen feature, when you don't use the use-pkgconfig feature. So they're never used in bundled.
It's not a problem if they are different versions, but I'll suspect we'll soon encounter someone with this issue (2.0.8 in headers but 2.0.9 in bundled) soon enough. So for the sake of consistency, lets update the headers as well.
While we're at it, we could also update the pregenerated bindgen bindings to use 2.0.9 headers as well, but that's also optional.

[rasky]

So basically they are used when there is now way to detect where they are installed? Is that the rationale?

[Cobrand]

That is correct, but for now there are no ways for the build.rs to know if you have SDL2 installed without explicitely enabling the feature "use-pkgconfig", which is disabled by default even in linux environments.

In the end if you don't feel up to it (updating the headers), it's a relatively minor matter that we could take care of only later. It's up to you to decide, it's your PR.

[Cobrand]

As per the comment in #808 (comment), please remove:

• https://github.com/Rust-SDL2/rust-sdl2/blob/master/sdl2-sys/patches/SDL2-2.0.8-4234-mac-os-dylib-fix.patch
• https://github.com/Rust-SDL2/rust-sdl2/blob/master/sdl2-sys/build.rs#L126-L132 (leave a comment at that place, saying that the last patch was removed when we upgraded to 2.0.9 in PR Upgrade to SDL 2.0.9 #817)

After that, I'll ping someone with macOS to test whether the changes break anything on macOS or not.

Otherwise, it's a straightforward PR so looks good to me.

[rasky]

No it’s ok I was just trying to understand the context.

If I update the embedded headers to 2.0.9, what happens if somebody uses them against a previous version? Wouldn’t bindgen generate bindings for functions that then don’t exist?

[Cobrand]

If someone uses the new headers with an old version of SDL2, they'll get either a linker error, or a runtime error (depending if the old version is on the host or the client, if both, they'll get a linker error).

If you ever encounter this problem, you can use the "bindgen" + "use-pkgconfig" feature to use both the pkgconfig headers for the bindgen generation. This time, if you try to use sdl2 functions (such as vulkan) that are not in the old version of SDL2, you'll get a compile time error (linker error).

If you don't have access to pkg-config, there is an env variable that allows you to override the default headers (SDL2_INCLUDE_PATH). In any case, there'll always be a way to override what we change here, we're merely changing the defaults.

[rasky]

Done, I've pushed a commit that upgrades embedded headers to 2.0.9.

[rasky]

FWIW, I work on MacOS, and I've personally tested this PR only on MacOS, but of course double checks always help (especially since I've not personally experimented the problem that caused the reversion).

[Cobrand]

As per the comment in #808 (comment), please remove:

* https://github.com/Rust-SDL2/rust-sdl2/blob/master/sdl2-sys/patches/SDL2-2.0.8-4234-mac-os-dylib-fix.patch

* https://github.com/Rust-SDL2/rust-sdl2/blob/master/sdl2-sys/build.rs#L126-L132 (leave a comment at that place, saying that the last patch was removed when we upgraded to 2.0.9 in PR #817)

After that, I'll ping someone with macOS to test whether the changes break anything on macOS or not.

Otherwise, it's a straightforward PR so looks good to me.

I've made this comment in case you hadn't seen it the first time.

By the way, you'll need to rebase upon master, I've merged another PR that changes build.rs
The changes are pretty straightforward though, I believe only "2.0.8" to "2.0.9" will have conflict in build.rs

[rasky]

@Cobrand done, removed the patch and rebased.

[Cobrand]

LGTM! I'll merge whenever CI's ready