rust-sfml icon indicating copy to clipboard operation
rust-sfml copied to clipboard

Shader.set_uniform_texture() texture borrow lifetime is too restrictive

Open Lichtso opened this issue 5 years ago • 9 comments

Borrowing the texture for its entire lifetime renders this method unusable:

  1. It means the texture can not be updated anymore.
  2. Even if the texture is created just for the call, it can not be discarded anymore because: borrow might be used here, when 'shader' is dropped and runs the 'Drop' code for type 'sfml::graphics::Shader'
  3. Thus, the shader needs to be recompiled for every texture update, which is not acceptable.

Lichtso avatar Jan 05 '20 09:01 Lichtso

I'm not sure how to solve this in an easy way. If we didn't require that the texture outlives the shader, the user could drop the texture before the shader stops using it, causing undefined behavior.

This will probably require finding a good solution to allow Shader/Sprite/etc. to accept Rc<Texture> or maybe even Rc<RefCell<Texture>> instead of just &Texture. Related: https://github.com/jeremyletang/rust-sfml/issues/204 But I don't know how to solve this in a satisfactory way.

crumblingstatue avatar Jan 05 '20 23:01 crumblingstatue

One idea I have is allowing ownership of the texture by Shader. Maybe have a method for temporarily borrowing it mutably if it needs to be modified. Moving the Shader shouldn't cause problems, because the texture is at a fixed location on the heap, thanks to SfBox.

crumblingstatue avatar Jan 07 '20 11:01 crumblingstatue

Are there any updates on this? I'm incurring in the same problem.

I can help with the implementation if a design is agreed upon.

silverweed avatar May 06 '20 08:05 silverweed

We could potentially have an unsafe method that accepts a raw pointer to a Texture and offloads the responsibility of ensuring soundness to the user, until we come up with a better solution.

If anyone has good ideas on safe solutions though, I'm happy to review them!

crumblingstatue avatar May 06 '20 15:05 crumblingstatue

I think that as a temporary solution it can be ok, as long as we can document well how to use it safely. Meanwhile I'm also thinking about safe solutions, but it's not easy to find one while keeping the same structure as SFML :(

silverweed avatar May 07 '20 17:05 silverweed

From what I understand of the concepts of Rust you either have:

  • to use Rc<RefCell<T>> to allow multiple objects to carry reference and update resources (-> replace SfBox<T> by SfRc<RefCell<T>>?)
  • to send references of resources everytime you want to use an object that needs them. Tedious especially for shaders using multiple textures.
  • have the objects own resources directly but it means duplicating textures / fonts / ... for every object that needs them... unacceptable in my point of view.

I would like to help on the subject but I am really a noob in the world of bindings and I will need help myself to make the subject move forward...

Tyrendel avatar Jun 02 '21 14:06 Tyrendel

Is there anything I can do as a temporary solution? I am having the same issue and I can't find a away around it. I have a shader stored in a struct along with textures, then when I try to assign them as a uniform it doesn't work because the reference doesn't live longer than the function it is called in, so it can't be stored in the shader. Thanks for any help in advance! : )

Edit: I solved this comment, you have to dereference the texture borrow as *const then reborrow so it will effectively become a static borrow from what I understand. I leave this comment here because it might help someone, maybe.

MyUsernamee avatar Aug 22 '22 04:08 MyUsernamee

Thank you so much for having shared your solution!

Tyrendel avatar Nov 27 '22 18:11 Tyrendel

Could someone perhaps provide an example of how this would look in practice? I'm currently struggling to implement the proposed solution. How can uniform textures in the shader even be set at all while it is borrowed by RenderStates (required to apply shader using draw_with_render_states)?

Many thanks in advance to any future legend that might help me out here.

FraggedPU avatar Aug 26 '24 07:08 FraggedPU