blender
/
blender
136
411
Fork 595
Code Issues 6.3k Pull Requests 774 Projects 19 Wiki Activity

Geometry Nodes: add Skip input to Simulation Output node #112140

Merged
Jacques Lucke merged 12 commits from JacquesLucke/blender:sim-muted into main 2023-09-09 09:53:08 +02:00
Conversation 5 Commits 12 Files Changed 4 +129 -57
4 changed files with 129 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
source/blender/modifiers/intern/MOD_nodes.cc
View File

@ -892,36 +892,32 @@ class NodesModifierSimulationParams : public nodes::GeoNodesSimulationParams {
void output_store_frame_cache(bake::NodeCache &node_cache,
nodes::SimulationZoneBehavior &zone_behavior) const
{
auto &store_and_pass_through_info =
zone_behavior.output.emplace<sim_output::StoreAndPassThrough>();
store_and_pass_through_info.store_fn =
[simulation_cache = modifier_cache_,
node_cache = &node_cache,
current_frame = current_frame_](bke::bake::BakeState state) {
std::lock_guard lock{simulation_cache->mutex};
auto frame_cache = std::make_unique<bake::FrameCache>();
frame_cache->frame = current_frame;
frame_cache->state = std::move(state);
node_cache->frame_caches.append(std::move(frame_cache));
};
auto &store_new_state_info = zone_behavior.output.emplace<sim_output::StoreNewState>();
store_new_state_info.store_fn = [simulation_cache = modifier_cache_,
node_cache = &node_cache,
current_frame = current_frame_](bke::bake::BakeState state) {
std::lock_guard lock{simulation_cache->mutex};
auto frame_cache = std::make_unique<bake::FrameCache>();
frame_cache->frame = current_frame;
frame_cache->state = std::move(state);
node_cache->frame_caches.append(std::move(frame_cache));
};
}
void store_as_prev_items(bake::NodeCache &node_cache,
nodes::SimulationZoneBehavior &zone_behavior) const
{
auto &store_and_pass_through_info =
zone_behavior.output.emplace<sim_output::StoreAndPassThrough>();
store_and_pass_through_info.store_fn =
[simulation_cache = modifier_cache_,
node_cache = &node_cache,
current_frame = current_frame_](bke::bake::BakeState state) {
std::lock_guard lock{simulation_cache->mutex};
if (!node_cache->prev_cache) {
node_cache->prev_cache.emplace();
}
node_cache->prev_cache->state = std::move(state);
node_cache->prev_cache->frame = current_frame;
};
auto &store_new_state_info = zone_behavior.output.emplace<sim_output::StoreNewState>();
store_new_state_info.store_fn = [simulation_cache = modifier_cache_,
node_cache = &node_cache,
current_frame = current_frame_](bke::bake::BakeState state) {
std::lock_guard lock{simulation_cache->mutex};
if (!node_cache->prev_cache) {
node_cache->prev_cache.emplace();
}
node_cache->prev_cache->state = std::move(state);
node_cache->prev_cache->frame = current_frame;
};
}
void read_from_cache(const FrameIndices &frame_indices,

12
source/blender/nodes/NOD_geometry_nodes_lazy_function.hh
View File

@ -77,17 +77,17 @@ using Behavior = std::variant<PassThrough, OutputCopy, OutputMove>;
namespace sim_output {
/**
* The data is just passed through the node. Data that is incompatible with simulations (like
* anonymous attributes), is removed though.
* Output the data that comes from the corresponding simulation input node, ignoring the nodes in
* the zone.
*/
struct PassThrough {
};
/**
* Same as above, but also calls the given function with the data that is passed through the node.
* This allows the caller of geometry nodes (e.g. the modifier), to cache the new simulation state.
* Computes the simulation step and calls the given function to cache the new simulation state.
* The new simulation state is the output of the node.
*/
struct StoreAndPassThrough {
struct StoreNewState {
std::function<void(bke::bake::BakeState state)> store_fn;
};
@ -108,7 +108,7 @@ struct ReadInterpolated {
bke::bake::BakeStateRef next_state;
};
using Behavior = std::variant<PassThrough, StoreAndPassThrough, ReadSingle, ReadInterpolated>;
using Behavior = std::variant<PassThrough, StoreNewState, ReadSingle, ReadInterpolated>;
} // namespace sim_output

111
source/blender/nodes/geometry/nodes/node_geo_simulation_output.cc
View File

@ -30,6 +30,8 @@
#include "NOD_add_node_search.hh"
#include "BLT_translation.h"
#include "node_geometry_util.hh"
namespace blender::nodes {
@ -40,7 +42,9 @@ std::string socket_identifier_for_simulation_item(const NodeSimulationItem &item
}
static std::unique_ptr<SocketDeclaration> socket_declaration_for_simulation_item(
const NodeSimulationItem &item, const eNodeSocketInOut in_out, const int index)
const NodeSimulationItem &item,
const eNodeSocketInOut in_out,
const int corresponding_input = -1)
{
const eNodeSocketDatatype socket_type = eNodeSocketDatatype(item.socket_type);
BLI_assert(NOD_geometry_simulation_output_item_socket_type_supported(socket_type));
@ -50,32 +54,38 @@ static std::unique_ptr<SocketDeclaration> socket_declaration_for_simulation_item
case SOCK_FLOAT:
decl = std::make_unique<decl::Float>();
decl->input_field_type = InputSocketFieldType::IsSupported;
decl->output_field_dependency = OutputFieldDependency::ForPartiallyDependentField({index});
decl->output_field_dependency = OutputFieldDependency::ForPartiallyDependentField(
{corresponding_input});
break;
case SOCK_VECTOR:
decl = std::make_unique<decl::Vector>();
decl->input_field_type = InputSocketFieldType::IsSupported;
decl->output_field_dependency = OutputFieldDependency::ForPartiallyDependentField({index});
decl->output_field_dependency = OutputFieldDependency::ForPartiallyDependentField(
{corresponding_input});
break;
case SOCK_RGBA:
decl = std::make_unique<decl::Color>();
decl->input_field_type = InputSocketFieldType::IsSupported;
decl->output_field_dependency = OutputFieldDependency::ForPartiallyDependentField({index});
decl->output_field_dependency = OutputFieldDependency::ForPartiallyDependentField(
{corresponding_input});
break;
case SOCK_BOOLEAN:
decl = std::make_unique<decl::Bool>();
decl->input_field_type = InputSocketFieldType::IsSupported;
decl->output_field_dependency = OutputFieldDependency::ForPartiallyDependentField({index});
decl->output_field_dependency = OutputFieldDependency::ForPartiallyDependentField(
{corresponding_input});
break;
case SOCK_ROTATION:
decl = std::make_unique<decl::Rotation>();
decl->input_field_type = InputSocketFieldType::IsSupported;
decl->output_field_dependency = OutputFieldDependency::ForPartiallyDependentField({index});
decl->output_field_dependency = OutputFieldDependency::ForPartiallyDependentField(
{corresponding_input});
break;
case SOCK_INT:
decl = std::make_unique<decl::Int>();
decl->input_field_type = InputSocketFieldType::IsSupported;
decl->output_field_dependency = OutputFieldDependency::ForPartiallyDependentField({index});
decl->output_field_dependency = OutputFieldDependency::ForPartiallyDependentField(
{corresponding_input});
break;
case SOCK_STRING:
decl = std::make_unique<decl::String>();
@ -96,10 +106,12 @@ static std::unique_ptr<SocketDeclaration> socket_declaration_for_simulation_item
void socket_declarations_for_simulation_items(const Span<NodeSimulationItem> items,
NodeDeclaration &r_declaration)
{
const int inputs_offset = r_declaration.inputs.size();
for (const int i : items.index_range()) {
const NodeSimulationItem &item = items[i];
SocketDeclarationPtr input_decl = socket_declaration_for_simulation_item(item, SOCK_IN, i);
SocketDeclarationPtr output_decl = socket_declaration_for_simulation_item(item, SOCK_OUT, i);
SocketDeclarationPtr input_decl = socket_declaration_for_simulation_item(item, SOCK_IN);
SocketDeclarationPtr output_decl = socket_declaration_for_simulation_item(
item, SOCK_OUT, inputs_offset + i);
r_declaration.inputs.append(input_decl.get());
r_declaration.items.append(std::move(input_decl));
r_declaration.outputs.append(output_decl.get());
@ -516,6 +528,18 @@ static void mix_simulation_state(const NodeSimulationItem &item,
class LazyFunctionForSimulationOutputNode final : public LazyFunction {
const bNode &node_;
Span<NodeSimulationItem> simulation_items_;
int skip_input_index_;
Hans Goudey commented 2023-09-08 17:57:21 +02:00
Review
Copy Link

Sorry to get stuck on this, but I don't really understand the reasoning for storing these-- can't they be determined at runtime?

skip_input_index_ = 0
skip_inputs_offset_ = 1
solve_inputs_offset_ = simulation_items_.size() + 1

If that's right, I think it's a bit misleading to store them here rather than compute them on the fly.

Sorry to get stuck on this, but I don't really understand the reasoning for storing these-- can't they be determined at runtime? `skip_input_index_ = 0` `skip_inputs_offset_ = 1` `solve_inputs_offset_ = simulation_items_.size() + 1` If that's right, I think it's a bit misleading to store them here rather than compute them on the fly.
Jacques Lucke commented 2023-09-08 18:01:28 +02:00
Review
Copy Link

The reason I store them is that this makes the code a little less brittle. It allows storing the index at the same place where it is "created". Without this, it is much easier to mess up indices when moving/adding/removing sockets.

The reason I store them is that this makes the code a little less brittle. It allows storing the index at the same place where it is "created". Without this, it is much easier to mess up indices when moving/adding/removing sockets.
Hans Goudey commented 2023-09-08 18:05:35 +02:00
Review
Copy Link

I think asserts are another way to do that, but I don't care enough to continue this topic :P

I think asserts are another way to do that, but I don't care enough to continue this topic :P
Jacques Lucke commented 2023-09-08 18:10:20 +02:00
Review
Copy Link

I don't quite see how asserts should solve this. Here it's relatively simple still, because there are only three blocks of sockets. Computing the indices on the fly becomes harder and more error prone the more such blocks there are. If this would have a measurable impact on performance, I might choose differently, but it doesn't.

I don't quite see how asserts should solve this. Here it's relatively simple still, because there are only three blocks of sockets. Computing the indices on the fly becomes harder and more error prone the more such blocks there are. If this would have a measurable impact on performance, I might choose differently, but it doesn't.
/**
Hans Goudey commented 2023-09-08 17:06:56 +02:00
Outdated
Review
Copy Link

I think there needs to be some place where there's a comment about these extra links. As is, these three variable names are the best description of the design, which isn't really satisfying IMO. Looking back on the PR description it's obvious, but it took me a minute to figure out the purpose of these variables.

Maybe add a function to get these and return them as IndexRange or even Array<void *>? That would give a place to say something like "Retrieve the inputs connected directly to the simulation input node for when the "skip" is used"

I think there needs to be some place where there's a comment about these extra links. As is, these three variable names are the best description of the design, which isn't really satisfying IMO. Looking back on the PR description it's obvious, but it took me a minute to figure out the purpose of these variables. Maybe add a function to get these and return them as `IndexRange` or even `Array<void *>`? That would give a place to say something like "Retrieve the inputs connected directly to the simulation input node for when the "skip" is used"
* Start index of the simulation state inputs that are used when the simulation is skipped. Those
* inputs are linked directly to the simulation input node. Those inputs only exist internally,
* but not in the UI.
*/
int skip_inputs_offset_;
/**
* Start index of the simulation state inputs that are used when the simulation is actually
* computed. Those correspond to the sockets that are visible in the UI.
*/
int solve_inputs_offset_;
public:
LazyFunctionForSimulationOutputNode(const bNode &node,
@ -528,9 +552,26 @@ class LazyFunctionForSimulationOutputNode final : public LazyFunction {
MutableSpan<int> lf_index_by_bsocket = own_lf_graph_info.mapping.lf_index_by_bsocket;
const bNodeSocket &skip_bsocket = node.input_socket(0);
skip_input_index_ = inputs_.append_and_get_index_as(
"Skip", *skip_bsocket.typeinfo->geometry_nodes_cpp_type, lf::ValueUsage::Maybe);
lf_index_by_bsocket[skip_bsocket.index_in_tree()] = skip_input_index_;
skip_inputs_offset_ = inputs_.size();
/* Add the skip inputs that are linked to the simulation input node. */
for (const int i : simulation_items_.index_range()) {
const NodeSimulationItem &item = simulation_items_[i];
const bNodeSocket &input_bsocket = node.input_socket(i);
const CPPType &type = get_simulation_item_cpp_type(item);
inputs_.append_as(item.name, type, lf::ValueUsage::Maybe);
}
solve_inputs_offset_ = inputs_.size();
/* Add the solve inputs that correspond to the simulation state inputs in the UI. */
for (const int i : simulation_items_.index_range()) {
const NodeSimulationItem &item = simulation_items_[i];
const bNodeSocket &input_bsocket = node.input_socket(i + 1);
const bNodeSocket &output_bsocket = node.output_socket(i);
const CPPType &type = get_simulation_item_cpp_type(item);
@ -583,8 +624,8 @@ class LazyFunctionForSimulationOutputNode final : public LazyFunction {
else if (std::get_if<sim_output::PassThrough>(&output_behavior)) {
this->pass_through(params, user_data);
}
else if (auto *info = std::get_if<sim_output::StoreAndPassThrough>(&output_behavior)) {
this->store_and_pass_through(params, user_data, *info);
else if (auto *info = std::get_if<sim_output::StoreNewState>(&output_behavior)) {
this->store_new_state(params, user_data, *info);
}
else {
BLI_assert_unreachable();
@ -649,10 +690,8 @@ class LazyFunctionForSimulationOutputNode final : public LazyFunction {
void pass_through(lf::Params &params, GeoNodesLFUserData &user_data) const
{
/* Instead of outputting the initial values directly, convert them to a simulation state and
* then back. This ensures that some geometry processing happens on the data consistently (e.g.
* removing anonymous attributes). */
std::optional<bke::bake::BakeState> bake_state = this->get_bake_state_from_inputs(params);
std::optional<bke::bake::BakeState> bake_state = this->get_bake_state_from_inputs(params,
true);
if (!bake_state) {
/* Wait for inputs to be computed. */
return;
@ -673,11 +712,21 @@ class LazyFunctionForSimulationOutputNode final : public LazyFunction {
}
}
void store_and_pass_through(lf::Params &params,
GeoNodesLFUserData &user_data,
const sim_output::StoreAndPassThrough &info) const
void store_new_state(lf::Params &params,
GeoNodesLFUserData &user_data,
const sim_output::StoreNewState &info) const
{
std::optional<bke::bake::BakeState> bake_state = this->get_bake_state_from_inputs(params);
const bool *skip = params.try_get_input_data_ptr_or_request<bool>(skip_input_index_);
if (skip == nullptr) {
/* Wait for skip input to be computed. */
return;
}
JacquesLucke marked this conversation as resolved Outdated
Hans Goudey commented 2023-09-08 14:13:14 +02:00
Outdated
Review
Copy Link

I find this force_pass_through input confusing. Maybe it would be unnecessary if the functions were split up a bit? That way we wouldn't have the "boolean input significantly changes how a function works" situation here.

I find this `force_pass_through` input confusing. Maybe it would be unnecessary if the functions were split up a bit? That way we wouldn't have the "boolean input significantly changes how a function works" situation here.
/* Instead of outputting the values directly, convert them to a bake state and then back. This
* ensures that some geometry processing happens on the data consistently (e.g. removing
* anonymous attributes). */
std::optional<bke::bake::BakeState> bake_state = this->get_bake_state_from_inputs(params,
*skip);
if (!bake_state) {
/* Wait for inputs to be computed. */
return;
@ -686,11 +735,14 @@ class LazyFunctionForSimulationOutputNode final : public LazyFunction {
info.store_fn(std::move(*bake_state));
}
std::optional<bke::bake::BakeState> get_bake_state_from_inputs(lf::Params &params) const
std::optional<bke::bake::BakeState> get_bake_state_from_inputs(lf::Params &params,
const bool skip) const
{
Array<void *> input_values(inputs_.size());
for (const int i : inputs_.index_range()) {
input_values[i] = params.try_get_input_data_ptr_or_request(i);
/* Choose which set of input parameters to use. The others are ignored. */
const int params_offset = skip ? skip_inputs_offset_ : solve_inputs_offset_;
Array<void *> input_values(simulation_items_.size());
for (const int i : simulation_items_.index_range()) {
input_values[i] = params.try_get_input_data_ptr_or_request(i + params_offset);
}
if (input_values.as_span().contains(nullptr)) {
/* Wait for inputs to be computed. */
@ -722,6 +774,17 @@ static void node_declare_dynamic(const bNodeTree & /*node_tree*/,
NodeDeclaration &r_declaration)
{
const NodeGeometrySimulationOutput &storage = node_storage(node);
{
SocketDeclarationPtr skip_decl = std::make_unique<decl::Bool>();
skip_decl->name = "Skip";
skip_decl->identifier = skip_decl->name;
skip_decl->description = N_(
"Forward the output of the simulation input node directly to the output node and ignore "
"the nodes in the simulation zone");
skip_decl->in_out = SOCK_IN;
r_declaration.inputs.append(skip_decl.get());
r_declaration.items.append(std::move(skip_decl));
}
socket_declarations_for_simulation_items({storage.items, storage.items_num}, r_declaration);
}

17
source/blender/nodes/intern/geometry_nodes_lazy_function.cc
View File

@ -916,8 +916,7 @@ class LazyFunctionForSimulationInputsUsage : public LazyFunction {
params.set_output(
1,
solve_contains_side_effect ||
std::holds_alternative<sim_output::PassThrough>(zone_behavior->output) ||
std::holds_alternative<sim_output::StoreAndPassThrough>(zone_behavior->output));
std::holds_alternative<sim_output::StoreNewState>(zone_behavior->output));
}
};
@ -1882,6 +1881,8 @@ struct GeometryNodesLazyFunctionGraphBuilder {
const int zone_i = zone.index;
ZoneBuildInfo &zone_info = zone_build_infos_[zone_i];
lf::Graph &lf_graph = scope_.construct<lf::Graph>();
const auto &sim_output_storage = *static_cast<const NodeGeometrySimulationOutput *>(
zone.output_node->storage);
lf::Node *lf_zone_input_node = nullptr;
lf::Node *lf_main_input_usage_node = nullptr;
@ -1925,6 +1926,18 @@ struct GeometryNodesLazyFunctionGraphBuilder {
graph_params.usage_by_bsocket.add(bsocket, &lf_simulation_usage_node.output(1));
}
/* Link simulation input node directly to simulation output node for skip behavior. */
for (const int i : IndexRange(sim_output_storage.items_num)) {
lf::InputSocket &lf_to = lf_simulation_output.input(i + 1);
if (lf_simulation_input) {
lf::OutputSocket &lf_from = lf_simulation_input->output(i + 1);
lf_graph.add_link(lf_from, lf_to);
}
else {
lf_to.set_default_value(lf_to.type().default_value());
}
}
this->insert_nodes_and_zones(zone.child_nodes, zone.child_zones, graph_params);
if (zone.input_node) {