Repository: google/filament
File: filament/backend/src/opengl/ShaderCompilerService.cpp (C++)

Code before:
        }
    }

    std::optional<CallbackManager::Handle> handle{};

    // Only valid when the blob functions are provided by users. The validity of this variable
    // doesn't guarantee that the program was created from the cache blob.
    BlobCacheKey key;

    // Used for the `THREAD_POOL` mode.
    mutable Mutex lock;
    mutable Condition cond;
    bool signaled = false;

    // Indicate this program was created from the cache blob.
    bool retrievedFromBlobCache = false;

    // Indicate whether this token was canceled. Used for THREAD_POOL mode.
    std::atomic_bool cancelled = false;
};

ShaderCompilerService::OpenGLProgramToken::~OpenGLProgramToken() {
    // Clean up OpenGL resources. Typically, shader objects are destroyed, and ownership of the
    // created program is transferred to `OpenGLProgram` if everything goes well. However, these GL
    // resources may persist if the token was cancelled. This ensures thorough cleanup for the case.
    for (GLuint& shader: gl.shaders) {
        if (!shader) {
            continue;
        }
        if (gl.program) {
            glDetachShader(gl.program, shader);
        }
        glDeleteShader(shader);
        shader = 0;
    }
    if (gl.program) {
        glDeleteProgram(gl.program);
        gl.program = 0;
    }

    // Notify the caller that the resource loading has concluded.
    trySubmittingCallback();
}

/* static */ void ShaderCompilerService::setUserData(const program_token_t& token,
        void* user) noexcept {
    token->user = user;
}

/* static */ void* ShaderCompilerService::getUserData(const program_token_t& token) noexcept {
    return token->user;

Review comment (suggestion):
This probably works, but I'm not a fan of doing GL calls from the dtor of an object managed by a shared_pointer. It means we have no real control when this is called. It's currently not the case, but this could happen from a non-GL thread and this would be bad.  I would prefer to have the destroy be more controlled.

Code after:
        }
    }

    std::optional<CallbackManager::Handle> handle{};

    // Only valid when the blob functions are provided by users. The validity of this variable
    // doesn't guarantee that the program was created from the cache blob.
    BlobCacheKey key;

    // Used for the `THREAD_POOL` mode.
    mutable Mutex lock;
    mutable Condition cond;
    bool signaled = false;

    // Indicate this program was created from the cache blob.
    bool retrievedFromBlobCache = false;

    // The current state of token. Used for THREAD_POOL mode.
    enum class State : uint8_t {
        // The token is currently in the queue, awaiting the start of shader compilation.
        // From this state, the token can transition to either LOADING or CANCELED.
        WAITING,
        // This state indicates that shader compilation is currently underway for the token.
        // From this state, the token can transition to either COMPLETED or CANCELED.
        LOADING,
        // This state indicates that shader compilation for the token has finished.
        // No state transitions can occur from this state.
        COMPLETED,
        // This state indicates that shader compilation for the token has been canceled.
        // No state transitions can occur from this state.
        CANCELED,
    };
    std::atomic<State> state = State::WAITING;
};

ShaderCompilerService::OpenGLProgramToken::~OpenGLProgramToken() {
    // Notify the caller that the resource loading has concluded.
    trySubmittingCallback();
}

/* static */ void ShaderCompilerService::setUserData(const program_token_t& token,
        void* user) noexcept {
    token->user = user;
}

/* static */ void* ShaderCompilerService::getUserData(const program_token_t& token) noexcept {
    return token->user;
}

// ------------------------------------------------------------------------------------------------