Skip to content

Commit

Permalink
GH-423 Avoid reuse of default value instances
Browse files Browse the repository at this point in the history
  • Loading branch information
Naros committed Jun 22, 2024
1 parent 070481c commit 07c8e84
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 10 deletions.
25 changes: 25 additions & 0 deletions src/script/instances/node_instance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,28 @@ Ref<OScriptNode> OScriptNodeInstance::get_base_node()
{
return { _base };
}

OScriptNodeInstance::~OScriptNodeInstance()
{
if (input_pin_count > 0)
{
if (input_pins != nullptr)
memdelete_arr(input_pins);
if (input_default_stack_pos != nullptr)
memdelete_arr(input_default_stack_pos);
}

if (output_pin_count > 0)
{
if (output_pins != nullptr)
memdelete_arr(output_pins);
}

if (execution_output_pin_count > 0)
{
if (execution_output_pins != nullptr)
memdelete_arr(execution_output_pins);
if (execution_outputs != nullptr)
memdelete_arr(execution_outputs);
}
}
4 changes: 4 additions & 0 deletions src/script/instances/node_instance.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ class OScriptNodeInstance : public Object
Vector<OScriptNodeInstance*> dependencies; //! List of node instance dependencies for this node
int* input_pins{ nullptr }; //! Input pins
int input_pin_count{ 0 }; //! Input pin count
int* input_default_stack_pos{ nullptr }; //! Holds stack position references for default values
int* output_pins{ nullptr }; //! Output pins
int output_pin_count{ 0 }; //! Output pin count
int working_memory_index{ 0 }; //! Number of working memory slots
Expand All @@ -109,6 +110,9 @@ class OScriptNodeInstance : public Object
/// @param p_context execution context
/// @return the output port and bits
virtual int step(OScriptExecutionContext& p_context) = 0;

/// Destructor
~OScriptNodeInstance() override;
};

#endif // ORCHESTRATOR_SCRIPT_NODE_INSTANCE_H
7 changes: 6 additions & 1 deletion src/script/vm/execution_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,13 @@ class OScriptExecutionContext

/// Set the input value at the specified index to the default value
/// @param p_index the input stack index to mutate
/// @param p_stack_index the stack position to write the value
/// @param p_value the default value to use
_FORCE_INLINE_ void _set_input_from_default_value(int p_index, const Variant& p_value) { _inputs[p_index] = const_cast<Variant*>(&p_value); }
_FORCE_INLINE_ void _set_input_from_default_value(int p_index, int p_stack_index, const Variant& p_value)
{
_variant_stack[p_stack_index] = p_value.duplicate();
_inputs[p_index] = &_variant_stack[p_stack_index];
}

/// Copies a variant stack value to the input stack at the given indices
/// @param p_stack_index the variant stack index to copy from
Expand Down
35 changes: 27 additions & 8 deletions src/script/vm/script_vm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,20 +75,35 @@ static Ref<OScriptNodePin> get_data_pin_at_count_index(const Ref<OScriptNode>& p
return nullptr;
}

void OScriptVirtualMachine::_set_unassigned_inputs(const Ref<OScriptNode>& p_node, const OScriptNodeInstance* p_instance)
void OScriptVirtualMachine::_set_unassigned_inputs(const Ref<OScriptNode>& p_node, const OScriptNodeInstance* p_instance, Function& r_function)
{
for (int i = 0; i < p_instance->data_input_pin_count; i++)
{
// If the input pin is bound with a value of -1, it is unassigned.
if (p_instance->input_pins[i] == -1)
{
// Assign the default value index
p_instance->input_pins[i] = _default_values.size() | OScriptNodeInstance::INPUT_DEFAULT_VALUE_BIT;

// Place the pin's effective default value on the default value list
const Ref<OScriptNodePin> pin = get_data_pin_at_count_index(p_node, i, PD_Input);
if (pin.is_valid())
if (!pin.is_valid())
continue;

// Default values should be passed on the stack, assign a stack position for the value
int stack_pos = r_function.max_stack++;
p_instance->input_default_stack_pos[i] = stack_pos;

// Rather than duplicate default values for each node, reuse existing ones if possible.
int lookup_index = _default_values.find(pin->get_effective_default_value());
if (lookup_index != -1)
{
// Default value already exists, assign the stack position
p_instance->input_pins[i] = lookup_index | OScriptNodeInstance::INPUT_DEFAULT_VALUE_BIT;
}
else
{
// Default value doesn't exist, create a new entry
p_instance->input_pins[i] = _default_values.size() | OScriptNodeInstance::INPUT_DEFAULT_VALUE_BIT;
_default_values.push_back(pin->get_effective_default_value());
}
}
}
}
Expand Down Expand Up @@ -198,8 +213,12 @@ bool OScriptVirtualMachine::_create_node_instance_pins(const Ref<OScriptNode>& p
// Create the input array
// Each defaults to -1, and if left as -1 triggers default value serialization
p_instance->input_pins = memnew_arr(int, p_instance->data_input_pin_count);
p_instance->input_default_stack_pos = memnew_arr(int, p_instance->data_input_pin_count);
for (int i = 0; i < p_instance->data_input_pin_count; i++)
{
p_instance->input_pins[i] = -1;
p_instance->input_default_stack_pos[i] = -1;
}
}

if (p_instance->data_output_pin_count)
Expand Down Expand Up @@ -407,7 +426,7 @@ bool OScriptVirtualMachine::_build_function_node_graph(const Ref<OScriptFunction
const Ref<OScriptNode>& node = p_function->get_orchestration()->get_node(E);
const OScriptNodeInstance* instance = _nodes[E];

_set_unassigned_inputs(node, instance);
_set_unassigned_inputs(node, instance, r_function);
_set_unassigned_outputs(node, instance, r_function.trash_pos);
}

Expand Down Expand Up @@ -448,7 +467,7 @@ void OScriptVirtualMachine::_resolve_inputs(OScriptExecutionContext& p_context,
{
const int index = p_instance->input_pins[i] & OScriptNodeInstance::INPUT_MASK;
if (p_instance->input_pins[i] & OScriptNodeInstance::INPUT_DEFAULT_VALUE_BIT)
p_context._set_input_from_default_value(i, _default_values[index]);
p_context._set_input_from_default_value(i, p_instance->input_default_stack_pos[i], _default_values[index]);
else
p_context._copy_stack_to_input(index, i);
}
Expand Down Expand Up @@ -509,7 +528,7 @@ void OScriptVirtualMachine::_dependency_step(OScriptExecutionContext& p_context,
{
const int index = p_instance->input_pins[i] & OScriptNodeInstance::INPUT_MASK;
if (p_instance->input_pins[i] & OScriptNodeInstance::INPUT_DEFAULT_VALUE_BIT)
p_context.set_input(i, &_default_values[index]);
p_context._set_input_from_default_value(i, p_instance->input_default_stack_pos[i], _default_values[index]);
else
p_context._copy_stack_to_input(index, i);
}
Expand Down
3 changes: 2 additions & 1 deletion src/script/vm/script_vm.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ class OScriptVirtualMachine
/// Sets unassigned inputs on the specified node, if any exist.
/// @param p_node the script node
/// @param p_instance the node instance
void _set_unassigned_inputs(const Ref<OScriptNode>& p_node, const OScriptNodeInstance* p_instance);
/// @param r_function the function declaration
void _set_unassigned_inputs(const Ref<OScriptNode>& p_node, const OScriptNodeInstance* p_instance, Function& r_function);

/// Sets unassigned outputs on the specified node, if any exist.
/// @param p_node the script node
Expand Down

0 comments on commit 07c8e84

Please sign in to comment.