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

Display multiple images implementation, some minor fixes #307

Merged
merged 26 commits into from Jan 22, 2019

Conversation

Projects
None yet
2 participants
@lazypassion
Copy link
Contributor

lazypassion commented Jan 20, 2019

@theotherphil

This comment has been minimized.

Copy link
Member

theotherphil commented Jan 20, 2019

Thanks! I’ll have a look through this tomorrow.

Can the body of display_image now be replaced with a call to display_images that constructs a single element array?

@lazypassion

This comment has been minimized.

Copy link
Contributor Author

lazypassion commented Jan 21, 2019

I was getting segmentation fault's sometimes when running it on ubuntu 18.04, so tell me if you experience any issues. It seems to be a problem with sdl2 because it works fine on windows.

@lazypassion lazypassion force-pushed the lazypassion:master branch from 51f0fe3 to 5c3cff2 Jan 21, 2019

@lazypassion

This comment has been minimized.

Copy link
Contributor Author

lazypassion commented Jan 21, 2019

Yeah it looks like it can, I'll make the change if nothing weird crops up

Whoops, I've made a huge error and erased all of my changes with a rebase. I have to correct that. I was trying to rename the author for some commits i made lol.

lazypassion added some commits Jan 21, 2019

display multiple images
* working on multiple image displays in a new branch

* managed to open two windows without resizing events working

* got most of the logic working, now have to figure out how to quit out application after closing all windows one by one

* trying to work with sdl2_sys window flags

* ran rustfmt, implemented multiple image displays, added empire state building jpeg
@theotherphil
Copy link
Member

theotherphil left a comment

I've left a few minor comments. I don't know much about sdl2 but this otherwise look reasonable to me!

Show resolved Hide resolved src/window.rs Outdated
Show resolved Hide resolved src/window.rs Outdated
Show resolved Hide resolved src/window.rs Outdated
Show resolved Hide resolved src/window.rs
@lazypassion

This comment has been minimized.

Copy link
Contributor Author

lazypassion commented Jan 22, 2019

If and when you merge, can you squash and merge please?

@theotherphil
Copy link
Member

theotherphil left a comment

I've left a couple of comments, but just noticed that you're still in the middle of changing this. Give me a ping when this is ready for review. And yes, I'll squash and merge! Thanks

Show resolved Hide resolved examples/multiple_image_displays.rs Outdated
Show resolved Hide resolved examples/display_multiple_images.rs Outdated

lazypassion added some commits Jan 22, 2019

@lazypassion

This comment has been minimized.

Copy link
Contributor Author

lazypassion commented Jan 22, 2019

@theotherphil is this how u ping?

@theotherphil theotherphil merged commit a29b389 into PistonDevelopers:master Jan 22, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@theotherphil

This comment has been minimized.

Copy link
Member

theotherphil commented Jan 22, 2019

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment