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

Shader.set_uniform_texture() texture borrow lifetime is too restrictive #213

Open
Lichtso opened this issue Jan 5, 2020 · 9 comments
Open

Comments

@Lichtso
Copy link

Lichtso commented Jan 5, 2020

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.
@crumblingstatue
Copy link
Collaborator

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: #204
But I don't know how to solve this in a satisfactory way.

@crumblingstatue
Copy link
Collaborator

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.

@silverweed
Copy link
Contributor

silverweed commented May 6, 2020

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.

@crumblingstatue
Copy link
Collaborator

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!

@silverweed
Copy link
Contributor

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 :(

@Tyrendel
Copy link

Tyrendel commented Jun 2, 2021

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...

@MyUsernamee

This comment was marked as resolved.

@Tyrendel
Copy link

Thank you so much for having shared your solution!

@FraggedPU
Copy link

FraggedPU commented Aug 26, 2024

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.

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

No branches or pull requests

6 participants