Fix: Non-thread-safe access to metadata in Compositor
Image's render result might get freed from another thread while the compositor is running. Add an utility function which invokes callback on the image's stamp data from a thread-guarded block. Ref #118337, #121761 Pull Request: https://projects.blender.org/blender/blender/pulls/121907
This commit is contained in:
@@ -75,6 +75,10 @@ void BKE_stamp_info_callback(void *data,
|
||||
struct StampData *stamp_data,
|
||||
StampCallback callback,
|
||||
bool noskip);
|
||||
void BKE_image_multilayer_stamp_info_callback(void *data,
|
||||
const Image &image,
|
||||
StampCallback callback,
|
||||
bool noskip);
|
||||
void BKE_render_result_stamp_data(struct RenderResult *rr, const char *key, const char *value);
|
||||
struct StampData *BKE_stamp_data_copy(const struct StampData *stamp_data);
|
||||
void BKE_stamp_data_free(struct StampData *stamp_data);
|
||||
|
||||
@@ -2480,6 +2480,23 @@ void BKE_stamp_info_callback(void *data,
|
||||
#undef CALL
|
||||
}
|
||||
|
||||
void BKE_image_multilayer_stamp_info_callback(void *data,
|
||||
const Image &image,
|
||||
StampCallback callback,
|
||||
bool noskip)
|
||||
{
|
||||
BLI_mutex_lock(static_cast<ThreadMutex *>(image.runtime.cache_mutex));
|
||||
|
||||
if (!image.rr || !image.rr->stamp_data) {
|
||||
BLI_mutex_unlock(static_cast<ThreadMutex *>(image.runtime.cache_mutex));
|
||||
return;
|
||||
}
|
||||
|
||||
BKE_stamp_info_callback(data, image.rr->stamp_data, callback, noskip);
|
||||
|
||||
BLI_mutex_unlock(static_cast<ThreadMutex *>(image.runtime.cache_mutex));
|
||||
}
|
||||
|
||||
void BKE_render_result_stamp_data(RenderResult *rr, const char *key, const char *value)
|
||||
{
|
||||
StampData *stamp_data;
|
||||
|
||||
@@ -175,6 +175,7 @@ void CryptomatteNode::input_operations_from_image_source(
|
||||
op->set_framenumber(context.get_framenumber());
|
||||
op->set_render_data(context.get_render_data());
|
||||
op->set_view_name(context.get_view_name());
|
||||
op->set_layer_name(render_layer->name);
|
||||
op->set_pass_name(render_pass->name);
|
||||
r_input_operations.append(op);
|
||||
}
|
||||
|
||||
@@ -20,6 +20,7 @@ ImageNode::ImageNode(bNode *editor_node) : Node(editor_node)
|
||||
}
|
||||
NodeOperation *ImageNode::do_multilayer_check(NodeConverter &converter,
|
||||
const CompositorContext &context,
|
||||
const char *layer_name,
|
||||
const char *pass_name,
|
||||
Image *image,
|
||||
ImageUser *user,
|
||||
@@ -47,6 +48,7 @@ NodeOperation *ImageNode::do_multilayer_check(NodeConverter &converter,
|
||||
operation->set_framenumber(framenumber);
|
||||
operation->set_render_data(context.get_render_data());
|
||||
operation->set_view_name(context.get_view_name());
|
||||
operation->set_layer_name(layer_name);
|
||||
operation->set_pass_name(pass_name);
|
||||
|
||||
converter.add_operation(operation);
|
||||
@@ -95,6 +97,7 @@ void ImageNode::convert_to_operations(NodeConverter &converter,
|
||||
case 1:
|
||||
operation = do_multilayer_check(converter,
|
||||
context,
|
||||
rl->name,
|
||||
rpass->name,
|
||||
image,
|
||||
imageuser,
|
||||
@@ -107,6 +110,7 @@ void ImageNode::convert_to_operations(NodeConverter &converter,
|
||||
case 3:
|
||||
operation = do_multilayer_check(converter,
|
||||
context,
|
||||
rl->name,
|
||||
rpass->name,
|
||||
image,
|
||||
imageuser,
|
||||
@@ -117,6 +121,7 @@ void ImageNode::convert_to_operations(NodeConverter &converter,
|
||||
case 4:
|
||||
operation = do_multilayer_check(converter,
|
||||
context,
|
||||
rl->name,
|
||||
rpass->name,
|
||||
image,
|
||||
imageuser,
|
||||
|
||||
@@ -22,6 +22,7 @@ class ImageNode : public Node {
|
||||
private:
|
||||
NodeOperation *do_multilayer_check(NodeConverter &converter,
|
||||
const CompositorContext &context,
|
||||
const char *layer_name,
|
||||
const char *pass_name,
|
||||
Image *image,
|
||||
ImageUser *user,
|
||||
|
||||
@@ -41,23 +41,20 @@ void MultilayerBaseOperation::update_memory_buffer_partial(MemoryBuffer *output,
|
||||
std::unique_ptr<MetaData> MultilayerColorOperation::get_meta_data()
|
||||
{
|
||||
BLI_assert(buffer_);
|
||||
MetaDataExtractCallbackData callback_data = {nullptr};
|
||||
/* TODO: Make access to the render result thread-safe. */
|
||||
RenderResult *render_result = image_->rr;
|
||||
if (render_result && render_result->stamp_data) {
|
||||
const RenderLayer *render_layer = static_cast<const RenderLayer *>(
|
||||
BLI_findlink(&image_->rr->layers, image_user_.layer));
|
||||
std::string full_layer_name = std::string(render_layer->name) + "." + pass_name_;
|
||||
blender::StringRef cryptomatte_layer_name =
|
||||
blender::bke::cryptomatte::BKE_cryptomatte_extract_layer_name(full_layer_name);
|
||||
callback_data.set_cryptomatte_keys(cryptomatte_layer_name);
|
||||
|
||||
BKE_stamp_info_callback(&callback_data,
|
||||
render_result->stamp_data,
|
||||
MetaDataExtractCallbackData::extract_cryptomatte_meta_data,
|
||||
false);
|
||||
if (!image_) {
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
MetaDataExtractCallbackData callback_data = {nullptr};
|
||||
std::string full_layer_name = layer_name_ + "." + pass_name_;
|
||||
blender::StringRef cryptomatte_layer_name =
|
||||
blender::bke::cryptomatte::BKE_cryptomatte_extract_layer_name(full_layer_name);
|
||||
callback_data.set_cryptomatte_keys(cryptomatte_layer_name);
|
||||
|
||||
BKE_image_multilayer_stamp_info_callback(
|
||||
&callback_data, *image_, MetaDataExtractCallbackData::extract_cryptomatte_meta_data, false);
|
||||
|
||||
return std::move(callback_data.meta_data);
|
||||
}
|
||||
|
||||
|
||||
@@ -12,6 +12,9 @@ namespace blender::compositor {
|
||||
|
||||
class MultilayerBaseOperation : public BaseImageOperation {
|
||||
protected:
|
||||
/* NOTE: The layer name is only used for meta-data. The image user's layer index defines which
|
||||
* layer will be actually accessed for the image buffer. */
|
||||
std::string layer_name_;
|
||||
std::string pass_name_;
|
||||
|
||||
ImBuf *get_im_buf() override;
|
||||
@@ -19,6 +22,10 @@ class MultilayerBaseOperation : public BaseImageOperation {
|
||||
public:
|
||||
MultilayerBaseOperation() = default;
|
||||
|
||||
void set_layer_name(std::string layer_name)
|
||||
{
|
||||
layer_name_ = std::move(layer_name);
|
||||
}
|
||||
void set_pass_name(std::string pass_name)
|
||||
{
|
||||
pass_name_ = std::move(pass_name);
|
||||
|
||||
Reference in New Issue
Block a user