Psyop / CryptomatteArnold

Cryptomatte for Arnold
BSD 3-Clause "New" or "Revised" License
56 stars 19 forks source link

Further critsec removal #10

Closed jonahfriedman closed 1 year ago

jonahfriedman commented 1 year ago

@ThiagoIze - I added another commit to further remove the vestiges of the critical section, I figure just calling lock and unlock on something called "mutex" is nicer. Could you review this to make sure it's correct?

jonahfriedman commented 1 year ago

I like this:

node_update {
    std::lock_guard<AtMutex> guard(g_crypto_mutex);
    node_update_content(node);
}

But this a little less so.

        ...
        user_cryptomattes = UserCryptomattes(uc_aov_array, uc_src_array);

        std::lock_guard<AtMutex> guard(g_crypto_mutex);
        setup_outputs(universe);
    }

I think I'd want to put a scope around those last two just as a signal that the lock is meant to guard setup_outputs, where as this is just totally clear that "this is guarding that".

        ....
        user_cryptomattes = UserCryptomattes(uc_aov_array, uc_src_array);

        g_crypto_mutex.lock();
        setup_outputs(universe);
        g_crypto_mutex.unlock();
    }

So I think I'll stop here and accept the PR. Thanks @ThiagoIze!

jonahfriedman commented 1 year ago

(For posterity, Thiago was right and I did what he suggested in 4d94015.)