"unsafe_textures" breaks compatibility by changing the exposed API

[Rantanen]

_I tried searching through the existing issues and didn't see this discussed, so I'm opening this issue mostly for information. Feel free to close it, if it doesn't apply here for some reason. I'm not really a user of SDL2 anyway - just got referred to the unsafe_textures solution when discussing scoped lifetimes elsewhere._

The underlying issue is that features are compiler-wide. If I've got a project with a binary crate that depends on rust-sdl2 and a third party library crate that also depends on rust-sdl2, then specifying one of these crates to use unsafe_textures will change the API for both of them.

Or alternatively, if I've got a project that compiles just fine without using unsafe_textures and then I add a new dependency that also depends on rust-sdl2 but specifies the unsafe_textures feature, then my original project will fail to compile.

This isn't limited to compilation fails though. The implementation of Texture::drop changes based on that feature as well. So if one crate depends on the Texture cleaning up on Drop, then even if the crates somehow manage to compile, all of those textures will start leaking if another crate in the same project depends on the unsafe_textures feature.

As long as there are no third party helper libraries for rust-sdl2 that handle Textures, this isn't really a problem. If these start appearing, then the unsafe_textures feature may have an effect of splitting the ecosystem into "libraries that require unsafe_textures" and "libraries that are not compatible with unsafe_textures".

[Cobrand]

That's an extremely good point, and I've never really thought of that. To me, rust-sdl2 was always a "end user library", to be used by the end user only, and not a "dependency library" like rayon and others, where it can be used multiple times along the dependency chain.

unsafe-textures was a feature to help some end users with difficulties of rust-sdl2, but I still think the feature should be disabled by default unless really necessary.

I'm curious though, where exactly did you encounter this issue? There aren't many crates that use rust-sdl2 as a dependency.

[Rantanen]

As I tried to clarify in the original issue, I'm not a user of rust-sdl2 myself. The com-rs crate is struggling with a similar issue. It uses a runtime object that is used to instantiate COM objects, which shouldn't outlast the runtime. Rust-SDL2 was brought up in the issue discussion as an example of a crate that had implemented a solution to a similar problem and I looked into the implementation here.

I've never really thought of that

I had a feeling this was the case. The cross-crate impact is one of the more obscure aspects of features. The only reason I knew about it is because I was about to implement something similar elsewhere and someone else had to point it out to me. :)

The idea that rust-sdl2 would be considered to be more of an end user library, was the primary reason I figured this issue might not be that important in this context. I wanted to file this issue in case anyone ever faces it and tries searching for existing issues, but I don't see a problem with closing it as "by design".

I'll also link #929 here, since this would affect unsafe_fonts as well, if that gets implemented.

_(Without knowing anything more about the APIs, one option might be to change the APIs so that if unsafe_textures is enabled, then their respective APIs will return Texture<'static> instead - essentially keeping the lifetime parameters for API compatibility, but making the lifetime 'static to remove the dependency on other lifetimes. I feel like most of the code that is currently working with safe lifetimes should still be able to compile against 'static lifetimes - of course making that change now would be a breaking change to anyone currently using unsafe_textures - unsafe_static_textures maybe? 😄)_

[5GameMaker]

As I tried to clarify in the original issue, I'm not a user of rust-sdl2 myself. The com-rs crate is struggling with a similar issue. It uses a runtime object that is used to instantiate COM objects, which shouldn't outlast the runtime. Rust-SDL2 was brought up in the issue discussion as an example of a crate that had implemented a solution to a similar problem and I looked into the implementation here.

I've never really thought of that

I had a feeling this was the case. The cross-crate impact is one of the more obscure aspects of features. The only reason I knew about it is because I was about to implement something similar elsewhere and someone else had to point it out to me. :)

The idea that rust-sdl2 would be considered to be more of an end user library, was the primary reason I figured this issue might not be that important in this context. I wanted to file this issue in case anyone ever faces it and tries searching for existing issues, but I don't see a problem with closing it as "by design".

I'll also link #929 here, since this would affect unsafe_fonts as well, if that gets implemented.

_(Without knowing anything more about the APIs, one option might be to change the APIs so that if unsafe_textures is enabled, then their respective APIs will return Texture<'static> instead - essentially keeping the lifetime parameters for API compatibility, but making the lifetime 'static to remove the dependency on other lifetimes. I feel like most of the code that is currently working with safe lifetimes should still be able to compile against 'static lifetimes - of course making that change now would be a breaking change to anyone currently using unsafe_textures - unsafe_static_textures maybe? 😄)_

Maybe unsafe Texture.make_static() method so API isn't broken at all