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

Update bundled SDL2 to 2.0.14 #1078

Closed
wants to merge 2 commits into from
Closed

Update bundled SDL2 to 2.0.14 #1078

wants to merge 2 commits into from

Conversation

joao-conde
Copy link
Contributor

@Cobrand opening this for discussion referring to issue #1077 .

I changed what seemed like the relevant code but HAVE NOT TESTED anything because I am unaware of the correct testing procedure. Can you please give me some pointers? Also, feel like this could be added to the README.

@joao-conde
Copy link
Contributor Author

vcpkg CI is failing so might be the cause for the failures related with the Windows version

@joao-conde
Copy link
Contributor Author

joao-conde commented Mar 17, 2021

Tested in my project using my fork as a dependency and everything worked, no more bottleneck as discussed in the issue.

@Cobrand
Copy link
Member

Cobrand commented Mar 18, 2021

Weird that vcpkg fails, you didn't change anything that was related to it...

I can see you have changed the folder 2.0.12 into 2.0.14 and renamed the appropriate variable in build.rs. But the diff shows that every file has been renamed without changes: did you just rename the folder, or was the new folder exactly the same as the old one?

@joao-conde
Copy link
Contributor Author

I just renamed the folder, thought it was the appropriate way. Is it not?

Its falling only for windows-latest upon building cargo-vcpkg I think

@Cobrand
Copy link
Member

Cobrand commented Mar 19, 2021

You are supposed to take the content of this folder https://github.com/libsdl-org/SDL/tree/main/include instead. I think you'll be able to find it in this snapshot of 2.0.14 https://www.libsdl.org/release/SDL2-2.0.14.zip

@joao-conde
Copy link
Contributor Author

@Cobrand I feel a bit dumb 😅 I have now copied everything from the SDL release. Literally all files under include/. Even some .cmake ones. Is this the correct procedure?

@Taowyoo
Copy link

Taowyoo commented Mar 31, 2021

@joao-conde
@Cobrand
When trying bundled feature. I found this poll request.
I add some log steps in github actions to get more detailed log.
Here is the link of log:
https://github.com/Taowyoo/rust-sdl2/runs/2235510132?check_suite_focus=true#step:5:145
From the log, it seems like a MSVC related problem :

D:\a\rust-sdl2\rust-sdl2\target\vcpkg\buildtrees\sdl2-gfx\src\1.0.4-86ce4cf464\SDL2_gfxPrimitives.c(1766): error C2169: 'lrint': intrinsic function, cannot be defined

According to Microsoft Doc and A post from audacityteam forum:
The lrint is implemented begin from Visual Studio 2019 version 16.9.0.
The windows-latest env is now using Visual Studio Enterprise 2019 | 16.9.31105.61.

@joao-conde
Copy link
Contributor Author

I've been using my fork on windows as a project dependency and its works fine, I don't know how the CI windows build fails.

@Taowyoo
Copy link

Taowyoo commented Mar 31, 2021

I've been using my fork on windows as a project dependency and its works fine, I don't know how the CI windows build fails.

I test CI under windows-2019(which actually is windows-latest) and windows-2016.
https://github.com/Taowyoo/rust-sdl2/actions/runs/705986835
So, it can be confirmed that this is problem from latest update of MSVC 2019

@Cobrand
Copy link
Member

Cobrand commented Apr 2, 2021

Closed by #1086

@Cobrand Cobrand closed this Apr 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants