renderer should not be in charge of rendering + loading textures

[nrxus]

The current implementation of render::Renderer implements both the drawing methods (&mut self) and the texture loading methods (&self). This makes it very difficult to implement a resource manager that is able to load and cache textures, without having to do some ugly work-arounds, such as:

1. Wrap the Renderer around Rc<RefCell<_>> which is less than ideal performance-wise.
2. Make the ResourceManager wrap every function for the drawing methods.
3. Instead of making the ResourceManager have a "texture loader" as one of its fields, have it as a parameter everytime something needs to be loaded. This is a code smell since in theory the only one method on the resource manager is this texture loader and this method requires someone else to manage it and own it.

I have so far chosen the third option in my own code but on the road to usability we should consider splitting the Renderer onto a TextureLoader (or TextureCreator) and a Rendererer. I understand that SDL currently does combine the functionality of creating/loading textures as well as rendering that texture but as part of making this library more idiomatic ("rustifying" SLD2) we should look at ways of making it more usable given Rust's borrowing constraints. In plain C/C++ it is not a huge issue that the render context handles both since the language does not care for safety and lets us share pointers without a care.

A current idea I have is creating a wrapper around the *mut ll::SDL_Renderer (i.e., struct RendererContext(*mut ll::SDL_Renderer)) and share this context on both the Renderer and the new TextureLoader.

This will also change the way we currently implement Texture since it currently takes in a Rc<RefCell<bool>> to check if the renderer is alive. This might have to change, or perhaps check if the RendererContext is alive instead. I am a little uncertain as to the purpose of this safety, though. Is it supposed to prevent ever accessing the texture after the renderer is de-allocated? If so, is it because SDL2 says that the operation may crash? Or is it to prevent attempting to draw it on a different renderer after the renderer that created it has been de-allocated?

As a bonus, it will also be nice for the Image context to be required for loading textures from image paths instead of hoping the developer remembers to hold it somewhere even though it is never explicitely needed in the code.

Would this be something the rust-SDL2 team would be interested in?

[Cobrand]

Interested, definitely! I didn't know the implications of the current code base but your explanation makes it very clear.

However, I think I'm speaking on the behalf of the team here when I'm saying that we don't have the time to make huge changes like this anymore. I'd gladly accept a well done PR, but making this change on our end is right now a little difficult. What you describe is not easy work as well; refactoring such a key part of SDL2 means intensively testing afterwards, and that means even more time to spend on such a feature.

So if you want to implement this, go ahead you have our full support ! We can even help you if you have trouble understanding the current code base. But we won't be part of the fun (the coding part) this time unfortunately :(

[nrxus]

Fixed in latest beta release