diff --git a/source/blender/compositor/COM_evaluator.hh b/source/blender/compositor/COM_evaluator.hh index c155c90a7ff..c8139095853 100644 --- a/source/blender/compositor/COM_evaluator.hh +++ b/source/blender/compositor/COM_evaluator.hh @@ -22,27 +22,15 @@ using namespace nodes::derived_node_tree_types; /* ------------------------------------------------------------------------------------------------ * Evaluator * - * The evaluator is the main class of the compositor and the entry point of its execution. The - * evaluator compiles the compositor node tree and evaluates it to compute its output. It is - * constructed from a compositor node tree and a compositor context. Upon calling the evaluate - * method, the evaluator will check if the node tree is already compiled into an operations stream, - * and if it is, it will go over it and evaluate the operations in order. It is then the - * responsibility of the caller to call the reset method when the node tree changes to invalidate - * the operations stream. A reset is also required if the resources used by the node tree change, - * for instances, when the dimensions of an image used by the node tree changes. This is necessary - * because the evaluator compiles the node tree into an operations stream that is specifically - * optimized for the structure of the resources used by the node tree. + * The evaluator is the main class of the compositor and the entry point of its execution. It is + * constructed from a compositor node tree and a compositor context. It compiles the node tree into + * an operations stream, evaluating the operations in the process. It should be noted that + * operations are eagerly evaluated as soon as they are compiled, as opposed to compiling the whole + * operations stream and then evaluating it in a separate step. This is done because the evaluator + * uses the evaluated results of previously compiled operations to compile the operations that + * follow them in an optimized manner. * - * Otherwise, if the node tree is not yet compiled, the evaluator will compile it into an - * operations stream, evaluating the operations in the process. It should be noted that operations - * are evaluated as soon as they are compiled, as opposed to compiling the whole operations stream - * and then evaluating it in a separate step. This is important because, as mentioned before, the - * operations stream is optimized specifically for the structure of the resources used by the node - * tree, which is only known after the operations are evaluated. In other words, the evaluator uses - * the evaluated results of previously compiled operations to compile the operations that follow - * them in an optimized manner. - * - * Compilation starts by computing an optimized node execution schedule by calling the + * Evaluation starts by computing an optimized node execution schedule by calling the * compute_schedule function, see the discussion in COM_scheduler.hh for more details. For the node * tree shown below, the execution schedule is denoted by the node numbers. The compiler then goes * over the execution schedule in order and compiles each node into either a Node Operation or a @@ -101,51 +89,30 @@ class Evaluator { private: /* A reference to the compositor context. */ Context &context_; - /* A derived node tree representing the compositor node tree. This is constructed when the node - * tree is compiled and reset when the evaluator is reset, so it gets reconstructed every time - * the node tree changes. */ + /* A derived node tree representing the compositor node tree. */ std::unique_ptr derived_node_tree_; - /* The compiled operations stream. This contains ordered pointers to the operations that were - * compiled. This is initialized when the node tree is compiled and freed when the evaluator - * resets. The is_compiled_ member indicates whether the operation stream can be used or needs to - * be compiled first. Note that the operations stream can be empty even when compiled, this can - * happen when the node tree is empty or invalid for instance. */ + /* The compiled operations stream, which contains all compiled operations so far. */ Vector> operations_stream_; - /* True if the node tree is already compiled into an operations stream that can be evaluated - * directly. False if the node tree is not compiled yet and needs to be compiled. */ - bool is_compiled_ = false; public: /* Construct an evaluator from a context. */ Evaluator(Context &context); - /* Evaluate the compositor node tree. If the node tree is already compiled into an operations - * stream, that stream will be evaluated directly. Otherwise, the node tree will be compiled and - * evaluated. */ + /* Evaluates the compositor node tree by compiling it into an operations stream and evaluating + * it. */ void evaluate(); - /* Invalidate the operations stream that was compiled for the node tree. This should be called - * when the node tree changes or the structure of any of the resources used by it changes. By - * structure, we mean things like the dimensions of the used images, while changes to their - * contents do not necessitate a reset. */ - void reset(); - private: - /* Check if the compositor node tree is valid by checking if it has: - * - Cyclic links. - * - Undefined nodes or sockets. - * - Unsupported nodes. - * If the node tree is valid, true is returned. Otherwise, false is returned, and an appropriate - * error message is set by calling the context's set_info_message method. */ + /* Check if the compositor node tree is valid by checking if it has things like cyclic links and + * undefined nodes or sockets. If the node tree is valid, true is returned. Otherwise, false is + * returned, and an appropriate error message is set by calling the context's set_info_message + * method. */ bool validate_node_tree(); - /* Compile the node tree into an operations stream and evaluate it. */ - void compile_and_evaluate(); - /* Compile the given node into a node operation, map each input to the result of the output * linked to it, update the compile state, add the newly created operation to the operations * stream, and evaluate the operation. */ - void compile_and_evaluate_node(DNode node, CompileState &compile_state); + void evaluate_node(DNode node, CompileState &compile_state); /* Map each input of the node operation to the result of the output linked to it. Unlinked inputs * are mapped to the result of a newly created Input Single Value Operation, which is added to @@ -160,7 +127,7 @@ class Evaluator { * the result of the output linked to it, update the compile state, add the newly created * operation to the operations stream, evaluate the operation, and finally reset the pixel * compile unit. */ - void compile_and_evaluate_pixel_compile_unit(CompileState &compile_state); + void evaluate_pixel_compile_unit(CompileState &compile_state); /* Map each input of the pixel operation to the result of the output linked to it. This might * also correct the reference counts of the results, see the implementation for more details. */ diff --git a/source/blender/compositor/intern/evaluator.cc b/source/blender/compositor/intern/evaluator.cc index c3fdaea3d82..93f72bb0cee 100644 --- a/source/blender/compositor/intern/evaluator.cc +++ b/source/blender/compositor/intern/evaluator.cc @@ -2,6 +2,8 @@ * * SPDX-License-Identifier: GPL-2.0-or-later */ +#include "BLI_memory_utils.hh" + #include "DNA_node_types.h" #include "NOD_derived_node_tree.hh" @@ -28,30 +30,44 @@ void Evaluator::evaluate() { context_.reset(); - if (!is_compiled_) { - compile_and_evaluate(); + BLI_SCOPED_DEFER([&]() { + if (context_.profiler()) { + context_.profiler()->finalize(context_.get_node_tree()); + } + }); + + derived_node_tree_ = std::make_unique(context_.get_node_tree()); + + if (!this->validate_node_tree()) { + return; } - else { - for (const std::unique_ptr &operation : operations_stream_) { - if (context_.is_canceled()) { - this->cancel_evaluation(); - break; - } - operation->evaluate(); + + if (context_.is_canceled()) { + this->cancel_evaluation(); + return; + } + + const Schedule schedule = compute_schedule(context_, *derived_node_tree_); + + CompileState compile_state(schedule); + + for (const DNode &node : schedule) { + if (context_.is_canceled()) { + this->cancel_evaluation(); + return; + } + + if (compile_state.should_compile_pixel_compile_unit(node)) { + this->evaluate_pixel_compile_unit(compile_state); + } + + if (is_pixel_node(node)) { + compile_state.add_node_to_pixel_compile_unit(node); + } + else { + this->evaluate_node(node, compile_state); } } - - if (context_.profiler()) { - context_.profiler()->finalize(context_.get_node_tree()); - } -} - -void Evaluator::reset() -{ - operations_stream_.clear(); - derived_node_tree_.reset(); - - is_compiled_ = false; } bool Evaluator::validate_node_tree() @@ -69,47 +85,7 @@ bool Evaluator::validate_node_tree() return true; } -void Evaluator::compile_and_evaluate() -{ - derived_node_tree_ = std::make_unique(context_.get_node_tree()); - - if (!validate_node_tree()) { - return; - } - - if (context_.is_canceled()) { - this->cancel_evaluation(); - reset(); - return; - } - - const Schedule schedule = compute_schedule(context_, *derived_node_tree_); - - CompileState compile_state(schedule); - - for (const DNode &node : schedule) { - if (context_.is_canceled()) { - this->cancel_evaluation(); - reset(); - return; - } - - if (compile_state.should_compile_pixel_compile_unit(node)) { - compile_and_evaluate_pixel_compile_unit(compile_state); - } - - if (is_pixel_node(node)) { - compile_state.add_node_to_pixel_compile_unit(node); - } - else { - compile_and_evaluate_node(node, compile_state); - } - } - - is_compiled_ = true; -} - -void Evaluator::compile_and_evaluate_node(DNode node, CompileState &compile_state) +void Evaluator::evaluate_node(DNode node, CompileState &compile_state) { NodeOperation *operation = node->typeinfo->get_compositor_operation(context_, node); @@ -174,7 +150,7 @@ static PixelOperation *create_pixel_operation(Context &context, CompileState &co return new ShaderOperation(context, compile_unit, schedule); } -void Evaluator::compile_and_evaluate_pixel_compile_unit(CompileState &compile_state) +void Evaluator::evaluate_pixel_compile_unit(CompileState &compile_state) { PixelCompileUnit &compile_unit = compile_state.get_pixel_compile_unit(); @@ -204,10 +180,10 @@ void Evaluator::compile_and_evaluate_pixel_compile_unit(CompileState &compile_st const PixelCompileUnit end_compile_unit(compile_unit.as_span().drop_front(split_index)); compile_state.get_pixel_compile_unit() = start_compile_unit; - this->compile_and_evaluate_pixel_compile_unit(compile_state); + this->evaluate_pixel_compile_unit(compile_state); compile_state.get_pixel_compile_unit() = end_compile_unit; - this->compile_and_evaluate_pixel_compile_unit(compile_state); + this->evaluate_pixel_compile_unit(compile_state); /* No need to continue, the above recursive calls will eventually exist the loop and do the * actual compilation. */ diff --git a/source/blender/draw/engines/compositor/compositor_engine.cc b/source/blender/draw/engines/compositor/compositor_engine.cc index 0ecb79e14d1..911b3b3bcb0 100644 --- a/source/blender/draw/engines/compositor/compositor_engine.cc +++ b/source/blender/draw/engines/compositor/compositor_engine.cc @@ -209,51 +209,14 @@ class Context : public compositor::Context { class Engine { private: Context context_; - compositor::Evaluator evaluator_; - /* Stores the compositing region size at the time the last compositor evaluation happened. See - * the update_compositing_region_size method for more information. */ - int2 last_compositing_region_size_; public: - Engine(char *info_message) - : context_(info_message), - evaluator_(context_), - last_compositing_region_size_(context_.get_compositing_region_size()) - { - } + Engine(char *info_message) : context_(info_message) {} - /* Update the compositing region size and evaluate the compositor. */ void draw() { - update_compositing_region_size(); - /* We temporally disable caching of node tree compilation by always resting the evaluator for - * now. See pull request #134394 for more information. TODO: This should be cleaned up in the - * future. */ - evaluator_.reset(); - evaluator_.evaluate(); - } - - /* If the size of the compositing region changed from the last time the compositor was evaluated, - * update the last compositor region size and reset the evaluator. That's because the evaluator - * compiles the node tree in a manner that is specifically optimized for the size of the - * compositing region. This should be called before evaluating the compositor. */ - void update_compositing_region_size() - { - if (last_compositing_region_size_ == context_.get_compositing_region_size()) { - return; - } - - last_compositing_region_size_ = context_.get_compositing_region_size(); - - evaluator_.reset(); - } - - /* If the compositor node tree changed, reset the evaluator. */ - void update(const Depsgraph *depsgraph) - { - if (DEG_id_type_updated(depsgraph, ID_NT)) { - evaluator_.reset(); - } + compositor::Evaluator evaluator(context_); + evaluator.evaluate(); } }; @@ -313,18 +276,6 @@ static void compositor_engine_draw(void *data) #endif } -static void compositor_engine_update(void *data) -{ - COMPOSITOR_Data *compositor_data = static_cast(data); - - /* Clear any info message that was set in a previous update. */ - compositor_data->info[0] = '\0'; - - if (compositor_data->instance_data) { - compositor_data->instance_data->update(DRW_context_state_get()->depsgraph); - } -} - DrawEngineType draw_engine_compositor_type = { /*next*/ nullptr, /*prev*/ nullptr, @@ -336,7 +287,7 @@ DrawEngineType draw_engine_compositor_type = { /*cache_populate*/ nullptr, /*cache_finish*/ nullptr, /*draw_scene*/ &compositor_engine_draw, - /*view_update*/ &compositor_engine_update, + /*view_update*/ nullptr, /*id_update*/ nullptr, /*render_to_image*/ nullptr, /*store_metadata*/ nullptr, diff --git a/source/blender/render/intern/compositor.cc b/source/blender/render/intern/compositor.cc index 4529ccd7507..1c8cecdb416 100644 --- a/source/blender/render/intern/compositor.cc +++ b/source/blender/render/intern/compositor.cc @@ -629,8 +629,6 @@ class Compositor { } } - /* Always recreate the evaluator, as this only runs on compositing node changes and - * there is no reason to cache this. Unlike the viewport where it helps for navigation. */ { compositor::Evaluator evaluator(*context_); evaluator.evaluate();