ArthurSonzogni / smk

SMK - Simple multimedia kit - C++ WebAssembly
MIT License
125 stars 18 forks source link

Use refcounted classes for Shader / ShaderProgram #9

Closed ArthurSonzogni closed 3 years ago

ArthurSonzogni commented 3 years ago

@TheMaverickProgrammer prefers a reference instead of a pointer:

ArthurSonzogni commented 3 years ago

@TheMaverickProgrammer WDYT?

I am providing an overload taking both a pointer or a reference. You might be against it. On the other side, I don't want to do pointer->reference or reference->pointer conversion in the code. So I am divided.

TheMaverickProgrammer commented 3 years ago

why do you want to pass a shader program as a pointer exactly?

In the other PR, I added the suggestion that maybe the internal shader should be a shared ptr and use shared ptrs for shaders because if the shader is out of scope, it could crash the render target. This way, the render target will always draw and the shader would also exist until it was no longer used by anything

ArthurSonzogni commented 3 years ago

Maybe we could keep the existing class name, but define it as a shared_ptr over the implementation class. Something like:

class ShaderImpl;
using Shader = std::shared_ptr<ShaderImpl>

Then Shader / ShaderProgram will have move semantics for free, and will be refcounted.


Alternatively, I can make it refcounted internally: I already did this for VertexArray: https://github.com/ArthurSonzogni/smk/blob/master/include/smk/VertexArray.hpp#L51

Not sure what is preferable.

TheMaverickProgrammer commented 3 years ago

I would definitely avoid alias with using. Someone might accidentally Shader* my_shader and not understand.

I do like your idea of internally reference counting for shaders as well. Then you can just pass the shader object as a copy operation