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

Fix SDL samples on linux #3678

Closed
wants to merge 1 commit into from
Closed

Fix SDL samples on linux #3678

wants to merge 1 commit into from

Conversation

rbviz
Copy link

@rbviz rbviz commented Dec 12, 2016

None of them were working for me on debian. Works as intended with these changes.

@marcosdsanchez
Copy link
Contributor

With your changes I'm able to run the examples, but running fire.cr in release mode doesn't work for me:

➜  crystal git:(bf33cea) bin/crystal samples/sdl/fire.cr --release       
Using compiled compiler at .build/crystal
Invalid memory access (signal 11) at address 0x4
[4283462] ???
[4278302] __crystal_sigfault_handler +30
[4283736] sigfault_handler +40
[139807178141824] ???
[4218555] ???
[4277579] main +43
[139807165575825] __libc_start_main +241
[4203226] _start +42
[0] ???

➜  crystal git:(bf33cea) llvm-config --version
3.9.0

➜  crystal git:(bf33cea) uname -a
Linux marcos-macbook-arch 4.4.38-1-lts #1 SMP Sat Dec 10 20:21:58 CET 2016 x86_64 GNU/Linux

➜  crystal git:(bf33cea) sdl2-config --version
2.0.5

@rbviz
Copy link
Author

rbviz commented Dec 16, 2016

Interesting. All it did for me is add performance.

Testing on fresh VM install.

 $ lsb_release -d
 Description:	Ubuntu 16.04.1 LTS
 $ uname -a
 Linux ubuntu 4.4.0-53-generic #74-Ubuntu SMP Fri Dec 2 15:59:10 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
 $ bin/crystal samples/sdl/fire.cr 
 2132 frames in 6051 ms
 Average FPS: 352.33845645347873
 $ bin/crystal samples/sdl/fire.cr  --release
 16600 frames in 10221 ms
 Average FPS: 1624.107230212308
 $ llvm-config --version
 3.8.0
 $ sdl2-config --version
 2.0.4

Did these samples work for you without my changes? I have no way to test on a mac right now.

@marcosdsanchez
Copy link
Contributor

marcosdsanchez commented Dec 21, 2016

Did these samples work for you without my changes? I have no way to test on a mac right now.

No, they were not working. They are working with your changes in my linux machine (my mac runs linux, not osx) with the exception of fire.cr in release mode.

@rbviz
Copy link
Author

rbviz commented Dec 24, 2016

Seems like you are using your locally compiled crystal, which seems to have some kind of problem.

rm -r .build and then retry, should work as i see no issues with the code right now or was able to replicate the issue.

@rbviz
Copy link
Author

rbviz commented Dec 30, 2016

This is a small change, what is taking so long? Broken samples are rather discouraging for people new to crystal.

@asterite
Copy link
Member

I think it's better if we remove these samples and move them to, maybe, https://github.com/ysbaddaden/sdl.cr

@ysbaddaden
Copy link
Contributor

It's taking some time because:

  • it used to work the way it is (at least for some people);
  • I don't have to call SDL_main on Linux (as per documentation), but I know nothing about other platforms; I don't have a Mac at hand for example;
  • it's not critical to Crystal (compiler, core/std lib).

Last but not least: the samples are for SDL 1.2, not SDL 2 as far as I know, and both versions sre quite different.

@drhuffman12
Copy link

I think the Crystal-lang team have done a great job and more than deserve some time off to spend w/ family this holiday season! Merry [belated] Christmas and Happy New Year!

@rbviz
Copy link
Author

rbviz commented Dec 31, 2016

Indeed, happy new year!

@rbviz
Copy link
Author

rbviz commented Feb 4, 2017

Closing this. Samples on master still exist and fail, but i see no interest in this PR.

@rbviz rbviz closed this Feb 4, 2017
@rbviz rbviz deleted the fix-sdl-samples-on-linux branch February 4, 2017 00:51
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.

None yet

5 participants