From 39d0cff1dcd6b40e8975390b1a9f786721592f8f Mon Sep 17 00:00:00 2001 From: Omar Emara Date: Thu, 6 Mar 2025 08:50:22 +0100 Subject: [PATCH] Refactor: Compositor: Remove initial reference count This patch removes the initial reference count mechanism from the Result class. That's because results are no longer cached across evaluations ever since 97ada4f307, so it is no longer useful. Pull Request: https://projects.blender.org/blender/blender/pulls/135526 --- source/blender/compositor/COM_operation.hh | 4 -- source/blender/compositor/COM_result.hh | 42 ++++++------------- .../intern/input_single_value_operation.cc | 2 +- .../compositor/intern/node_operation.cc | 2 +- source/blender/compositor/intern/operation.cc | 9 ---- .../compositor/intern/pixel_operation.cc | 2 +- source/blender/compositor/intern/result.cc | 24 ++--------- .../compositor/intern/simple_operation.cc | 2 +- source/blender/render/intern/compositor.cc | 4 +- 9 files changed, 23 insertions(+), 68 deletions(-) diff --git a/source/blender/compositor/COM_operation.hh b/source/blender/compositor/COM_operation.hh index 9ecd6c4a002..5911949c16e 100644 --- a/source/blender/compositor/COM_operation.hh +++ b/source/blender/compositor/COM_operation.hh @@ -166,10 +166,6 @@ class Operation { * evaluated directly. Otherwise, the input processors will be added and evaluated. */ void evaluate_input_processors(); - /* Resets the results of the operation. See the reset method in the Result class for more - * 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. */ diff --git a/source/blender/compositor/COM_result.hh b/source/blender/compositor/COM_result.hh index 783298502ea..ed93269eba4 100644 --- a/source/blender/compositor/COM_result.hh +++ b/source/blender/compositor/COM_result.hh @@ -74,13 +74,10 @@ enum class ResultStorageType : uint8_t { * module, see the allocation method for more information. * * Results are reference counted and their data are released once their reference count reaches - * zero. After constructing a result, the set_initial_reference_count method is called to declare - * the number of operations that needs this result. Once each operation that needs the result no - * longer needs it, the release method is called and the reference count is decremented, until it - * reaches zero, where the result's data is then released. Since results are eventually - * decremented to zero by the end of every evaluation, the reference count is restored before every - * evaluation to its initial reference count by calling the reset method, which is why a separate - * member initial_reference_count_ is stored to keep track of the initial value. + * zero. After constructing a result, the set_reference_count method is called to declare the + * number of operations that needs this result. Once each operation that needs the result no longer + * needs it, the release method is called and the reference count is decremented, until it reaches + * zero, where the result's data is then released. * * A result not only represents an image, but also the area it occupies in the virtual compositing * space. This area is called the Domain of the result, see the discussion in COM_domain.hh for @@ -125,19 +122,13 @@ class Result { GPUTexture *gpu_texture_ = nullptr; GMutableSpan cpu_data_; }; - /* The number of operations that currently needs this result. At the time when the result is - * computed, this member will have a value that matches initial_reference_count_. Once each - * operation that needs the result no longer needs it, the release method is called and the - * reference count is decremented, until it reaches zero, where the result's data is then - * released. If this result have a master result, then this reference count is irrelevant and - * shadowed by the reference count of the master result. */ + /* The number of users that currently needs this result. Operations initializes this by calling + * the set_reference_count method before evaluation. Once each operation that needs the result no + * longer needs it, the release method is called and the reference count is decremented, until it + * reaches zero, where the result's data is then released. If this result have a master result, + * then this reference count is irrelevant and shadowed by the reference count of the master + * result. */ int reference_count_ = 1; - /* The number of operations that reference and use this result at the time when it was initially - * computed. Since reference_count_ is decremented and always becomes zero at the end of the - * evaluation, this member is used to reset the reference count of the results for later - * evaluations by calling the reset method. This member is also used to determine if this result - * should be computed by calling the should_compute method. */ - int initial_reference_count_ = 1; /* If the result is a single value, this member stores the value of the result, the value of * which will be identical to that stored in the data_ member. The active variant member depends * on the type of the result. This member is uninitialized and should not be used if the result @@ -301,16 +292,9 @@ class Result { * for more information. */ RealizationOptions &get_realization_options(); - /* Set the value of initial_reference_count_, see that member for more details. This should be - * called after constructing the result to declare the number of operations that needs it. */ - void set_initial_reference_count(int count); - - /* Reset the result to prepare it for a new evaluation. This should be called before evaluating - * the operation that computes this result. Keep the type, precision, context, and initial - * reference count, and rest all other members to their default value. Finally, set the value of - * reference_count_ to the value of initial_reference_count_ since reference_count_ may have - * already been decremented to zero in a previous evaluation. */ - void reset(); + /* Set the value of reference_count_, see that member for more details. This should be called + * after constructing the result to declare the number of operations that needs it. */ + void set_reference_count(int count); /* Increment the reference count of the result by the given count. If this result have a master * result, the reference count of the master result is incremented instead. */ diff --git a/source/blender/compositor/intern/input_single_value_operation.cc b/source/blender/compositor/intern/input_single_value_operation.cc index be803293dcc..ffd27e0d239 100644 --- a/source/blender/compositor/intern/input_single_value_operation.cc +++ b/source/blender/compositor/intern/input_single_value_operation.cc @@ -21,7 +21,7 @@ InputSingleValueOperation::InputSingleValueOperation(Context &context, DInputSoc Result result = context.create_result(result_type); /* The result of an input single value operation is guaranteed to have a single user. */ - result.set_initial_reference_count(1); + result.set_reference_count(1); populate_result(result); } diff --git a/source/blender/compositor/intern/node_operation.cc b/source/blender/compositor/intern/node_operation.cc index c2011e22e63..2d20e460709 100644 --- a/source/blender/compositor/intern/node_operation.cc +++ b/source/blender/compositor/intern/node_operation.cc @@ -85,7 +85,7 @@ void NodeOperation::compute_results_reference_counts(const Schedule &schedule) const int reference_count = number_of_inputs_linked_to_output_conditioned( doutput, [&](DInputSocket input) { return schedule.contains(input.node()); }); - get_result(doutput->identifier).set_initial_reference_count(reference_count); + get_result(doutput->identifier).set_reference_count(reference_count); } } diff --git a/source/blender/compositor/intern/operation.cc b/source/blender/compositor/intern/operation.cc index fc819cceab0..1b176d24fc5 100644 --- a/source/blender/compositor/intern/operation.cc +++ b/source/blender/compositor/intern/operation.cc @@ -27,8 +27,6 @@ void Operation::evaluate() { evaluate_input_processors(); - reset_results(); - execute(); compute_preview(); @@ -182,13 +180,6 @@ void Operation::evaluate_input_processors() } } -void Operation::reset_results() -{ - for (Result &result : results_.values()) { - result.reset(); - } -} - void Operation::release_inputs() { for (Result *result : results_mapped_to_inputs_.values()) { diff --git a/source/blender/compositor/intern/pixel_operation.cc b/source/blender/compositor/intern/pixel_operation.cc index 9e795dccd19..b17fa5de9eb 100644 --- a/source/blender/compositor/intern/pixel_operation.cc +++ b/source/blender/compositor/intern/pixel_operation.cc @@ -83,7 +83,7 @@ void PixelOperation::compute_results_reference_counts(const Schedule &schedule) reference_count++; } - get_result(item.value).set_initial_reference_count(reference_count); + get_result(item.value).set_reference_count(reference_count); } } diff --git a/source/blender/compositor/intern/result.cc b/source/blender/compositor/intern/result.cc index 6251c6f46c8..20494e22bb7 100644 --- a/source/blender/compositor/intern/result.cc +++ b/source/blender/compositor/intern/result.cc @@ -353,13 +353,7 @@ void Result::pass_through(Result &target) /* Increment the reference count of the master by the original reference count of the target. */ increment_reference_count(target.reference_count()); - /* Make the target an exact copy of this result, but keep the initial reference count, as this is - * a property of the original result and is needed for correctly resetting the result before the - * next evaluation. */ - const int initial_reference_count = target.initial_reference_count_; target = *this; - target.initial_reference_count_ = initial_reference_count; - target.master_ = this; } @@ -372,12 +366,10 @@ void Result::steal_data(Result &source) /* Overwrite everything except reference counts. */ const int reference_count = reference_count_; - const int initial_reference_count = initial_reference_count_; *this = source; reference_count_ = reference_count; - initial_reference_count_ = initial_reference_count; - source.reset(); + source = Result(*context_, type_, precision_); } /* Returns true if the given GPU texture is compatible with the type and precision of the given @@ -450,17 +442,9 @@ RealizationOptions &Result::get_realization_options() return domain_.realization_options; } -void Result::set_initial_reference_count(int count) +void Result::set_reference_count(int count) { - initial_reference_count_ = count; -} - -void Result::reset() -{ - const int initial_reference_count = initial_reference_count_; - *this = Result(*context_, type_, precision_); - initial_reference_count_ = initial_reference_count; - reference_count_ = initial_reference_count; + reference_count_ = count; } void Result::increment_reference_count(int count) @@ -541,7 +525,7 @@ void Result::free() bool Result::should_compute() { - return initial_reference_count_ != 0; + return reference_count_ != 0; } DerivedResources &Result::derived_resources() diff --git a/source/blender/compositor/intern/simple_operation.cc b/source/blender/compositor/intern/simple_operation.cc index 5cfe8c9d9c1..51133c5527b 100644 --- a/source/blender/compositor/intern/simple_operation.cc +++ b/source/blender/compositor/intern/simple_operation.cc @@ -39,7 +39,7 @@ void SimpleOperation::populate_result(Result result) Operation::populate_result(output_identifier_, result); /* The result of a simple operation is guaranteed to have a single user. */ - get_result().set_initial_reference_count(1); + get_result().set_reference_count(1); } void SimpleOperation::declare_input_descriptor(InputDescriptor descriptor) diff --git a/source/blender/render/intern/compositor.cc b/source/blender/render/intern/compositor.cc index 1c8cecdb416..c71d4e76b39 100644 --- a/source/blender/render/intern/compositor.cc +++ b/source/blender/render/intern/compositor.cc @@ -184,7 +184,7 @@ class Context : public compositor::Context { /* Otherwise, the size changed, so release its data and reset it, then we reallocate it on * the new render size below. */ output_result_.release(); - output_result_.reset(); + output_result_ = this->create_result(compositor::ResultType::Color); } output_result_.allocate_texture(render_size, false); @@ -208,7 +208,7 @@ class Context : public compositor::Context { /* Otherwise, the size or precision changed, so release its data and reset it, then we * reallocate it on the new domain below. */ viewer_output_result_.release(); - viewer_output_result_.reset(); + viewer_output_result_ = this->create_result(compositor::ResultType::Color); } viewer_output_result_.set_precision(precision);