Refactor: Compositor: Assert for unneeded allocations

This patch refactors how the compositor deals with outputs that are
allocated but not needed. Previously we allowed such allocations and
implicitly release them right after operations are computed. This is a
weak design, so we now require developers to only allocate outputs that
are actually needed and assert otherwise. The release mechanism is
therefore removed.
This commit is contained in:
Omar Emara
2025-03-04 15:05:41 +02:00
parent 0a9dc14c2e
commit b03e9be22d
4 changed files with 6 additions and 36 deletions

View File

@@ -174,12 +174,6 @@ class Operation {
* 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
* the evaluation of the operation. */
void release_unneeded_results();
};
} // namespace blender::compositor

View File

@@ -224,18 +224,7 @@ class Result {
* same evaluation.
*
* If the context of the result uses GPU, then GPU allocation will be done, otherwise, CPU
* allocation will be done.
*
* If the result should not be computed, that is, should_compute() returns false, yet this method
* is called, that means the result is only being allocated because the shader that computes it
* also computes another result that is actually needed, and shaders needs to have a texture
* bound to all their images units for a correct invocation, even if some of those textures are
* not needed and will eventually be discarded. In that case, since allocating the full texture
* is not needed, allocate_single_value() is called instead and the reference count is set to 1.
* This essentially allocates a dummy 1x1 texture, which works because out of bound shader writes
* to images are safe. Since this result is not referenced by any other operation, it should be
* manually released after the operation is evaluated, which is implemented by calling the
* Operation::release_unneeded_results() method. */
* allocation will be done. */
void allocate_texture(Domain domain, bool from_pool = true);
/* Declare the result to be a single value result, allocate a texture of an appropriate type with

View File

@@ -35,8 +35,6 @@ void Operation::evaluate()
release_inputs();
release_unneeded_results();
context().evaluate_operation_post();
}
@@ -164,15 +162,6 @@ 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_;

View File

@@ -261,13 +261,8 @@ eGPUTextureFormat Result::get_gpu_texture_format() const
void Result::allocate_texture(Domain domain, bool from_pool)
{
/* The result is not actually needed, so allocate a dummy single value texture instead. See the
* method description for more information. */
if (!should_compute()) {
allocate_single_value();
increment_reference_count();
return;
}
/* Make sure we are not allocating a result that should not be computed. */
BLI_assert(this->should_compute());
is_single_value_ = false;
this->allocate_data(domain.size, from_pool);
@@ -276,6 +271,9 @@ void Result::allocate_texture(Domain domain, bool from_pool)
void Result::allocate_single_value()
{
/* Make sure we are not allocating a result that should not be computed. */
BLI_assert(this->should_compute());
/* Single values are stored in 1x1 image as well as the single value members. Further, they
* are always allocated from the pool. */
is_single_value_ = true;