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

Add 'Unsafe textures' feature #683

Merged
merged 4 commits into from
Sep 5, 2017
Merged

Add 'Unsafe textures' feature #683

merged 4 commits into from
Sep 5, 2017

Conversation

Cobrand
Copy link
Member

@Cobrand Cobrand commented Jul 9, 2017

cc @tanis2000


This change is Reviewable

@Cobrand
Copy link
Member Author

Cobrand commented Jul 9, 2017

Linked to issue #667, as a workaround to the headaches that might cause the changes in #632

@Cobrand
Copy link
Member Author

Cobrand commented Jul 23, 2017

@tanis2000 you said in the other issue you would be up to test this feature, would you mind giving me feedback on this?

@tanis2000
Copy link
Contributor

@Cobrand sure. I'm in the middle of moving. I'll have a look at this later this week. Hold tight! :)

@Cobrand
Copy link
Member Author

Cobrand commented Jul 24, 2017 via email

@Cobrand
Copy link
Member Author

Cobrand commented Jul 26, 2017

@icefoxen I think that feature may interest you as well for ggez, I saw that you were reluctant to bump ggez's sdl2 dep to 0.30, probably because of the changes with lifetimes and textures made in #632.

I think that pull request is a fair tradeoff for those who don't want to have headaches with lifetimes, at the price of optional manual destruction of the textures.

Would you mind trying it and telling me what you think of it?

@icefoxen
Copy link
Contributor

icefoxen commented Jul 26, 2017

Thanks, good to know! Though, uh, ggez doesn't actually use sdl2 for drawing currently. So the lack of a 0.30 bump was a combination of "we don't need it" and "I don't want to figure out how to deal with it" more than anything else.

I'll noodle around with this and see how it works though, sure!

@Cobrand
Copy link
Member Author

Cobrand commented Jul 26, 2017

I am aware that a lot of people are using sdl2 as a unified backend and use directly OpenGL behind it. That is fine as well! I just want to make sure no one is left behind because of a feature that might be too limiting for some people.

@tanis2000
Copy link
Contributor

It looks like it's working fine for my use case. Here's the branch of my own project using this PR: https://github.com/tanis2000/minigame-rust/tree/unsafe-textures

I still have to run some thorough tests to make sure that I'm not leaking or losing stuff around but it definitely made the code easier to work with due to less lifetimes to take care of so far :)

@Cobrand
Copy link
Member Author

Cobrand commented Aug 8, 2017

Glad it worked! Did you find the new documentation clear enough for this feature?

@Cobrand Cobrand force-pushed the unsafe-textures-feature branch from 2c967ac to 59a9591 Compare September 5, 2017 05:13
@Cobrand Cobrand merged commit 1756e02 into master Sep 5, 2017
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