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

Add ShaderUnmanaged #329

Merged

Conversation

furudbat
Copy link
Contributor

@furudbat furudbat commented Jul 9, 2024

Hello,
I added the ShaderUnmanaged class for #299 (comment), but honestly it didn't really solve the problem with the ownership of Models, Materials and Shader...

IF you are using (Load) Material with Shaders, use NEED to use ShaderUnmanaged or raylib Shader (struct).
Basically the ownership gets transfert to the Material.

Otherwise it's a bit more complicated when you are using Model, see raylib shaders_basic_pbr example:

    // Load old car model using PBR maps and shader
    // WARNING: We know this model consists of a single model.meshes[0] and
    // that model.materials[0] is by default assigned to that mesh
    // There could be more complex models consisting of multiple meshes and
    // multiple materials defined for those meshes... but always 1 mesh = 1 material
    Model car = LoadModel("resources/models/old_car_new.glb");

    // Assign already setup PBR shader to model.materials[0], used by models.meshes[0]
    car.materials[0].shader = shader;

...

    // De-Initialization
    //--------------------------------------------------------------------------------------
    // Unbind (disconnect) shader from car.material[0] 
    // to avoid UnloadMaterial() trying to unload it automatically
    car.materials[0].shader = (Shader){ 0 };
    UnloadMaterial(car.materials[0]);
    car.materials[0].maps = NULL;
    UnloadModel(car);
    
    floor.materials[0].shader = (Shader){ 0 };
    UnloadMaterial(floor.materials[0]);
    floor.materials[0].maps = NULL;
    UnloadModel(floor);
    
    UnloadShader(shader);       // Unload Shader

Right now you NEED to know when to unload the Material which also unloades the (non-default) Shader, so you also NEED to "unbind" the Shader from the Material before calling UnloadMaterial.


I personally use raylib for more 2D Projects, so my experience on 3D with raylib is limited
Maybe there is a way to track the materials/shaders in the Model, to know when to UnloadMaterial (UnloadShader).

I experimenting a bit with the setters of Model: SetShader and SetMaterial, on my own branch to control the ownership.

Model.hpp

enum class ModelMaterialShaderOption : uint8_t {
    NoUnload = 0,                                      ///< Do Nothing when UnloadingModel, manage materials and shaders on your own
    UnloadMaterial = 1,                                ///< UnloadMaterial: assume shader ownership is in Model (Material), shader gets unloaded by UnloadMaterial
    UnbindShader = 2,                                  ///< Unbind (disconnect) shader from Model (Material), to avoid UnloadMaterial() trying to unload it automatically (NO UnloadMaterial)
    UnbindShaderBeforeUnloadAndUnloadMaterial  = 3,    ///< UnloadMaterial AND Unbind (disconnect) shader from Model (Material), to avoid UnloadMaterial() trying to unload it automatically (manage shader ownership on your own)
};

class Model {
    ...
    void SetMaterialShader(size_t material_index, ::Shader& shader, ModelMaterialShaderOption option) {
        assert(material_index < m_data.materialCount);
        m_data.materials[material_index].shader = shader;
        trackMaterialOwnership(material_index, option);
        switch(m_trackMaterialOwnership[material_index]) {
            case ModelMaterialOptions::None:
                break;
            case ModelMaterialOptions::UnloadMaterial:
                // ownership of shader moved into material
                shader.id = rlGetShaderIdDefault();
                shader.locs = nullptr;
                break;
            case ModelMaterialOptions::UnbindShader:
            case ModelMaterialOptions::UnbindShaderBeforeUnloadAndUnloadMaterial:
                // nothing to move, user should manage shader on its own
                break;
        }
    }

    void Unload() {
        if (m_data.meshes != nullptr || m_data.materials != nullptr) {
            for(const auto& [material_index , option] : m_trackMaterialOwnership) {
                switch(option) {
                    case ModelMaterialOptions::None:
                        // do nothing
                        break;
                    case ModelMaterialOptions::UnloadMaterial:
                        // UnloadMaterial, should also unload shader
                        UnloadMaterial(m_data.materials[material_index]);
                        // maps also gets unloaded in UnloadMaterial
                        m_data.materials[material_index].maps = nullptr;
                        break;
                    case ModelMaterialOptions::UnbindShader:
                        m_data.materials[material_index].shader.id = rlGetShaderIdDefault();
                        m_data.materials[material_index].shader.locs = nullptr;
                        break;
                    case ModelMaterialOptions::UnbindShaderBeforeUnloadAndUnloadMaterial:
                        m_data.materials[material_index].shader.id = rlGetShaderIdDefault();
                        m_data.materials[material_index].shader.locs = nullptr;
                        // UnloadMaterial, should NOT unload the shader
                        UnloadMaterial(m_data.materials[material_index]);
                        // maps also gets unloaded in UnloadMaterial
                        m_data.materials[material_index].maps = nullptr;
                        break;
                }
            }
            ::UnloadModel(m_data);
            m_data.meshes = nullptr;
            m_data.materials = nullptr;
            m_trackMaterialOwnership.clear();
        }
    }

example

    // Assign material shader for our floor model, same PBR shader
    floor.SetMaterialShader(0, shader, raylib::ModelMaterialShaderOption::UnbindShaderBeforeUnloadAndUnloadMaterial);

    // connect road textures
    floor.SetMaterialMapTexture(0, MATERIAL_MAP_ALBEDO, LoadTexture("resources/road_a.png"));
    floor.SetMaterialMapTexture(0, MATERIAL_MAP_METALNESS, LoadTexture("resources/road_mra.png"));
    floor.SetMaterialMapTexture(0, MATERIAL_MAP_NORMAL, LoadTexture("resources/road_n.png"));

@RobLoach
Copy link
Owner

Thanks for pushing this up. Didn't see the notification for it. I think this is a great addition, is there anything missing for it? The ownership of the objects does get a bit messy across these objects.

@furudbat
Copy link
Contributor Author

Yea this is just the ShaderUnmanaged to solve the problem from #299, but the ownership problems for models etc. is just an issue on it's own and need more design and work to utilize the full C++ feature like RAII.

@RobLoach RobLoach merged commit 4c3e25c into RobLoach:master Aug 30, 2024
8 checks passed
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

Successfully merging this pull request may close these issues.

2 participants