diff --git a/source/blender/compositor/realtime_compositor/COM_operation.hh b/source/blender/compositor/realtime_compositor/COM_operation.hh index 5e111fee5a5..976a08c9b51 100644 --- a/source/blender/compositor/realtime_compositor/COM_operation.hh +++ b/source/blender/compositor/realtime_compositor/COM_operation.hh @@ -159,6 +159,11 @@ class Operation { /* Get a reference to the descriptor of the input identified by the given identified. */ InputDescriptor &get_input_descriptor(StringRef identifier); + /* Release the results that are mapped to the inputs of the operation. This is called after the + * evaluation of the operation to declare that the results are no longer needed by this + * operation. */ + virtual void release_inputs(); + /* Returns a reference to the compositor context. */ Context &context() const; @@ -174,11 +179,6 @@ class Operation { * information. */ void reset_results(); - /* Release the results that are mapped to the inputs of the operation. This is called after the - * evaluation of the operation to declare that the results are no longer needed by this - * operation. */ - void release_inputs(); - /* Release the results that were allocated in the execute method but are not actually needed. * This can be the case if the execute method allocated a dummy texture for an unneeded result, * see the description of Result::allocate_texture() for more information. This is called after diff --git a/source/blender/compositor/realtime_compositor/COM_pixel_operation.hh b/source/blender/compositor/realtime_compositor/COM_pixel_operation.hh index 6e8c052218c..0bdbabe6345 100644 --- a/source/blender/compositor/realtime_compositor/COM_pixel_operation.hh +++ b/source/blender/compositor/realtime_compositor/COM_pixel_operation.hh @@ -77,6 +77,12 @@ class PixelOperation : public Operation { /* A map that associates the output socket of a node that is not part of the pixel operation to * the identifier of the input of the operation that was declared for it. */ Map outputs_to_declared_inputs_map_; + /* A map that associates the identifier of each input of the operation with the number of node + * inputs that use it, that is, its reference count. This is needed to properly release inputs + * after evaluation, since the results that provide the inputs aren't aware that multiple of + * their outgoing links are now part of a single pixel operation, so we need to release the + * inputs with their internal reference count stored here. See PixelOperation::release_inputs. */ + Map inputs_to_reference_counts_map_; /* A map that associates the output socket that provides the result of an output of the operation * with the identifier of that output. This is needed to help the compiler establish links * between operations. */ @@ -128,6 +134,12 @@ class PixelOperation : public Operation { * * The node execution schedule is given as an input. */ void compute_results_reference_counts(const Schedule &schedule); + + protected: + /* Release the results that are mapped to the inputs of the operation, making sure to release the + * input with a count equivalent to its reference count stored in inputs_to_reference_counts_map_ + * to avoid leaking the result. */ + void release_inputs() override; }; } // namespace blender::realtime_compositor diff --git a/source/blender/compositor/realtime_compositor/COM_result.hh b/source/blender/compositor/realtime_compositor/COM_result.hh index 87ee3ea3225..5165fda19b9 100644 --- a/source/blender/compositor/realtime_compositor/COM_result.hh +++ b/source/blender/compositor/realtime_compositor/COM_result.hh @@ -341,10 +341,10 @@ class Result { * result, the reference count of the master result is incremented instead. */ void increment_reference_count(int count = 1); - /* Decrement the reference count of the result and free its texture if the reference count - * reaches zero. This should be called when an operation that used this result no longer needs - * it. If this result have a master result, the master result is released instead. */ - void release(); + /* Decrement the reference count of the result by the given count and free its data if it reaches + * zero. The given count should not be more than the current reference count. If this result have + * a master result, the master result is released instead. */ + void release(const int count = 1); /* Frees the result data. If the result is not allocated or wraps external data, then this does * nothing. If this result have a master result, the master result is freed instead. */ diff --git a/source/blender/compositor/realtime_compositor/intern/multi_function_procedure_operation.cc b/source/blender/compositor/realtime_compositor/intern/multi_function_procedure_operation.cc index c1c7a3f93d8..a74e8119a19 100644 --- a/source/blender/compositor/realtime_compositor/intern/multi_function_procedure_operation.cc +++ b/source/blender/compositor/realtime_compositor/intern/multi_function_procedure_operation.cc @@ -237,17 +237,21 @@ mf::Variable *MultiFunctionProcedureOperation::get_multi_function_input_variable DInputSocket input_socket, DOutputSocket output_socket) { /* An input was already declared for that same output socket, so no need to declare it again and - * we just return its variable. But we update the domain priority of the input descriptor to be - * the higher priority of the existing descriptor and the descriptor of the new input socket. - * That's because the same output might be connected to multiple inputs inside the multi-function - * procedure operation which have different priorities. */ + * we just return its variable. */ if (output_to_variable_map_.contains(output_socket)) { + /* But first we update the domain priority of the input descriptor to be the higher priority of + * the existing descriptor and the descriptor of the new input socket. That's because the same + * output might be connected to multiple inputs inside the multi-function procedure operation + * which have different priorities. */ const std::string input_identifier = outputs_to_declared_inputs_map_.lookup(output_socket); InputDescriptor &input_descriptor = this->get_input_descriptor(input_identifier); input_descriptor.domain_priority = math::min( input_descriptor.domain_priority, input_descriptor_from_input_socket(input_socket.bsocket()).domain_priority); + /* Increment the input's reference count. */ + inputs_to_reference_counts_map_.lookup(input_identifier)++; + return output_to_variable_map_.lookup(output_socket); } @@ -274,6 +278,10 @@ mf::Variable *MultiFunctionProcedureOperation::get_multi_function_input_variable /* Map the output socket to the identifier of the operation input that was declared for it. */ outputs_to_declared_inputs_map_.add_new(output_socket, input_identifier); + /* Map the identifier of the operation input to a reference count of 1, this will later be + * incremented if that same output was referenced again. */ + inputs_to_reference_counts_map_.add_new(input_identifier, 1); + return &variable; } diff --git a/source/blender/compositor/realtime_compositor/intern/operation.cc b/source/blender/compositor/realtime_compositor/intern/operation.cc index 49cc2e63510..504756b12cc 100644 --- a/source/blender/compositor/realtime_compositor/intern/operation.cc +++ b/source/blender/compositor/realtime_compositor/intern/operation.cc @@ -173,6 +173,15 @@ InputDescriptor &Operation::get_input_descriptor(StringRef identifier) return input_descriptors_.lookup(identifier); } +void Operation::release_unneeded_results() +{ + for (Result &result : results_.values()) { + if (!result.should_compute() && result.is_allocated()) { + result.release(); + } + } +} + Context &Operation::context() const { return context_; @@ -212,13 +221,4 @@ void Operation::release_inputs() } } -void Operation::release_unneeded_results() -{ - for (Result &result : results_.values()) { - if (!result.should_compute() && result.is_allocated()) { - result.release(); - } - } -} - } // namespace blender::realtime_compositor diff --git a/source/blender/compositor/realtime_compositor/intern/pixel_operation.cc b/source/blender/compositor/realtime_compositor/intern/pixel_operation.cc index b6d4b24692e..25ac2b25100 100644 --- a/source/blender/compositor/realtime_compositor/intern/pixel_operation.cc +++ b/source/blender/compositor/realtime_compositor/intern/pixel_operation.cc @@ -93,4 +93,11 @@ void PixelOperation::compute_results_reference_counts(const Schedule &schedule) } } +void PixelOperation::release_inputs() +{ + for (const auto item : inputs_to_reference_counts_map_.items()) { + this->get_input(item.key).release(item.value); + } +} + } // namespace blender::realtime_compositor diff --git a/source/blender/compositor/realtime_compositor/intern/result.cc b/source/blender/compositor/realtime_compositor/intern/result.cc index 68009c013ca..4f1af7ea298 100644 --- a/source/blender/compositor/realtime_compositor/intern/result.cc +++ b/source/blender/compositor/realtime_compositor/intern/result.cc @@ -627,17 +627,19 @@ void Result::increment_reference_count(int count) reference_count_ += count; } -void Result::release() +void Result::release(const int count) { + BLI_assert(count > 0); + /* If there is a master result, release it instead. */ if (master_) { - master_->release(); + master_->release(count); return; } - /* Decrement the reference count, and if it reaches zero, release the texture back into the - * texture pool. */ - reference_count_--; + /* Decrement the reference count, and if it is not yet zero, return and do not free. */ + reference_count_ -= count; + BLI_assert(reference_count_ >= 0); if (reference_count_ != 0) { return; } diff --git a/source/blender/compositor/realtime_compositor/intern/shader_operation.cc b/source/blender/compositor/realtime_compositor/intern/shader_operation.cc index a2d4034734d..a1c7dc127b8 100644 --- a/source/blender/compositor/realtime_compositor/intern/shader_operation.cc +++ b/source/blender/compositor/realtime_compositor/intern/shader_operation.cc @@ -189,6 +189,9 @@ void ShaderOperation::link_node_input_external(DInputSocket input_socket, input_descriptor.domain_priority = math::min( input_descriptor.domain_priority, input_descriptor_from_input_socket(input_socket.bsocket()).domain_priority); + + /* Increment the input's reference count. */ + inputs_to_reference_counts_map_.lookup(input_identifier)++; } /* Link the attribute representing the shader operation input corresponding to the given output @@ -244,6 +247,10 @@ void ShaderOperation::declare_operation_input(DInputSocket input_socket, /* Map the output socket to the identifier of the operation input that was declared for it. */ outputs_to_declared_inputs_map_.add_new(output_socket, input_identifier); + + /* Map the identifier of the operation input to a reference count of 1, this will later be + * incremented if that same output was referenced again. */ + inputs_to_reference_counts_map_.add_new(input_identifier, 1); } void ShaderOperation::populate_results_for_node(DNode node)